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

NodeView is detached from DOM during scrollIntoView transaction #75

Open
konstantinmuenster opened this issue Dec 18, 2023 · 1 comment

Comments

@konstantinmuenster
Copy link

konstantinmuenster commented Dec 18, 2023

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.

CleanShot.2023-12-18.at.17.04.13.mp4

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:

CleanShot.2023-12-18.at.16.58.34.mp4

How To Reproduce

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

@smoores-dev
Copy link
Collaborator

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.

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