tiptap icon indicating copy to clipboard operation
tiptap copied to clipboard

Tweak `getTextBetween` to prepend `blockSeparator` to block node even if block node doesn't have any children

Open hivokas opened this issue 1 year ago • 4 comments

Please describe your changes

Currently getTextBetween function doesn't prepend blockSeparator to block nodes without children.

This PR updates it to prepend blockSeparator to block nodes in any case (even if block node doesn't have any children).

How did you accomplish your changes

I have tweaked the implementation to not depend on child nodes of blocks when deciding whether blockSeparator should be prepended.

How have you tested your changes

I used preview link generated by Netlify bot below.

How can we verify your changes

Use preview link generated by Netlify bot below.

Remarks

Below are before/after examples.

Without empty paragraph

Hello!
How are you?

Before: 'Hello!'+BLOCK_SEPARATOR+'How are you?' After: 'Hello!'+BLOCK_SEPARATOR+'How are you?'

With empty paragraphs

Hello!

How are you?

Before: 'Hello!'+BLOCK_SEPARATOR+'How are you?' After: 'Hello!'+BLOCK_SEPARATOR+BLOCK_SEPARATOR+'How are you?' (it's breaking change, but I consider it a bugfix)

Checklist

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

Related issues

N/A

hivokas avatar Apr 11 '24 09:04 hivokas

Deploy Preview for tiptap-embed ready!

Name Link
Latest commit 7cfb56fbcd49ac3e4227616e45f1c562ba676b2d
Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/662f3ad4b2dfa50008d74d88
Deploy Preview https://deploy-preview-5055--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.

netlify[bot] avatar Apr 11 '24 09:04 netlify[bot]

@bdbch @svenadlung +1 on this one Currently concurrent empty like breaks are not reflected in getText()

E.g. in the screenshots between line 2 and 5 there are 2 paragraphs (line breaks), but getText only returns one \n

getText vs DOM:

Selection_414

Selection_413

knitevision1 avatar Apr 15 '24 15:04 knitevision1

@knitevision1 if this PR gets merged, you (and I) will be able to achieve the desired result with getText({ blockSeparator: '\n' }) (default separator is \n\n).

hivokas avatar Apr 18 '24 16:04 hivokas

@bdbch could you take a look at this PR when you get a moment? Looking at reactions in this PR, it looks like I'm not the only one who wants this change.

hivokas avatar Apr 29 '24 06:04 hivokas

Thanks @bdbch!

hivokas avatar May 10 '24 07:05 hivokas

When will this get released? So happy with this change, it's made dealing with whitespaced text content so annoying

itzcull avatar May 14 '24 00:05 itzcull

@itzcull it's released 🎉

hivokas avatar May 15 '24 06:05 hivokas