Skip to content

_TapStatusTrackerMixin should wait until the next PointerDownEvent before resetting its state when the tap timer has elapsed #129312

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
Merged
auto-submit merged 10 commits into from
Jun 23, 2023
Merged

_TapStatusTrackerMixin should wait until the next PointerDownEvent before resetting its state when the tap timer has elapsed #129312

auto-submit merged 10 commits into from
Jun 23, 2023

Conversation

@Renzo-Olivares
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares commented Jun 21, 2023 •
edited
Loading

_TapStatusTrackerMixin used by BaseTapAndDragGestureRecognizer should wait until the next tap down before resetting its state when the _consecutiveTapTimer times out. This is because BaseTapAndDragGestureRecognizer may not have fired its tap down/tap up event before the state has been reset preventing it from firing the tap down/tap up callbacks at all because currentDown and currentUp are reset to null.

Fixes #129161

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 f: gestures flutter/packages/flutter/gestures repository. framework flutter/packages/flutter repository. See also f: labels. labels Jun 21, 2023
@Renzo-Olivares Renzo-Olivares changed the title BaseTapAndDragGestureRecognizer should track its down/up events locally BaseTapAndDragGestureRecognizer should track its down/up events locally Jun 21, 2023
@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. labels Jun 21, 2023
@Renzo-Olivares Renzo-Olivares force-pushed the textfield-regression-fix branch from b4be58d to 76ea108 Compare June 22, 2023 17:17
@Renzo-Olivares Renzo-Olivares changed the title BaseTapAndDragGestureRecognizer should track its down/up events locally _TapStatusTrackerMixin should wait until the next PointerDownEvent before resetting its state when timer has elapsed Jun 22, 2023
@Renzo-Olivares Renzo-Olivares changed the title _TapStatusTrackerMixin should wait until the next PointerDownEvent before resetting its state when timer has elapsed _TapStatusTrackerMixin should wait until the next PointerDownEvent before resetting its state when the tap timer has elapsed Jun 22, 2023
@Renzo-Olivares Renzo-Olivares force-pushed the textfield-regression-fix branch from ca1a429 to 0819f0f Compare June 22, 2023 19:36
@Renzo-Olivares Renzo-Olivares marked this pull request as ready for review June 22, 2023 19:36
@Renzo-Olivares Renzo-Olivares requested a review from justinmc June 22, 2023 19:36
justinmc
justinmc approved these changes Jun 22, 2023
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Maybe you can get rid of the boolean you added though, I left a comment.

packages/flutter/lib/src/widgets/tap_and_drag_gestures.dart Outdated
Copy link
Contributor

@justinmc justinmc Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you look at the timer instead to get rid of the boolean? Something like this:

if (_consecutiveTapTimer != null && !_consecutiveTapTimer.isActive) {

Copy link
Contributor Author

@Renzo-Olivares Renzo-Olivares Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works!

packages/flutter/test/material/text_field_test.dart Outdated
Copy link
Contributor

@justinmc justinmc Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a constant to use here instead of 300? kDoubleTapTimeout?

@Renzo-Olivares Renzo-Olivares force-pushed the textfield-regression-fix branch from 7b4a158 to 8808b55 Compare June 22, 2023 22:40
@Renzo-Olivares Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 22, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jun 22, 2023

auto label is removed for flutter/flutter, pr: 129312, due to - The status or check suite Linux web_canvaskit_tests_3 has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 22, 2023
@Renzo-Olivares Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 22, 2023
@auto-submit auto-submit bot merged commit 3c366b7 into flutter:master Jun 23, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 23, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 23, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
74fa2d3 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 23, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 23, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
9542222 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 24, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
7641d3c 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 24, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
c845e52 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 24, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
1e6292f 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 25, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
a57c8f2 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 25, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
75f1da5 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 25, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
bc79d96 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 26, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
7b9452a 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 26, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
5cf6988 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 26, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
14f26e2 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
112992a 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
70bdbda 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
Renzo-Olivares added a commit to Renzo-Olivares/flutter that referenced this pull request Jun 26, 2023
@Renzo-Olivares
_TapStatusTrackerMixin should wait until the next `PointerDownEvent…
164eb2e 
…` before resetting its state when the tap timer has elapsed (flutter#129312)

`_TapStatusTrackerMixin` used by `BaseTapAndDragGestureRecognizer` should wait until the next tap down before resetting its state when the `_consecutiveTapTimer` times out. This is because `BaseTapAndDragGestureRecognizer` may not have fired its tap down/tap up event before the state has been reset preventing it from firing the tap down/tap up callbacks at all because `currentDown` and `currentUp` are reset to `null`.

Fixes flutter#129161
@Renzo-Olivares Renzo-Olivares mentioned this pull request Jun 26, 2023
Merged
8 tasks
@HansMuller HansMuller mentioned this pull request Jun 30, 2023
Closed
auto-submit bot pushed a commit that referenced this pull request Jul 10, 2023
@Renzo-Olivares
[CP] Fix TextField not responding to taps when wrapped in a GestureDe…
9a3a31d 
…tector (#129576)

Cherry pick for issue #129161 / PR #129312

CP Issue: #129577
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
7da5ff5 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
a029ddb 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
3bc8b14 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
3c366b7 _TapStatusTrackerMixin should wait until the next `PointerD…
7e74029 
…ownEvent` before resetting its state when the tap timer has elapsed (flutter/flutter#129312)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinmc justinmc justinmc approved these changes

Assignees

No one assigned

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: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TextField that embedded in the GestureDetector can't be selected

2 participants

@Renzo-Olivares @justinmc