Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Presentation Click-to-edit fails consistently on second use, for PortableText fields having only a single paragraph (block) #1557

Open
narration-sd opened this issue May 23, 2024 · 6 comments

Comments

@narration-sd
Copy link

narration-sd commented May 23, 2024

As in the title, the simplest form of this problem shows when you have a PT field with just one paragraph in it. On the web page panel, in the stuudio, you can see its blue outline, click, and the field will be selected properly in the editor panel. But click the page to work on another field, click back on the page for the PT field, and the editor will fail to respond.

On this fail moment, the blue rectangle on the PT field appears to fade, actually a narrower border width I think, which possibly might help to localize the state that's failing. And if you scroll the editor side to the field manually, edits on it will still propagate immediately to the web result, so the data stream is still correct, though some aspect of the map may not be. But the clickability will not recover, so long as there is only the one paragraph in the Portable Text field.

If you use return to add another paragraph, then you can click that in the web page with its own blue border, and it will respond. After that you can click the initial paragraph, and it will have returned to live targeting action, even if you scroll the editor any distance of offset to test, to properly be editing that item. Any other paragraphs willl also be fine, and the field will appear fully click-editable. But once you click off to another field, and try to click back, now the second paragraph you brought life back with has become neutralized, though the first works again.

There are various maneuvers you can use, but this is the core of what happens. If there are two PT fields in the document, one of them single line/paragraph, as in the otherwise nicely performing college news page Astro Presentation was introduced to, the dance between the two can be particularly frustrating.

Overview

These errors reported hide as a 'small' problem, easily overlooked, as arrangements demonstrating it here have seemed to test flawlessly over months, but as will be seen, they are actually a quite serious operational situation for Presentation, to the extent that the issue is indeed in Presentation, as seems quite indicated by details of symptoms.

Whether any or all of the three also-reported React warnings on setup contribute, isn't discernable -- some sense that they may not, but that they could. It appears that VisualEditing book-keeping gets mixed up per PortableText blocks, on Studio or page component sides, less likely by the channel between, but possible. Locating the source of the React warnings should indicate whether what they indicated can contribute to this later failure under normal active Presentation activity.

To Reproduce

Start up a Presentation edit of a document having at least the PT field and one other editable. Perform the sequences just described.

Expected behavior

Presentation should always follow a click on a blue-bordered page PT result with a jump in the editor to the paragraph clicked, as other than in this case, it always does.

Screenshots

I'm providing two movies. One of them shows the primary problem, plus a pair of persistent startup warnings React gives on the Presentation -enabled Studio, which I've traced back to happening with versions at least six weeks ago. The code it reports on seems clearly within the Studio.

The second shows a third React warning, which occurs as the web page starts up in the left pane of a Presentation Studio window, or in a tab of its own. Disabling the Presentation VisualEditing component in the Layout of the web page causes that warning also to cease, and the content is clearly from Presentation, referencing as it does Qbservers and so forth.

However, both of the React warnings occur on startup of the Presentation session -- there are no errors or warnings as the click failures occur.

As these are warnings from React's dev-time version, they disappear on production built use, but the faults with Presentation on PT fields that this issue reports, remain as expected fully occurring, whether or not they have relation to what's warned.

Video of the failure sequences in Presentation, startup-only Studio warnings shown towards end
https://github.com/sanity-io/visual-editing/assets/247945/264565a3-6c97-44bf-a2ef-123678e1ab8d

Video of the VisualEditing-related failure on startup of Presentation-enabled web page
https://github.com/sanity-io/visual-editing/assets/247945/eb800c47-e1ec-4fb0-bbf2-fcef3c0a4c02

Zip of the node_modules/.vite/deps chunk file and map, referenced by errors described and viewable towards end of video:
chunk-QWIXSBWO.js.zip

Which versions of Sanity are you using?

$ sanity versions
@sanity/cli (global) 3.36.4 (latest: 3.43.0)
@sanity/image-url 1.0.2 (up to date)
@sanity/react-loader 1.9.19 (up to date)
@sanity/ui 2.1.11 (up to date)
@sanity/vision 3.43.0 (up to date)
sanity 3.43.0 (up to date)

What operating system are you using?

Windows 11 latest up-to-date, carefully maintained, JetBrains WebStorm running node dev server, most observations in Firefox latest, no differences on Chrome latest

Which versions of Node.js / npm are you running?

$ npm -v && node -v
10.2.4
v20.11.1

Additional context

I brought back code with package versions from as early as six weeks prior, and found all in this report happening the same. So it is not a result of recent slight instabilities surrounding product release.

The same problems, unaffected by pnpm framework and more complex schemas, occur on the college system where they were first discovered. That is not the simpler system demostrating here.

It's probably worth mentioning that the Sanity Astro Presentation provision uses a normal ReactLoader operating in a non-presenting (unless debug set) component on the Astro page, sending its data map and streamed results to Presentation-active React elements behind Astro JSX on the page via nanostore atom.

One factor then is that response is probably faster than with a microcaching system as in NextJs implementations, and response speed can sometimes affect how less-than-sychronized code acts, though what happens here is consistent between fast srerverless (Vercel in college case) and local dev runs which are slower.

