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(core): only respect text of node before current position (#2937) #2941

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

svenadlung
Copy link
Contributor

@svenadlung svenadlung commented Jun 30, 2022

Hopefully this PR fixes #2937

Since node.textContent always looks at the entire text of the node, the desired behavior worked out well at the end but not once the position is somewhere else within the text.

I replaced it with nodeBefore

Get the node directly before the position, if any. If the position points into a text node, only the part of that node before the position is returned.

@svenadlung svenadlung requested a review from bdbch June 30, 2022 19:07
@netlify
Copy link

netlify bot commented Jun 30, 2022

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit e155b31
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/62bdf47ca2468b0008dd870a
😎 Deploy Preview https://deploy-preview-2941--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.

@svenadlung svenadlung self-assigned this Jun 30, 2022
@gnapse
Copy link

gnapse commented Jul 1, 2022

Hey @svenadlung, I work closely with @rfgamaral, and he's going to be unavailable for a while, so I can take over his involvement in this issue. Would you be so kind to let me know what would be the best way to try these changes locally? I have very little experience with tiptap's development, unlike @rfgamaral.

@svenadlung
Copy link
Contributor Author

svenadlung commented Jul 4, 2022

Hi @gnapse, I think you could try https://deploy-preview-2941--tiptap-embed.netlify.app/preview/ and see if the behaviour works like expected.

If you want to try it on your local machine you could clone the fix/input-rule-fix branch.

@gnapse
Copy link

gnapse commented Jul 5, 2022

Hi @gnapse, I think you could try deploy-preview-2941--tiptap-embed.netlify.app/preview and see if the behaviour works like expected.

If you want to try it on your local machine you could clone the fix/input-rule-fix branch.

Thanks for the pointer. Indeed, it seems to work as expected in that instance:

CleanShot.2022-07-05.at.14.18.30.mp4

As opposed to when I try the same in https://tiptap.dev/examples/default

So what are the next steps to have this become part of a new release? Can I help in any way?

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.

Good find! Thanks for fixing this issue.

@bdbch bdbch merged commit e280a02 into main Jul 6, 2022
@bdbch bdbch deleted the fix/input-rule-fix branch July 6, 2022 10:19
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.

Input rules seem completely broken
3 participants