[lexical][lexical-rich-text] Bug Fix: Restore Shift+Cmd+Arrow selection extension on leading inline DecoratorNode#8583
Closed
mayrang wants to merge 1 commit into
Closed
Conversation
…on extension on leading inline DecoratorNode PR facebook#8577 removed shiftKey: 'any' from isMoveToStart / isMoveToEnd to avoid a reported crash on decorator-only paragraphs. This also disabled the selection-extension functionality facebook#8558 added (per etrepum: the shiftKey behavior was intentional to catch an intentional selection expansion). Root cause: avoid the selectEnd() recursion chain on decorator-only elements instead of disabling the matcher. Restore shiftKey: 'any' on the matchers and rewrite MOVE_TO_END to call node.select() directly, branching on lastDescendant — text → text.select(), otherwise → element.select(childrenSize, childrenSize) for the decorator-only case. Drop MOVE_TO_START's lastDescendant guard: that handler never called selectStart(), so the guard was symmetry-only.
mayrang
requested review from
acywatson,
etrepum,
fantactuka,
ivailop7,
potatowagon and
zurfyx
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
Contributor
Author
|
Duplicate of #8581 — @potatowagon already opened it with the matcher restore. Closing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PR #8577 removed
shiftKey: 'any'fromisMoveToStart/isMoveToEndto avoid a reported crash on decorator-only paragraphs. That also disabled the Shift+Cmd+Arrow selection-extension functionality #8558 added. Per the #8577 discussion, the matcher's Shift behavior was intentional — the right fix is to address theselectEnd()recursion on decorator-only elements, not to disable the matcher.What changed
isMoveToStart/isMoveToEndaccept Shift again (shiftKey: 'any'restored).MOVE_TO_ENDhandler callsnode.select()directly, branching onlastDescendant: text →text.select(), otherwise →element.select(childrenSize, childrenSize). The decorator-only path is now explicit instead of routed throughelement.selectEnd()→decorator.selectEnd()→parent.select(...).MOVE_TO_STARThandler drops thelastDescendant === decoratorguard. That handler never calledselectStart(), so the guard was symmetry-only.Behavior on decorator-only paragraphs
Cmd+ArrowLeft / Right now move the caret to the position before / after the decorator. Shift variants extend the selection over it. Previously these fell through to native, which left the caret stuck.
Out of scope
text [decorator] long-wrapping-textstill uses native Cmd+ArrowRight (= visual line end) while[decorator] long-wrapping-textjumps to paragraph end via this handler. That inconsistency was introduced by #8558 and isn't touched here.Test plan
RichTextInlineDecoratorMoveToEnd.test.ts+RichTextInlineDecoratorMoveToStart.test.ts—decorator-onlycases moved out ofno-opinto cursor-move / decorator-selected expectations; mixed-content cases preserved.pnpm vitest run --project unit) — 140 files / 2764 pass / 1 skip / 0 fail.pnpm tsc --noEmit -p tsconfig.json,pnpm flow, prettier, eslint clean.[equation]):[eq]|+ Cmd+ArrowLeft → caret moves to element offset 0.[eq]|+ Shift+Cmd+ArrowLeft → selection (anchor=element 1, focus=element 0).|[eq]+ Cmd+ArrowRight → caret moves to element offset 1.|[eq]+ Shift+Cmd+ArrowRight → selection (anchor=element 0, focus=element 1).[equation]hello):|[eq]hello+ Cmd+ArrowRight → caret at text "hello" end.|[eq]hello+ Shift+Cmd+ArrowRight → selection (anchor=element 0, focus=text end).[eq]hello|+ Cmd+ArrowLeft → caret at element offset 0.[eq]hello|+ Shift+Cmd+ArrowLeft → selection (anchor=text end, focus=element 0).[equation]long-wrapping-text— Cmd+ArrowRight / Shift+Cmd+ArrowRight reach paragraph end (handler fires).text [equation] long-wrapping-text— handler doesn't fire (first child is text); native Cmd+ArrowRight reaches visual line end only.Refs #8558, #8577.