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

[WIP][lexical][lexical-selection] Manually handle selection around TextNode boundaries #7317

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

etrepum
Copy link
Collaborator

@etrepum etrepum commented Mar 12, 2025

Description

See #7301 - when alternative display is used for TextNode elements (e.g. not inline span) the behavior of the DOM's Selection.modify doesn't really work anymore because it jumps from one container to the next instead of across to the next logical character. By catching these scenarios and moving the DOM selection into the next element in advance this handles all of the (known) scenarios with arrows and deletions.

The arrow keys need to be handled unconditionally at TextNode boundaries because the selection is often out of sync with exactly what the Lexical points are.

Other issues likely still exist when moving by word or line, but character is the important granularity for most situations.

For future consideration we might want to move RangeSelection.modify into a command, or parts of it into a command, so that people can more easily work around these sorts of issues.

Closes #7301

Test plan

TBD - The plan will probably be to add an e2e suite outside of the playground with the project sitting in scripts/__tests__/integration/fixtures/lexical-adjacent-span-deletion which can be used to demonstrate that this bug is fixed.

Before

Run with release version of lexical, observe that arrow keys and delete doesn't work consistently (the provided workarounds useNavigateBase and useSpanDeletion are disabled)

cd scripts/__tests__/integration/fixtures/lexical-adjacent-span-deletion
npm install
npm run dev

After

Run with the monorepo version of lexical, observe that the arrow keys and delete work correctly

cd scripts/__tests__/integration/fixtures/lexical-adjacent-span-deletion
npm install
npm run monorepo:dev

…Node boundaries which is necessary when display is not inline
Copy link

vercel bot commented Mar 12, 2025

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 Mar 12, 2025 3:27pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 12, 2025 3:27pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 12, 2025
@etrepum etrepum added extended-tests Run extended e2e tests on a PR and removed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 12, 2025
@etrepum
Copy link
Collaborator Author

etrepum commented Mar 12, 2025

Ignore the failing unit tests for now, I don't think that suite has any real value as-is (it's testing a mock implementation that doesn't really behave like any real browser) and needs to be converted to an e2e suite.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Proper Handling of Deletions Across Adjacent Unmergeable Text Spans in Grid Containers
2 participants