Skip to content

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

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

ivailop7 merged 3 commits into
facebook:mainfrom
jvithlani:jv_reverse_selection

Conversation

@jvithlani

@jvithlani jvithlani commented Aug 4, 2025 •

Copy link
Copy Markdown
Contributor

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

vercel Bot commented Aug 4, 2025 •

Copy link
Copy Markdown

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

@etrepum etrepum left a comment

Copy link
Copy Markdown
Collaborator

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 ?

@etrepum etrepum left a comment

Copy link
Copy Markdown
Collaborator

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
@etrepum etrepum mentioned this pull request Aug 7, 2025

This branch was successfully deployed

2 active deployments
Preview – lexical-playground — 43c6b2f9 Deployed Aug 4, 2025 by vercel[bot]
Preview – lexical — 43c6b2f9 Deployed Aug 4, 2025 by vercel[bot]
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