Skip to content

Fix/insert content at block insertions #5651

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

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

bdbch
Copy link
Member

@bdbch bdbch commented Sep 24, 2024

Changes Overview

This PR fixes an issue that was brought up by a community member about how the insertContent and insertContentAt commands always split text nodes when the selection starts at the beginning of the text. This would always lead to the inserted content having an empty paragraph inserted above it.

In the following example I'll try to point out the issue more

<p>Hello World</p>

Let's say your cursor right now is at the start of Hello World and you would insert an image, the resulting HTML previously would return

<!-- This paragraph was created because Tiptap was -->
<!-- trying to insert the node literally into the --> 
<!-- paragraph at position 1 which won't work so it splits it -->
<p></p>
<img src="https://image.domain/path.jpg" alt="My image" />
<p>Hello World</p>

After this fix the HTML will result to this:

<img src="https://image.domain/path.jpg" alt="My image" />
<p>Hello World</p>

When the content is inserted between existing text, it will just work as usual via splitting the paragraph/text node apart.

Implementation Approach

Before we run the actual inserting of block node content, I added a check to see if

  1. The cursor is currently at the start of it's parent node
  2. The cursor is inside a text or textblock node

If both requirements are met, we jump out of the node via from =- 1 to prepend said node before the current text block.

Testing Done

I added two test cases for those scenarios and both run valid + did manual tests.

Verification Steps

  1. Check the tests (should run through)
  2. Clone manually if needed & open the insertContent demo

Additional Notes

This is a major change as it could break existing insertContent usages.

Checklist

  • I have created a changeset for this PR if necessary.
  • My changes do not break the library.
  • I have added tests where applicable.
  • I have followed the project guidelines.
  • I have fixed any lint issues.

@bdbch bdbch requested a review from svenadlung as a code owner September 24, 2024 14:44
Copy link

changeset-bot bot commented Sep 24, 2024

🦋 Changeset detected

Latest commit: 86b1a4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 54 packages
Name Type
@tiptap/core Major
@tiptap/extension-blockquote Major
@tiptap/extension-bold Major
@tiptap/extension-bubble-menu Major
@tiptap/extension-bullet-list Major
@tiptap/extension-character-count Major
@tiptap/extension-code-block-lowlight Major
@tiptap/extension-code-block Major
@tiptap/extension-code Major
@tiptap/extension-collaboration-cursor Major
@tiptap/extension-collaboration Major
@tiptap/extension-color Major
@tiptap/extension-document Major
@tiptap/extension-dropcursor Major
@tiptap/extension-floating-menu Major
@tiptap/extension-focus Major
@tiptap/extension-font-family Major
@tiptap/extension-gapcursor Major
@tiptap/extension-hard-break Major
@tiptap/extension-heading Major
@tiptap/extension-highlight Major
@tiptap/extension-history Major
@tiptap/extension-horizontal-rule Major
@tiptap/extension-image Major
@tiptap/extension-italic Major
@tiptap/extension-link Major
@tiptap/extension-list-item Major
@tiptap/extension-list-keymap Major
@tiptap/extension-mention Major
@tiptap/extension-ordered-list Major
@tiptap/extension-paragraph Major
@tiptap/extension-placeholder Major
@tiptap/extension-strike Major
@tiptap/extension-subscript Major
@tiptap/extension-superscript Major
@tiptap/extension-table-cell Major
@tiptap/extension-table-header Major
@tiptap/extension-table-row Major
@tiptap/extension-table Major
@tiptap/extension-task-item Major
@tiptap/extension-task-list Major
@tiptap/extension-text-align Major
@tiptap/extension-text-style Major
@tiptap/extension-text Major
@tiptap/extension-typography Major
@tiptap/extension-underline Major
@tiptap/extension-youtube Major
@tiptap/html Major
@tiptap/pm Major
@tiptap/react Major
@tiptap/starter-kit Major
@tiptap/suggestion Major
@tiptap/vue-2 Major
@tiptap/vue-3 Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@bdbch bdbch requested a review from nperez0111 September 24, 2024 14:44
Copy link

netlify bot commented Sep 24, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 86b1a4d
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/66f55461a79cfa000872d6b3
😎 Deploy Preview https://deploy-preview-5651--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 configuration.

@bdbch
Copy link
Member Author

bdbch commented Sep 26, 2024

@nperez0111 tests should now work - we actually had this fix already implemented but only on an extension level for the HorizontalRule extension. Removed it there.

Copy link
Contributor

@nperez0111 nperez0111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

@bdbch bdbch merged commit 12bb31a into next Sep 26, 2024
14 checks passed
@bdbch bdbch deleted the fix/insert-content-at-block-insertions branch September 26, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants