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): fix insertContentAt keeping new lines in html content #4465

Merged
merged 5 commits into from
Jan 8, 2024

Conversation

bdbch
Copy link
Contributor

@bdbch bdbch commented Sep 21, 2023

Please describe your changes

This PR fixes the issue described in #2720 which was caused by keeping new lines in string content and using the DOMParser which will add breaks for new lines too.

How did you accomplish your changes

I strip out \n content from the content string so new lines are interpretet by the DOMParser only.

How have you tested your changes

I created a demo where this can be tested. (See here)

How can we verify your changes

Test in the demo above

Checklist

  • The changes are not breaking the editor
  • Added tests where possible
  • Followed the guidelines
  • Fixed linting issues

Related issues

@bdbch bdbch self-assigned this Sep 21, 2023
@netlify
Copy link

netlify bot commented Sep 21, 2023

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit ca6546f
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/6558678b2d6595000816762b
😎 Deploy Preview https://deploy-preview-4465--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 bdbch changed the base branch from develop to main September 21, 2023 08:54
@bdbch
Copy link
Contributor Author

bdbch commented Sep 21, 2023

@svenadlung can you take a look too? self reviewing right now.

@bdbch
Copy link
Contributor Author

bdbch commented Sep 21, 2023

Update: Found some more issues I'd have to take a look into.

@bdbch
Copy link
Contributor Author

bdbch commented Sep 21, 2023

Okay - ready. @svenadlung if you have some time a review would be great.

@bdbch
Copy link
Contributor Author

bdbch commented Sep 28, 2023

Ping @svenadlung

@svenadlung
Copy link
Contributor

@bdbch Sorry for the delay. At the first moment it feels to me like something could break of the change. Please give me some more time for a review. Probably until the end of next week due to public holidays.

@gdfreitas
Copy link

We're also facing this issue, do we have any updates? @svenadlung

@kaiserkiwi
Copy link

Any progress here?

@bdbch
Copy link
Contributor Author

bdbch commented Jan 8, 2024

@svenadlung should we include this in our next release or should we postpone it for a later release?

@bdbch bdbch merged commit 135a12f into main Jan 8, 2024
14 checks passed
@bdbch bdbch deleted the bdbch/fix-2720 branch January 8, 2024 19:21
@murisceman
Copy link

This fix breaks <pre><code>foo\nbar</code></pre>.

@bdbch
Copy link
Contributor Author

bdbch commented Jan 10, 2024

working on a fix @murisceman

@bdbch
Copy link
Contributor Author

bdbch commented Jan 10, 2024

PR is here: #4767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

The insertContent command adds unnecessary blank paragraphs with HTML as value
6 participants