search

nytimes / react-prosemirror

A library for safely integrating ProseMirror and React.
Other
433 stars 15 forks source link

NodeView is detached from DOM during `scrollIntoView` transaction #75

Open konstantinmuenster opened 9 months ago

konstantinmuenster commented 9 months ago

Issue

We use a node view that contains a list of items. These items can be indented and outdented via ProseMirror's helper commands sinkListItem and liftListItem. We noticed that these transactions don't work well with react-prosemirror's rendering cycle.

When ProseMirror executes the scrollIntoView transaction at the very end of the command, the node view is still detached from the DOM.

https://github.com/nytimes/react-prosemirror/assets/46243719/fbff800b-c456-4220-96cd-04b0862ba331

Due to that, the scroll calculation doesn't lead to the expected result. For example, if you lift/sink an item at a position further down in the document, it always scrolls the page back to the very top. This is how it behaves with and without node views:

https://github.com/nytimes/react-prosemirror/assets/46243719/63096870-0e80-405b-a468-71232cbb6345

How To Reproduce

I added a setup that reproduces the issue here: https://stackblitz.com/edit/stackblitz-starters-wu7os2

smoores-dev commented 9 months ago

Thank you for the detailed bug report, @konstantinmuenster! This does make sense; the React/ProseMirror integration renders the node views in a layout effect, so any commands that try to synchronously interact with the DOM after dispatching transactions are going to run into this issue. This shouldn't be an issue with the "new architecture" (available on the next tag on npm), and it's possible that, in contradiction with what I mentioned to @alexanderjulmer early, this is an indication that dskrpt might want to look into trying out that new architecture.

In the meantime, I will see whether there's anything to be done about this specific issue, though I suspect that there are other, similarly subtle flavors of it that you may run into. It's possible that an interim solution may involve writing your own list commands that don't immediately call scrollIntoView, and delegate that responsibility to React, somehow, so that you can ensure that the components have rendered before it runs.