Skip to content

Conversation

@Renzo-Olivares
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares commented Oct 31, 2023

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 IgnorePointer in the widget tree.

This change:

  • Removes the IgnorePointer so the underlying TextField can receive pointer events.
  • Introduces TextField.onTapAlwaysCalled and TextSelectionGestureDetector.onUserTapAlwaysCalled, so a user provided on tap callback can be called on consecutive taps. This is so that the user provided on tap callback for SearchAnchor/SearchBar that was previously only handled by InkWell will still work if a tap occurs in the TextFields hit box. The TextFields default behavior is maintained outside of the context of SearchAnchor/SearchBar.

Fixes #128332 and #134965

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: text input Entering text in a text field or keyboard related problems labels Oct 31, 2023
@Renzo-Olivares Renzo-Olivares force-pushed the fix-search-text-selection branch from 16b47c0 to d54ad44 Compare November 6, 2023 17:17
@Renzo-Olivares Renzo-Olivares marked this pull request as ready for review November 6, 2023 18:30
@Renzo-Olivares Renzo-Olivares changed the title Fix text selection in SearchAnchor Fix text selection in SearchAnchor/SearchBar Nov 6, 2023
Copy link
Contributor

@QuncCccccc QuncCccccc left a 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:)

Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Contributor Author

@Renzo-Olivares Renzo-Olivares Nov 8, 2023

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).

Copy link
Contributor

@QuncCccccc QuncCccccc left a 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😲!

@Renzo-Olivares Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 8, 2023
@Renzo-Olivares Renzo-Olivares force-pushed the fix-search-text-selection branch from a235b09 to 399ccfb Compare November 9, 2023 21:00
@Renzo-Olivares Renzo-Olivares added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Nov 9, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 11, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 11, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix text selection in search bar

2 participants