-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix cursor disappear on undo. #122402
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 cursor disappear on undo. #122402
Conversation
33895b2 to
a5756c3
Compare
justinmc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one idea to possibly get rid of _hasPendingChange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to get rid of _hasPendingChange and use _throttleTimer?.isActive instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great idea. I updated the PR and it works nicely, thanks!
a5756c3 to
1d5e3e3
Compare
justinmc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thank you for fixing this!
a68fbb0 to
87e43de
Compare
Description
This PR updates the undo logic to fix two problems:
Steps to Reproduce
Code sample (which creates a TextField with the non empty text, "Flutter").
Problem 1: Tap ‘3’ and undo before the throttling delay ends (500ms).
Before this fix: the text is ‘Flutter’ and the cursor disappeared. Tapping a new key will insert it before ‘Flutter’.
After this fix: the text should be ‘Flutter’ and the cursor at the end.
Problem 2: Tap ‘3’, wait for the throttling delay, tap ‘3’ again and undo before the throttling delay ends.
Before this fix: the text value is ‘Flutter’ (penultimate state).
After this fix: the text value is ‘Flutter3’.
Related Issue
Fixes #120194
Tests
Adds 2 tests.