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

Fix #5738 wrong selection on mouse click #5969

Merged
merged 3 commits into from
Apr 29, 2024
Merged

Fix #5738 wrong selection on mouse click #5969

merged 3 commits into from
Apr 29, 2024

Conversation

Sahejkm
Copy link
Contributor

@Sahejkm Sahejkm commented Apr 26, 2024

  • The Crash looks partially resolved before this fix - it doesn't crash anymore.
  • But the wrong selection leads to wrong behaviour on backspace command and doesn't delete hr correctly if we try delete from the wrong selection state

Fix: Fix the wrong selection Resolved state due to Block Cursor element

closes #5738

Before ( delete command doesn't delete hr element)

Screen.Recording.2024-04-26.at.7.22.30.PM.mov

After ( delete command deletes hr element correctly )

Screen.Recording.2024-04-26.at.10.25.28.PM.mov

@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 Apr 26, 2024
Copy link

vercel bot commented Apr 26, 2024

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 Apr 29, 2024 4:44am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 29, 2024 4:44am

Copy link

github-actions bot commented Apr 26, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/lexical/dist/Lexical.js 23.69 KB (+0.11% 🔺) 474 ms (+0.11% 🔺) 182 ms (+271.11% 🔺) 656 ms
packages/lexical-rich-text/dist/LexicalRichText.js 33.91 KB (+0.11% 🔺) 679 ms (+0.11% 🔺) 162 ms (-13.12% 🔽) 840 ms
packages/lexical-plain-text/dist/LexicalPlainText.js 33.91 KB (+0.16% 🔺) 679 ms (+0.16% 🔺) 184 ms (+10.39% 🔺) 863 ms

zurfyx
zurfyx previously approved these changes Apr 26, 2024
packages/lexical/src/LexicalSelection.ts Outdated Show resolved Hide resolved
@zurfyx
Copy link
Member

zurfyx commented Apr 26, 2024

LGTM! You may want to update the description and add some test if possible

Co-authored-by: Gerard Rovira <zurfyx@users.noreply.github.com>
Copy link
Contributor

@potatowagon potatowagon left a comment

Choose a reason for hiding this comment

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

lgtm!

@Sahejkm
Copy link
Contributor Author

Sahejkm commented Apr 29, 2024

Lemme add the tests in a separate PR since this is already approved.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Horizontal Rule node or Page break node selection problem
4 participants