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

Fix cursor not following to new node when using a react node view #3331

Conversation

ruipserra
Copy link
Contributor

@ruipserra ruipserra commented Oct 20, 2022

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 the flushSync call into a microtask. This delays the flushSync, 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 the componentDidMount and componentDidUpdate 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):

diff --git a/demos/src/GuideNodeViews/ReactComponentContent/React/Extension.js b/demos/src/GuideNodeViews/ReactComponentContent/React/Extension.js
index b3cde8328..6d70f3c61 100644
--- a/demos/src/GuideNodeViews/ReactComponentContent/React/Extension.js
+++ b/demos/src/GuideNodeViews/ReactComponentContent/React/Extension.js
@@ -10,6 +10,18 @@ export default Node.create({
 
   content: 'inline*',
 
+  addKeyboardShortcuts() {
+    return {
+      Enter: ({ editor }) => {
+        if (editor.state.selection.$from.parent.type.name === this.name) {
+          return this.editor.chain().splitBlock().setNode(this.name).run()
+        }
+
+        return false
+      }
+    }
+  },
+
   parseHTML() {
     return [
       {

Then I tested hitting enter at the end of the react node:

Before After
before.mov
after.mov

Additional changes in this PR

  • Moved the code that was updating EditorContent's state in ReactRenderer into instance methods in EditorContent.
  • Updated 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

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.
@netlify
Copy link

netlify bot commented Oct 20, 2022

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit c602f72
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/6350baceba0c0c0008608dbf
😎 Deploy Preview https://deploy-preview-3331--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@stevemckenzie
Copy link

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.

@ruipserra
Copy link
Contributor Author

ruipserra commented Oct 24, 2022

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 git@github.com: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

@stevemckenzie
Copy link

@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!

@mylesj
Copy link
Contributor

mylesj commented Nov 4, 2022

Oh I didn't see a notification for this. Nice one!
I can confirm that this build fixes the original issue for me too.

@stevemckenzie
Copy link

So how do we get this merged? It's a pretty bad bug for React users.

@svenadlung
Copy link
Contributor

@stevemckenzie we are on it. I am pretty sure it will be in next release. Sorry for the inconvenience.

@stevemckenzie
Copy link

@svenadlung no worries, thanks for the quick update!

@stevemckenzie
Copy link

stevemckenzie commented Nov 15, 2022

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:

content: 'text*',

group: 'inline',

inline: true,

isTextblock: true,

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 splitBlock() call is happening. Any thoughts?

Edit:

I reported this as its own issue.

@bdbch bdbch self-assigned this Nov 25, 2022
Copy link
Contributor

@bdbch bdbch left a 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.

@bdbch bdbch merged commit 369f109 into ueberdosis:main Nov 25, 2022
bdbch pushed a commit that referenced this pull request Mar 27, 2023
* Add custom paragraph example

* Remove unnecessary queueMicrotask
aliasliao pushed a commit to aliasliao/tiptap that referenced this pull request May 24, 2023
…berdosis#3533 (ueberdosis#3862)

* Add custom paragraph example

* Remove unnecessary queueMicrotask
andrewlu0 pushed a commit to trybaseplate/tiptap that referenced this pull request Oct 20, 2023
…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.
andrewlu0 pushed a commit to trybaseplate/tiptap that referenced this pull request Oct 20, 2023
…berdosis#3533 (ueberdosis#3862)

* Add custom paragraph example

* Remove unnecessary queueMicrotask
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

Successfully merging this pull request may close these issues.

Cursor does not follow new line / paragraph when configured with a node view
5 participants