-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix selection menu not showing after clear #37042
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
Conversation
| } | ||
| } | ||
|
|
||
| void _hideSelectionOverlayIfNeeded() { |
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.
I think some of the problem came from the fact that there are two subtly different methods with confusing names: _hideSelectionOverlayIfNeeded and hideToolbar. I got rid of this one altogether and I think it makes more sense now.
HansMuller
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
| }); | ||
|
|
||
| tearDown(() { | ||
| controller = null; |
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.
controller.dispose() first?
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.
Good catch.
| await tester.pump(); | ||
| expect(find.text('PASTE'), findsOneWidget); | ||
|
|
||
| // Hide the toolbar and clear the text and selection. |
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.
Maybe factor this out as a separate test and mark it as a regression test for #35998
Description
If you long press an empty text field, then the selection menu shows, but if you type some content and then delete it all so it's empty again, the menu doesn't show. This PR fixes that.
Related Issues
Closes #35998
Tests
Tested adding text, deleting it, and then showing the menu in editable_text_test.dart.