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.

Sign up for GitHub

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

Jump to bottom

fix get text selectRect error #4263

Merged
merged 3 commits into from Apr 4, 2023
Merged

fix get text selectRect error #4263

merged 3 commits into from Apr 4, 2023

Conversation

yjhtry
Copy link
Contributor

@yjhtry yjhtry commented Apr 4, 2023

getBoundingClientRectis not a function whenselectionTarget is of typeText, So just by judging selectionTarget! == null will not avoid this error

image

@vercel
Copy link

vercel bot commented Apr 4, 2023 •
edited

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 4, 2023 1:32pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 4, 2023 1:32pm

@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 4, 2023
@zurfyx
Copy link
Member

zurfyx commented Apr 4, 2023

Thank you! Can you please expand on your test plan and how you repro'ed the issue? I don't think I've come across this issue before when typing on Lexical

@yjhtry
Copy link
Contributor Author

yjhtry commented Apr 4, 2023

Thank you! Can you please expand on your test plan and how you repro'ed the issue? I don't think I've come across this issue before when typing on Lexical

Here's a recurring example https://codesandbox.io/p/sandbox/relaxed-river-h6oe24

I'm not sure it has anything to do with this setting
image

Here's the replay video
This problem occurs when I try to select 00:331 and This problem also occurs when the mouse is accidentally crossed

video.mp4

@yjhtry @zurfyx
Update packages/lexical/src/LexicalSelection.ts
fef2ee6 
fix Text scrollIntoView

Co-authored-by: Gerard Rovira <zurfyx@users.noreply.github.com>
@yjhtry
Update LexicalSelection.ts
cf8bff2 
fix range.select to range.selectNode
@zurfyx
Copy link
Member

zurfyx commented Apr 4, 2023

The closest we have (built-in) are token nodes but I don't think they're great for your use case either. In any case, it shouldn't crash. The code I provided is pseudocode, you will have to adjust it a bit

@yjhtry
Copy link
Contributor Author

yjhtry commented Apr 4, 2023

The closest we have (built-in) are token nodes but I don't think they're great for your use case either. In any case, it shouldn't crash. The code I provided is pseudocode, you will have to adjust it a bit

Yes, I've adjusted

@yjhtry yjhtry requested a review from zurfyx April 4, 2023 13:53
@zurfyx
Copy link
Member

zurfyx commented Apr 4, 2023

Thank you, LGTM!

@zurfyx zurfyx merged commit e27acc4 into facebook:main Apr 4, 2023
39 of 43 checks passed
@acywatson acywatson mentioned this pull request Apr 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zurfyx zurfyx zurfyx approved these changes

@fantactuka fantactuka Awaiting requested review from fantactuka fantactuka is a code owner

@acywatson acywatson Awaiting requested review from acywatson acywatson is a code owner

@tylerjbainbridge tylerjbainbridge Awaiting requested review from tylerjbainbridge

@thegreatercurve thegreatercurve Awaiting requested review from thegreatercurve

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yjhtry @zurfyx @facebook-github-bot