[lexical] Fix: adjust text selection by node position before applying block-level formats #7677
Conversation
salihudickson
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz,
ivailop7,
Sahejkm,
potatowagon,
lilshady and
takuyakanbr
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Hi @salihudickson! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
A simpler and more efficient approach here would be to have a function to normalize a RangeSelection such that the points are moved closer to each other so long as it can be done in a "zero distance" manner. It's not necessary to know the absolute positions, only the relative positions, and even then you don't have to know how much is between them only which direction they are from each other. Either way, the most important part of this work would be to write a few tests to show that it works as expected in all of the scenarios it's supposed to cover. The actual change itself is less important than the tests that define the behavior and prevent it from regressing in the future. |
|
IIRC the normalization that currently happens already has a right-bias so mostly it's the focus point that would need to move (assuming the selection is already normalized to be forwards such that the focus is not before the anchor). |
facebook-github-bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
@etrepum, yeah that makes sense. Thanks for the suggestion. And I'll be sure to add a bunch of test cases. |
salihudickson
changed the title
|
hey @etrepum, could you take another look at this PR? |
|
It looks like the tests are not passing, I will take a look after the tests pass. When updating this PR with main you may have to make some additional changes because the HTML output has changed after #7727 (the ltr/rtl classes are gone and the dir attribute may be set differently or not at all) |
salihudickson
force-pushed
the
fix/block-text-formatting
branch
from
d2c866e to
4897b6c
Compare
Description
Current Behavior
When you triple-click to select an entire line in the editor, the start of the next line is sometimes unintentionally included in the selection. This happens due to the browser’s default selection behavior and is not visibly noticeable to the user. However, it becomes apparent when applying block-level formats — since these styles affect the entire line, the formatting gets applied to both the intended line and part of the next one. This issue is most common in Chromium-based browsers.
How This PR Fixes It.
https://github.com/user-attachments/assets/2b59e59c-155b-4f79-8896-d7e88ce9cb12
How this PR addresses this
This PR addresses the issue by normalizing the selection before applying block-level formats. It calculates the absolute numeric position of the selection’s end, then filters out any nodes whose start position is greater than or equal to that endpoint. This ensures that only the intended line receives the formatting.
https://github.com/user-attachments/assets/832a1350-622b-4418-be18-29fbceb67455