From f2fd5e32a5c2befdb3bd7307eec5bd6bbd8d50e9 Mon Sep 17 00:00:00 2001 From: Kris Giesing Date: Mon, 26 Oct 2015 13:41:29 -0700 Subject: [PATCH] Restore previous tap behaviors: no timeout, one pointer Don't use a timeout to cancel tap tracking. Track only one primary pointer and ignore non-primary pointers. Update tests to reflect desired behaviors. Fixes #1779, #1780, #1781. --- .../flutter/lib/src/gestures/constants.dart | 2 +- .../flutter/lib/src/gestures/double_tap.dart | 2 - .../flutter/lib/src/gestures/long_press.dart | 2 +- .../flutter/lib/src/gestures/show_press.dart | 2 +- packages/flutter/lib/src/gestures/tap.dart | 84 ++++++++++++++----- .../flutter/lib/src/material/ink_well.dart | 4 +- .../unit/test/gestures/double_tap_test.dart | 6 +- packages/unit/test/gestures/tap_test.dart | 12 +-- 8 files changed, 76 insertions(+), 38 deletions(-) diff --git a/packages/flutter/lib/src/gestures/constants.dart b/packages/flutter/lib/src/gestures/constants.dart index 804a0fb1e52..fd8fa38c222 100644 --- a/packages/flutter/lib/src/gestures/constants.dart +++ b/packages/flutter/lib/src/gestures/constants.dart @@ -6,7 +6,7 @@ // https://github.com/android/platform_frameworks_base/blob/master/core/java/android/view/ViewConfiguration.java const Duration kLongPressTimeout = const Duration(milliseconds: 500); -const Duration kTapTimeout = const Duration(milliseconds: 100); +const Duration kPressTimeout = const Duration(milliseconds: 100); const Duration kJumpTapTimeout = const Duration(milliseconds: 500); const Duration kDoubleTapTimeout = const Duration(milliseconds: 300); const Duration kDoubleTapMinTime = const Duration(milliseconds: 40); diff --git a/packages/flutter/lib/src/gestures/double_tap.dart b/packages/flutter/lib/src/gestures/double_tap.dart index 6b5d4c8fe88..1ecaf680689 100644 --- a/packages/flutter/lib/src/gestures/double_tap.dart +++ b/packages/flutter/lib/src/gestures/double_tap.dart @@ -52,7 +52,6 @@ class DoubleTapGestureRecognizer extends DisposableArenaMember { entry: GestureArena.instance.add(event.pointer, this) ); _trackers[event.pointer] = tracker; - tracker.startTimer(() => _reject(tracker)); tracker.startTrackingPointer(router, handleEvent); } @@ -145,7 +144,6 @@ class DoubleTapGestureRecognizer extends DisposableArenaMember { } void _freezeTracker(TapTracker tracker) { - tracker.stopTimer(); tracker.stopTrackingPointer(router, handleEvent); } diff --git a/packages/flutter/lib/src/gestures/long_press.dart b/packages/flutter/lib/src/gestures/long_press.dart index 2db0483772d..07b16e69701 100644 --- a/packages/flutter/lib/src/gestures/long_press.dart +++ b/packages/flutter/lib/src/gestures/long_press.dart @@ -12,7 +12,7 @@ typedef void GestureLongPressCallback(); class LongPressGestureRecognizer extends PrimaryPointerGestureRecognizer { LongPressGestureRecognizer({ PointerRouter router, this.onLongPress }) - : super(router: router, deadline: kTapTimeout + kLongPressTimeout); + : super(router: router, deadline: kLongPressTimeout); GestureLongPressCallback onLongPress; diff --git a/packages/flutter/lib/src/gestures/show_press.dart b/packages/flutter/lib/src/gestures/show_press.dart index 18ee1ca5953..1ecc4f7622c 100644 --- a/packages/flutter/lib/src/gestures/show_press.dart +++ b/packages/flutter/lib/src/gestures/show_press.dart @@ -11,7 +11,7 @@ typedef void GestureShowPressCallback(); class ShowPressGestureRecognizer extends PrimaryPointerGestureRecognizer { ShowPressGestureRecognizer({ PointerRouter router, this.onShowPress }) - : super(router: router, deadline: kTapTimeout); + : super(router: router, deadline: kPressTimeout); GestureShowPressCallback onShowPress; diff --git a/packages/flutter/lib/src/gestures/tap.dart b/packages/flutter/lib/src/gestures/tap.dart index a0b1ee56966..1ca09677617 100644 --- a/packages/flutter/lib/src/gestures/tap.dart +++ b/packages/flutter/lib/src/gestures/tap.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; import 'dart:ui' as ui; import 'arena.dart'; @@ -13,9 +12,56 @@ import 'recognizer.dart'; typedef void GestureTapCallback(); -enum TapResolution { - tap, - cancel +/// TapGestureRecognizer is a tap recognizer that tracks only one primary +/// pointer per gesture. That is, during tap recognition, extra pointer events +/// are ignored: down-1, down-2, up-1, up-2 produces only one tap on up-1. +class TapGestureRecognizer extends PrimaryPointerGestureRecognizer { + TapGestureRecognizer({ PointerRouter router, this.onTap }) + : super(router: router); + + GestureTapCallback onTap; + GestureTapCallback onTapDown; + GestureTapCallback onTapCancel; + + bool _wonArena = false; + bool _didTap = false; + + void handlePrimaryPointer(PointerInputEvent event) { + if (event.type == 'pointerdown') { + if (onTapDown != null) + onTapDown(); + } else if (event.type == 'pointerup') { + _didTap = true; + _check(); + } + } + + void acceptGesture(int pointer) { + super.acceptGesture(pointer); + if (pointer == primaryPointer) { + _wonArena = true; + _check(); + } + } + + void rejectGesture(int pointer) { + super.rejectGesture(pointer); + if (pointer == primaryPointer) { + assert(state == GestureRecognizerState.defunct); + _wonArena = false; + _didTap = false; + if (onTapCancel != null) + onTapCancel(); + } + } + + void _check() { + if (_wonArena && _didTap) { + resolve(GestureDisposition.accepted); + if (onTap != null) + onTap(); + } + } } /// TapTracker helps track individual tap sequences as part of a @@ -33,18 +79,6 @@ class TapTracker { GestureArenaEntry entry; ui.Point _initialPosition; bool _isTrackingPointer; - Timer _timer; - - void startTimer(void callback()) { - _timer ??= new Timer(kTapTimeout, callback); - } - - void stopTimer() { - if (_timer != null) { - _timer.cancel(); - _timer = null; - } - } void startTrackingPointer(PointerRouter router, PointerRoute route) { if (!_isTrackingPointer) { @@ -67,6 +101,11 @@ class TapTracker { } +enum TapResolution { + tap, + cancel +} + /// TapGesture represents a full gesture resulting from a single tap /// sequence. Tap gestures are passive, meaning that they will not /// pre-empt any other arena member in play. @@ -77,11 +116,10 @@ class TapGesture extends TapTracker { entry = GestureArena.instance.add(event.pointer, gestureRecognizer); _wonArena = false; _didTap = false; - startTimer(cancel); startTrackingPointer(gestureRecognizer.router, handleEvent); } - TapGestureRecognizer gestureRecognizer; + MultiTapGestureRecognizer gestureRecognizer; bool _wonArena; bool _didTap; @@ -93,7 +131,6 @@ class TapGesture extends TapTracker { } else if (event.type == 'pointercancel') { cancel(); } else if (event.type == 'pointerup') { - stopTimer(); stopTrackingPointer(gestureRecognizer.router, handleEvent); _didTap = true; _check(); @@ -106,7 +143,6 @@ class TapGesture extends TapTracker { } void reject() { - stopTimer(); stopTrackingPointer(gestureRecognizer.router, handleEvent); gestureRecognizer._resolveTap(pointer, TapResolution.cancel); } @@ -127,8 +163,12 @@ class TapGesture extends TapTracker { } -class TapGestureRecognizer extends DisposableArenaMember { - TapGestureRecognizer({ this.router, this.onTap, this.onTapDown, this.onTapCancel }); +/// MultiTapGestureRecognizer is a tap recognizer that treats taps +/// independently. That is, each pointer sequence that could resolve to a tap +/// does so independently of others: down-1, down-2, up-1, up-2 produces two +/// taps, on up-1 and up-2. +class MultiTapGestureRecognizer extends DisposableArenaMember { + MultiTapGestureRecognizer({ this.router, this.onTap, this.onTapDown, this.onTapCancel }); PointerRouter router; GestureTapCallback onTap; diff --git a/packages/flutter/lib/src/material/ink_well.dart b/packages/flutter/lib/src/material/ink_well.dart index 0d5ca2d47b3..f0bb6514519 100644 --- a/packages/flutter/lib/src/material/ink_well.dart +++ b/packages/flutter/lib/src/material/ink_well.dart @@ -35,8 +35,8 @@ class _InkSplash { duration: new Duration(milliseconds: (_targetRadius / _kSplashUnconfirmedVelocity).floor()) )..addListener(_handleRadiusChange); - // Wait kTapTimeout to avoid creating tiny splashes during scrolls. - _startTimer = new Timer(kTapTimeout, _play); + // Wait kPressTimeout to avoid creating tiny splashes during scrolls. + _startTimer = new Timer(kPressTimeout, _play); } final Point position; diff --git a/packages/unit/test/gestures/double_tap_test.dart b/packages/unit/test/gestures/double_tap_test.dart index 934a4703e25..7d4e6c3c686 100644 --- a/packages/unit/test/gestures/double_tap_test.dart +++ b/packages/unit/test/gestures/double_tap_test.dart @@ -223,7 +223,7 @@ void main() { tap.dispose(); }); - test('Intra-tap delay cancels double tap', () { + test('Intra-tap delay does not cancel double tap', () { PointerRouter router = new PointerRouter(); DoubleTapGestureRecognizer tap = new DoubleTapGestureRecognizer(router: router); @@ -252,9 +252,9 @@ void main() { expect(doubleTapRecognized, isFalse); router.route(up2); - expect(doubleTapRecognized, isFalse); + expect(doubleTapRecognized, isTrue); GestureArena.instance.sweep(2); - expect(doubleTapRecognized, isFalse); + expect(doubleTapRecognized, isTrue); }); tap.dispose(); diff --git a/packages/unit/test/gestures/tap_test.dart b/packages/unit/test/gestures/tap_test.dart index 95062db93df..d1e5dd43396 100644 --- a/packages/unit/test/gestures/tap_test.dart +++ b/packages/unit/test/gestures/tap_test.dart @@ -84,7 +84,7 @@ void main() { tap.dispose(); }); - test('Should recognize two overlapping taps', () { + test('Should not recognize two overlapping taps', () { PointerRouter router = new PointerRouter(); TapGestureRecognizer tap = new TapGestureRecognizer(router: router); @@ -112,9 +112,9 @@ void main() { expect(tapsRecognized, 1); router.route(up2); - expect(tapsRecognized, 2); + expect(tapsRecognized, 1); GestureArena.instance.sweep(2); - expect(tapsRecognized, 2); + expect(tapsRecognized, 1); tap.dispose(); }); @@ -144,7 +144,7 @@ void main() { tap.dispose(); }); - test('Timeout cancels tap', () { + test('Timeout does not cancel tap', () { PointerRouter router = new PointerRouter(); TapGestureRecognizer tap = new TapGestureRecognizer(router: router); @@ -163,9 +163,9 @@ void main() { async.elapse(new Duration(milliseconds: 500)); expect(tapRecognized, isFalse); router.route(up1); - expect(tapRecognized, isFalse); + expect(tapRecognized, isTrue); GestureArena.instance.sweep(1); - expect(tapRecognized, isFalse); + expect(tapRecognized, isTrue); }); tap.dispose();