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: Support inline nodes with content in @tiptap/suggestion #2648

Merged
merged 3 commits into from
Apr 29, 2022

Conversation

thatsjonsense
Copy link
Contributor

Problem: @tiptap/suggestion's findSuggestionMatch only works when inline nodes are leaves. When you have an inline node with content, in our case a footnote modeled off of https://prosemirror.net/examples/footnote/, suggestions after that node are broken.

This happens because the current logic relies on textBetween returning a string whose length exactly matches the absolute position range of the node. This is true with text nodes, as well as leaf nodes like a mention, but not a content node like footnote because its opening/close take up position spaces.

Solution: I don't think textBetween on the entire parent node is actually needed here. Just looking at the text node immediately before the suggestion fixes the problem and seems simpler as well.

@netlify
Copy link

netlify bot commented Mar 24, 2022

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit fadf440
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/623cc7fafe7b930009fe692e
😎 Deploy Preview https://deploy-preview-2648--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.

@bdbch
Copy link
Contributor

bdbch commented Apr 9, 2022

Hey @thatsjonsense, thanks for your pull request. Do you have a demo where you could show the previous/after behaviour so we can see the issue and how the fix is resolving that?

As far as I understood the problem is that a footnote contains more content in the popover which is not respected via textBetween?

I'll take a look into this PR again if you can help me review this PR so we can get your fix merged ASAP! 👍

@bdbch bdbch self-assigned this Apr 9, 2022
@thatsjonsense
Copy link
Contributor Author

Hey @thatsjonsense, thanks for your pull request. Do you have a demo where you could show the previous/after behaviour so we can see the issue and how the fix is resolving that?

As far as I understood the problem is that a footnote contains more content in the popover which is not respected via textBetween?

I'll take a look into this PR again if you can help me review this PR so we can get your fix merged ASAP! 👍

Hi @bdbch thanks for offering to review. Here is a CSB demonstrating the issue and the fix: https://codesandbox.io/s/tiptap-footnote-suggestion-fix-0ef5c9?file=/src/App.js

Just go to mention/mention.ts and uncomment line 5 to see the fix in action.

@bdbch
Copy link
Contributor

bdbch commented Apr 29, 2022

Great one! Thanks @thatsjonsense !

@bdbch bdbch merged commit 0819e29 into ueberdosis:main Apr 29, 2022
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.

None yet

2 participants