Skip to content

[lexical-utils] fix: Backward selection was not being retained#7737

Merged
ivailop7 merged 3 commits intofacebook:mainfrom
jvithlani:jv_reverse_selection
Aug 4, 2025
Merged

[lexical-utils] fix: Backward selection was not being retained#7737
ivailop7 merged 3 commits intofacebook:mainfrom
jvithlani:jv_reverse_selection

Conversation

@jvithlani
Copy link
Copy Markdown
Contributor

@jvithlani jvithlani commented Aug 4, 2025

Description

Problem
The selectionAlwaysOnDisplay utility wasn't working correctly for right-to-left text selections (when users drag from right to left). The visual selection mark would not appear properly when the editor lost focus after making a right-to-left selection.

Root Cause
The issue was in the rangeFromPoints function in markSelection.ts. The function was only checking focusNode.isBefore(anchorNode) to determine selection direction, which works for selections across different DOM nodes but fails for selections within the same text node.

For same-node selections:
Left-to-right: anchor.offset < focus.offset (e.g., 5 → 10)
Right-to-left: anchor.offset > focus.offset (e.g., 10 → 5)
The original logic didn't account for this offset-based direction detection.

Solution
Enhanced the selection direction detection logic to handle both cases:

Closes #7736

Before

Screen.Recording.2025-08-04.at.6.55.16.PM.mov

After

Screen.Recording.2025-08-04.at.9.32.03.PM.mov

@meta-cla meta-cla 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 Aug 4, 2025
@vercel
Copy link
Copy Markdown

vercel bot commented Aug 4, 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 Aug 4, 2025 7:54pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2025 7:54pm

await expect(heightDifference).toBeLessThanOrEqual(5);
});

test(`retain selection works with reverse selection`, async ({
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmed this fails on the main branch

Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simpler fix would be to use the isBackwards method of RangeSelection, e.g.

function $getOrderedSelectionPoints(selection: RangeSelection): [PointType, PointType] {
  // there's no reason for this to be nullable when called from RangeSelection, that could be fixed
  const points = selection.getStartEndPoints()!;
  return selection.isBackward() ? points.reverse() : points;
}

With this method to retrieve the anchor and focus, you could remove the broken direction code from rangeFromPoints altogether.

@etrepum etrepum added the extended-tests Run extended e2e tests on a PR label Aug 4, 2025
@jvithlani
Copy link
Copy Markdown
Contributor Author

something like this @etrepum ?

Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is what I was thinking, although not necessarily with all of the renaming (but that does make it clearer, so perfectly ok!)

@ivailop7 ivailop7 changed the title fix: Backward selection was not being retained [lexical-utils] fix: Backward selection was not being retained Aug 4, 2025
@ivailop7 ivailop7 added this pull request to the merge queue Aug 4, 2025
Merged via the queue into facebook:main with commit abd7130 Aug 4, 2025
40 checks passed
@etrepum etrepum mentioned this pull request Aug 7, 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.

Bug: Selection doesnt retain when text is selected from right to left

3 participants