Security issue?

No issues in what's disclosed here

@narration-sd
Copy link
Author

narration-sd commented May 23, 2024

For Sanity team, I remembered this morning that I've already set up a private repo with the fully implemented Astro Presentation application/Studio, which s exactly the one that you are seeing in the issue and its movies.

It even includes a sample dataset, so you can immediately run it on dev or your preference of deployment.

This was for other purposes at Sanity, so it's already entirely private for you. All I need is your github Id or an email address to invite and thus enable you at Sanity.

All code is visible in this, even the package, so you can rapidly determine if the problem is indeed on the Presentation side, or somehow in how the system is employing it -- fully open mind as always there....

@narration-sd
Copy link
Author

narration-sd commented May 24, 2024

Just a note to explain the referenced post just above -- it's because this same problem identically has shown up in a completely different Presentation implementation.

Thus I think you'll find it in all of them, and that this is the place to have lodged an issue so it 'gets well', which am very confident it will. Best fortune on it.

@narration-sd
Copy link
Author

An update here, and hope that I may not have slowed down attention, by being too explicit above!

  • this bug continues to show always, on up-to-today current versions, and on several applications

  • it will absolutely prevent second or later click-to-edit from a Portable Text which has only one block/paragraph, after you've used click-to-edit once

  • I had to make an 'unusual' workaround for that, to succeed in a Sanity Proof of Concept for a college Marketing director, enclosing the only-ever-single but rich text paragraph's PT with a presentation-aware div.

  • the workaround suffices for the single-paragraph case, but can't for alternate paragraphs in a single portable text

  • again, it's a fault that can easily slip through testing, though a test could be devised, but it will certainly show to real content editors, as they won't be able to click-to-edit the previous paragraph they were on -- a fault enterable in several possible ways.

Fully appreciated you've had your hands full entirely, and hope that's alleviating, or soon :)

@narration-sd
Copy link
Author

narration-sd commented Nov 8, 2024

I've just checked this issue today -- on NextJS.

It is a hard fail, using the cms-sanity example from https://github.com/sanity/nextjs, either a mid-July commit on Next 14, or the. fresh today, on Next 15 and with 'Live'.

I am testing the simplest case, as mentioned in the original issue text.

  • Create a post. Fill in an image and author (must be saved) so that you have basis, then make a one-paragraph content (which is in Portable Text, the failing type).
  • Bringing the page up in Presentation, click the one-paragraph content on the page, which will be properly outlined. You will, as expected, go its entry form on the right pane, and can edit, with live updates.
  • Now click the title, so you go to its form block. Success again.
  • But now find the content again, the one paragraph, and click it. You will stay on the Title form block, not go to the Content block. That is the error.
  • You can notice the blue outline is dimmed slightly, also -- probably has lost a pixel in width.
  • You can never get out of this blocked state, unless you refresh the entire studio. There are some interesting states besides this, but not relevant to discuss.

So now it is clear this problem has nothing to do Astro, but is a basis visual-editing issue. It shows identically on older and current NextJS, whether from the automated Vercel install or from local pnpm run dev at localhost.

Hoping this helps get it solved, and I am including below a screenshot where you can see the simple one-paragraph content involved, and perhaps the slightly dimmer blue outline, along with the entry form block not entered.

This problem showed originally on a college website which had a single-paragraph Portable Text element. As mentioned above, there are other variants, as you lose actually connection to any paragraph, once having clicked and edited on it, until you click another paragraph. The one-paragraph case just gives no way out like this, though it's not a satisfactory workaround even in that case, for content persons.

Again, very much hoping to help.

Screenshot 2024-11-07 160341

_

@narration-sd
Copy link
Author

Very glad to have heard this is in line for fixing now, thanks to team 👍

I've remembered one more case which should be tested, to assure the upgrade.

It uses two paragraphs instead of one, in a Portable Text. The paras caneach be a single line, not more needed.

  • bring up the page in Presentation, as you would normally.
  • pick one of the lines, and click it. The editor in right pane jumps properly to the beginning of that line.
  • now click something else on the page for edit - the title, the image, etc.. This also selects in the edit pane properly.
  • but now click the line you clicked at first. It will not go to its editor point, and will have that slightly dimmed blue outline. This is the fault, same as with only one line.
  • however, now click the other paragraph. It edits properly in the right pane
  • and now, clicking the first line will also work.

This demonstrates that:

  • the fault always occurs, blanking further response of the first paragraph clicked in a Portable Text, even if there are other paragraphs. The fail is not restricted to single-paragraph PT elements.
  • it also demonstrates how it's been easy to miss this fault for so long -- you might think you'd just mis-clicked, if the locked paragraph later becomes responsive again.

Ok, that's it, and should give a satisfying experience, in assuring the fix is in.

Thanks to each,
Clive

@rdunk
Copy link
Member

rdunk commented Nov 12, 2024

Thanks for the clear and detailed report once again! As you mentioned, we've discussed this one internally, replicated and confirmed the issue. Hoping we can get a fix out for you soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants