-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fix cursor not following to new node when using a react node view #3331
Fix cursor not following to new node when using a react node view #3331
Conversation
The React docs recommend treating the `state` as immutable. See e.g.: https://reactjs.org/docs/react-component.html#state
This fixes the cursor problem described in tiptap#3200 Node views need to be rendered immediately when they're created so that the editor can correctly position the cursor. That's achieved using `flushSync` whenever a new node view renderer is added. However, `flushSync` cannot be used from inside a React component lifecycle method. By keeping an instance variable to determine if initialization has happened, we can avoid using `flushSync` from inside the `componentDidMount` and `componentDidUpdate` methods, and still call it whenever a new node view is created afterwards.
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Thanks for this @ruipserra ! This fixes the problem for me but the cursor doesn't actually show until you start typing so you can't actually see that the new node is in focus. |
Hey @stevemckenzie glad to hear your original problem was fixed! I tried the code from the CodeSandbox you linked in #3338 with the fix from this PR and I couldn't see any issues with the cursor. I uploaded a branch to my fork with the code, would you mind giving it a try? git clone [email protected]:ruipserra/tiptap.git ruipserra/tiptap
cd ruipserra/tiptap
git checkout custom-react-paragraph
npm install
npm start And then open http://localhost:3000/preview/Examples/CustomParagraph in your browser. This is what I see btw: custom_react_paragraph.mov |
@ruipserra well that's odd lol. I actually changed my project to temporarily use your fork already and ya for some reason the cursor just doesn't show when a new custom paragraph is created until I start to type so the focus clearly works properly. But you are correct, your example above works as expected so I will dig more into our own setup. Thanks for working on this! |
Oh I didn't see a notification for this. Nice one! |
So how do we get this merged? It's a pretty bad bug for React users. |
@stevemckenzie we are on it. I am pretty sure it will be in next release. Sorry for the inconvenience. |
@svenadlung no worries, thanks for the quick update! |
I was just doing some more testing around this and found another issue that I'm not sure if it is related. I have a custom extension with the following configuration:
When I press enter, nothing happens. I also can't seem to move the cursor past this element once the cursor is at the end of it. I assume this is related to it being an inline block. I can confirm that the Edit: I reported this as its own issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR. Looks pretty good to me. I'll add this for the next release.
* Add custom paragraph example * Remove unnecessary queueMicrotask
…berdosis#3533 (ueberdosis#3862) * Add custom paragraph example * Remove unnecessary queueMicrotask
…berdosis#3331) * Refactor: extract `setRenderer` and `removeRenderer` methods * Refactor: avoid using a mutable ES6 Map in React component state The React docs recommend treating the `state` as immutable. See e.g.: https://reactjs.org/docs/react-component.html#state * Fix: flush EditorContent state changes immediately once initialized This fixes the cursor problem described in tiptap#3200 Node views need to be rendered immediately when they're created so that the editor can correctly position the cursor. That's achieved using `flushSync` whenever a new node view renderer is added. However, `flushSync` cannot be used from inside a React component lifecycle method. By keeping an instance variable to determine if initialization has happened, we can avoid using `flushSync` from inside the `componentDidMount` and `componentDidUpdate` methods, and still call it whenever a new node view is created afterwards.
…berdosis#3533 (ueberdosis#3862) * Add custom paragraph example * Remove unnecessary queueMicrotask
This fixes the cursor problem described in #3200.
Node views need to be rendered immediately when they're created so that the editor can correctly position the cursor inside them. That's achieved using
flushSync
whenever a new node view renderer is added.However,
flushSync
cannot be used from inside a React component lifecycle method. Thus #3188 moved theflushSync
call into a microtask. This delays theflushSync
, and so the editor can't correctly position the cursor inside the new node view, because it doesn't exist in the DOM yet.By keeping an instance variable to determine if initialization has happened, we can avoid using
flushSync
from inside thecomponentDidMount
andcomponentDidUpdate
methods, and still call it whenever a new node view is created afterwards.How I tested this
I added the following code to the
ReactComponentContent
demo (but I didn't commit this):Then I tested hitting enter at the end of the react node:
before.mov
after.mov
Additional changes in this PR
EditorContent
's state inReactRenderer
into instance methods inEditorContent
.EditorContent
to keep the renderers in a JS object instead of an ES6 Map. ES6 Maps are mutable, and React discourages mutating state.Fixes #3200