[lexical] Bug Fix: TextNode in token mode should not be split by removeText

[etrepum]

Description

The #6456 removeText rewrite introduced a regression in token mode behavior where it behaved too similarly to segmented nodes, it would be split and replaced with a plain TextNode instead of being deleted in full. This takes the simple approach of just expanding the selection outwards if the focus or anchor node contains any content from a token node.

Closes #6687

Test plan

Before

There were no tests to show that token TextNode should be deleted in full by removeText

After

Now there are tests that cover removeText behavior for token TextNode

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 1, 2024 8:03pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 1, 2024 8:03pm

[github-actions]

size-limit report 📦

Path	Size
lexical - cjs	29.82 KB (0%)
lexical - esm	29.67 KB (0%)
@lexical/rich-text - cjs	38.42 KB (0%)
@lexical/rich-text - esm	31.51 KB (0%)
@lexical/plain-text - cjs	37.01 KB (0%)
@lexical/plain-text - esm	28.9 KB (0%)
@lexical/react - cjs	40.17 KB (0%)
@lexical/react - esm	32.97 KB (0%)

[adrianmxb]

Appreciate the proposed changes and prefer this approach. Since I didn’t have the chance to follow up with tests earlier and you seem to have everything covered, I’ve gone ahead and closed my PR in favour of this one. That said, I do have a suggestion: it might be worth updating the fixText function to only create a new Text Node when in segmented mode. This would help prevent potential confusion later on.

[etrepum]

I don't think it's really worth the code churn to change fixText, the code now handles the token cases correctly because they all fall under node.getTextContent() === '' or del === 0. If I wrote this originally I might do it a bit differently but the logic down there doesn't seem wrong.

I do wonder if this makes sense at all since it seems that the logic to handle segmented deletion is elsewhere but if this better matches the previous behavior then it probably is the right thing to do.

[adrianmxb]

Just to be clear, I only talk about replacing $isTokenOrSegmented(...) through a node.isSegmented() call. I would argue that relying on implicit behaviours (like node.getTextContent() === '' or del === 0) to exclude token nodes isn't clear to readers.

Making the exclusion explicit avoids potential confusion.

[potatowagon]

thanks for adding a test case