-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix text selection in SearchAnchor/SearchBar
#137636
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 text selection in SearchAnchor/SearchBar
#137636
Conversation
16b47c0 to
d54ad44
Compare
SearchAnchorSearchAnchor/SearchBar
QuncCccccc
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.
This looks good to me! Just left some questions:)
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.
Can we just do onTap: widget.onTap?
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.
Should we just remove _focusNode and the dispose part because I think it's unused?
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.
Seems _state.widget.onTap?.call(); is removed from onSingleTapUp. Just wonder if this is intended?
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.
This is intended, it has been moved to onUserTap so there can be some differentiation between the framework onTap callback and a user provided onTap callback. This is a non-breaking change because onUserTap will still be called with onSingleTapUp in TextSelectionGestureDetector as it was before this change. Extenders of TextSelectionGestureDetectorBuilder will be okay because TextSelectionGestureDetector retains its previous behavior as the default (A user can still choose to put _state.widget.onTap?.call() in onSingleTapUp and it will work as expected).
QuncCccccc
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! Thanks so much for this fix and all the unit tests😲!
…g the focus change as an internal change instead of a focus change as a result of a tab
a235b09 to
399ccfb
Compare
This changes fixes text selection gestures on the search field when using
SearchAnchor.Before this change text selection gestures did not work due to an
IgnorePointerin the widget tree.This change:
IgnorePointerso the underlyingTextFieldcan receive pointer events.TextField.onTapAlwaysCalledandTextSelectionGestureDetector.onUserTapAlwaysCalled, so a user provided on tap callback can be called on consecutive taps. This is so that the user provided on tap callback forSearchAnchor/SearchBarthat was previously only handled byInkWellwill still work if a tap occurs in theTextFields hit box. TheTextFields default behavior is maintained outside of the context ofSearchAnchor/SearchBar.Fixes #128332 and #134965
Pre-launch Checklist
///).