Merge pull request #1911 from Hixie/so-long-show-press

Replace ShowPress with TapDown

packages/flutter/lib/gestures.dart

@@ -14,6 +14,5 @@ export 'src/gestures/lsq_solver.dart';
 export 'src/gestures/pointer_router.dart';
 export 'src/gestures/recognizer.dart';
 export 'src/gestures/scale.dart';
-export 'src/gestures/show_press.dart';
 export 'src/gestures/tap.dart';
 export 'src/gestures/velocity_tracker.dart';

packages/flutter/lib/src/gestures/constants.dart

@@ -5,8 +5,8 @@
 // Modeled after Android's ViewConfiguration:
 // https://github.com/android/platform_frameworks_base/blob/master/core/java/android/view/ViewConfiguration.java
 
-const Duration kLongPressTimeout = const Duration(milliseconds: 500);
 const Duration kPressTimeout = const Duration(milliseconds: 100);
+const Duration kLongPressTimeout = const Duration(milliseconds: 500);
 const Duration kJumpTapTimeout = const Duration(milliseconds: 500);
 const Duration kDoubleTapTimeout = const Duration(milliseconds: 300);
 const Duration kDoubleTapMinTime = const Duration(milliseconds: 40);

packages/flutter/lib/src/gestures/show_press.dart

-// Copyright 2015 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-import 'arena.dart';
-import 'constants.dart';
-import 'events.dart';
-import 'recognizer.dart';
-
-typedef void GestureShowPressCallback();
-
-class ShowPressGestureRecognizer extends PrimaryPointerGestureRecognizer {
-  ShowPressGestureRecognizer({ PointerRouter router, this.onShowPress })
-    : super(router: router, deadline: kPressTimeout);
-
-  GestureShowPressCallback onShowPress;
-
-  void didExceedDeadline() {
-    // Show press isn't an exclusive gesture. We can recognize a show press
-    // as well as another gesture, like a long press.
-    resolve(GestureDisposition.rejected);
-    onShowPress();
-  }
-
-  void handlePrimaryPointer(PointerInputEvent event) {
-    if (event.type == 'pointerup')
-      resolve(GestureDisposition.rejected);
-  }
-}

packages/flutter/lib/src/gestures/tap.dart

@@ -26,31 +26,34 @@ class TapGestureRecognizer extends PrimaryPointerGestureRecognizer {
     this.onTapUp,
     this.onTap,
     this.onTapCancel
-  }) : super(router: router);
+  }) : super(router: router, deadline: kPressTimeout);
 
   GestureTapDownCallback onTapDown;
   GestureTapDownCallback onTapUp;
   GestureTapCallback onTap;
   GestureTapCancelCallback onTapCancel;
 
+  bool _sentTapDown = false;
   bool _wonArena = false;
   Point _finalPosition;
 
   void handlePrimaryPointer(PointerInputEvent event) {
-    if (event.type == 'pointerdown') {
-      if (onTapDown != null)
-        onTapDown(event.position);
-    } else if (event.type == 'pointerup') {
+    if (event.type == 'pointerup') {
       _finalPosition = event.position;
-      _check();
+      _checkUp();
+    }
   }
+
+  void didExceedDeadline() {
+    _checkDown();
   }
 
   void acceptGesture(int pointer) {
     super.acceptGesture(pointer);
     if (pointer == primaryPointer) {
+      _checkDown();
       _wonArena = true;
-      _check();
+      _checkUp();
     }
   }
 
@@ -64,7 +67,15 @@ class TapGestureRecognizer extends PrimaryPointerGestureRecognizer {
     }
   }
 
-  void _check() {
+  void _checkDown() {
+    if (!_sentTapDown) {
+      if (onTapDown != null)
+        onTapDown(initialPosition);
+      _sentTapDown = true;
+    }
+  }
+
+  void _checkUp() {
     if (_wonArena && _finalPosition != null) {
       resolve(GestureDisposition.accepted);
       if (onTapUp != null)
@@ -76,6 +87,7 @@ class TapGestureRecognizer extends PrimaryPointerGestureRecognizer {
   }
 
   void _reset() {
+    _sentTapDown = false;
     _wonArena = false;
     _finalPosition = null;
   }

packages/flutter/lib/src/material/ink_well.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:math' as math;
 
 import 'package:flutter/animation.dart';
@@ -88,13 +87,8 @@ class _InkSplash {
         curve: Curves.easeOut
       ),
       duration: new Duration(milliseconds: (_targetRadius / _kSplashUnconfirmedVelocity).floor())
-    )..addListener(_handleRadiusChange);
-
-    // Wait kPressTimeout to avoid creating tiny splashes during scrolls.
-    // TODO(ianh): Instead of a timer in _InkSplash, we should start splashes from the gesture recognisers' onTapDown.
-    // ...and onTapDown should use a timer _or_ fire as soon as the tap is committed.
-    // When we do this, make sure it works even if we're only listening to onLongPress.
-    _startTimer = new Timer(kPressTimeout, _play);
+    )..addListener(_handleRadiusChange)
+     ..play();
   }
 
   final Point position;
@@ -103,45 +97,26 @@ class _InkSplash {
   double _targetRadius;
   double _pinnedRadius;
   ValuePerformance<double> _radius;
-  Timer _startTimer;
-
-  bool _cancelStartTimer() {
-    if (_startTimer != null) {
-      _startTimer.cancel();
-      _startTimer = null;
-      return true;
-    }
-    return false;
-  }
-
-  void _play() {
-    _cancelStartTimer();
-    _radius.play();
-  }
 
   void _updateVelocity(double velocity) {
     int duration = (_targetRadius / velocity).floor();
     _radius.duration = new Duration(milliseconds: duration);
-    _play();
+    _radius.play();
   }
 
   void confirm() {
-    if (_cancelStartTimer())
-      return;
     _updateVelocity(_kSplashConfirmedVelocity);
     _pinnedRadius = null;
   }
 
   void cancel() {
-    if (_cancelStartTimer())
-      return;
     _updateVelocity(_kSplashCanceledVelocity);
     _pinnedRadius = _radius.value;
   }
 
   void _handleRadiusChange() {
     if (_radius.value == _targetRadius)
-      renderer._splashes.remove(this);
+      renderer._removeSplash(this);
     renderer.markNeedsPaint();
   }
 
@@ -183,15 +158,21 @@ class _RenderInkSplashes extends RenderProxyBox {
   _HighlightChangedCallback onHighlightChanged;
 
   final List<_InkSplash> _splashes = new List<_InkSplash>();
+  _InkSplash _lastSplash;
 
   TapGestureRecognizer _tap;
   LongPressGestureRecognizer _longPress;
 
+  void _removeSplash(_InkSplash splash) {
+    _splashes.remove(splash);
+    if (_lastSplash == splash)
+      _lastSplash = null;
+  }
+
   void handleEvent(InputEvent event, BoxHitTestEntry entry) {
-    if (event.type == 'pointerdown' && (_tap != null || _longPress != null)) {
+    if (event.type == 'pointerdown' && (onTap != null || onLongPress != null)) {
       _tap?.addPointer(event);
       _longPress?.addPointer(event);
-      _splashes.add(new _InkSplash(entry.localPosition, this));
     }
   }
 
@@ -208,7 +189,7 @@ class _RenderInkSplashes extends RenderProxyBox {
   }
 
   void _syncTapRecognizer() {
-    if (onTap == null) {
+    if (onTap == null && onLongPress == null) {
       _disposeTapRecognizer();
     } else {
       _tap ??= new TapGestureRecognizer(router: FlutterBinding.instance.pointerRouter)
@@ -237,30 +218,33 @@ class _RenderInkSplashes extends RenderProxyBox {
     _longPress = null;
   }
 
-  void _handleTapDown(_) {
+  void _handleTapDown(Point position) {
+    _lastSplash = new _InkSplash(globalToLocal(position), this);
+    _splashes.add(_lastSplash);
     if (onHighlightChanged != null)
       onHighlightChanged(true);
   }
 
   void _handleTap() {
-    if (_splashes.isNotEmpty)
-      _splashes.last.confirm();
-
+    _lastSplash?.confirm();
+    _lastSplash = null;
     if (onHighlightChanged != null)
       onHighlightChanged(false);
-
     if (onTap != null)
       onTap();
   }
 
   void _handleTapCancel() {
-    _splashes.last?.cancel();
+    _lastSplash?.cancel();
+    _lastSplash = null;
     if (onHighlightChanged != null)
       onHighlightChanged(false);
   }
 
   void _handleLongPress() {
-    _splashes.last?.confirm();
+    _lastSplash?.confirm();
+    _lastSplash = null;
+    if (onLongPress != null)
       onLongPress();
   }
 

packages/flutter/lib/src/widgets/gesture_detector.dart

@@ -13,7 +13,6 @@ export 'package:flutter/gestures.dart' show
   GestureTapUpCallback,
   GestureTapCallback,
   GestureTapCancelCallback,
-  GestureShowPressCallback,
   GestureLongPressCallback,
   GestureDragStartCallback,
   GestureDragUpdateCallback,
@@ -37,7 +36,6 @@ class GestureDetector extends StatefulComponent {
     this.onTap,
     this.onTapCancel,
     this.onDoubleTap,
-    this.onShowPress,
     this.onLongPress,
     this.onVerticalDragStart,
     this.onVerticalDragUpdate,
@@ -61,7 +59,6 @@ class GestureDetector extends StatefulComponent {
   final GestureTapCancelCallback onTapCancel;
   final GestureTapCallback onDoubleTap;
 
-  final GestureShowPressCallback onShowPress;
   final GestureLongPressCallback onLongPress;
 
   final GestureDragStartCallback onVerticalDragStart;
@@ -88,7 +85,6 @@ class _GestureDetectorState extends State<GestureDetector> {
 
   TapGestureRecognizer _tap;
   DoubleTapGestureRecognizer _doubleTap;
-  ShowPressGestureRecognizer _showPress;
   LongPressGestureRecognizer _longPress;
   VerticalDragGestureRecognizer _verticalDrag;
   HorizontalDragGestureRecognizer _horizontalDrag;
@@ -107,7 +103,6 @@ class _GestureDetectorState extends State<GestureDetector> {
   void dispose() {
     _tap = _ensureDisposed(_tap);
     _doubleTap = _ensureDisposed(_doubleTap);
-    _showPress = _ensureDisposed(_showPress);
     _longPress = _ensureDisposed(_longPress);
     _verticalDrag = _ensureDisposed(_verticalDrag);
     _horizontalDrag = _ensureDisposed(_horizontalDrag);
@@ -119,7 +114,6 @@ class _GestureDetectorState extends State<GestureDetector> {
   void _syncAll() {
     _syncTap();
     _syncDoubleTap();
-    _syncShowPress();
     _syncLongPress();
     _syncVerticalDrag();
     _syncHorizontalDrag();
@@ -149,15 +143,6 @@ class _GestureDetectorState extends State<GestureDetector> {
     }
   }
 
-  void _syncShowPress() {
-    if (config.onShowPress == null) {
-      _showPress = _ensureDisposed(_showPress);
-    } else {
-      _showPress ??= new ShowPressGestureRecognizer(router: _router);
-      _showPress.onShowPress = config.onShowPress;
-    }
-  }
-
   void _syncLongPress() {
     if (config.onLongPress == null) {
       _longPress = _ensureDisposed(_longPress);
@@ -227,8 +212,6 @@ class _GestureDetectorState extends State<GestureDetector> {
       _tap.addPointer(event);
     if (_doubleTap != null)
       _doubleTap.addPointer(event);
-    if (_showPress != null)
-      _showPress.addPointer(event);
     if (_longPress != null)
       _longPress.addPointer(event);
     if (_verticalDrag != null)
@@ -255,8 +238,6 @@ class _GestureDetectorState extends State<GestureDetector> {
       gestures.add('tap');
     if (_doubleTap != null)
       gestures.add('double tap');
-    if (_showPress != null)
-      gestures.add('show press');
     if (_longPress != null)
       gestures.add('long press');
     if (_verticalDrag != null)

packages/unit/test/gestures/long_press_test.dart

@@ -67,14 +67,14 @@ void main() {
     longPress.dispose();
   });
 
-  test('Should recognize both show press and long press', () {
+  test('Should recognize both tap down and long press', () {
     PointerRouter router = new PointerRouter();
-    ShowPressGestureRecognizer showPress = new ShowPressGestureRecognizer(router: router);
+    TapGestureRecognizer tap = new TapGestureRecognizer(router: router);
     LongPressGestureRecognizer longPress = new LongPressGestureRecognizer(router: router);
 
-    bool showPressRecognized = false;
-    showPress.onShowPress = () {
-      showPressRecognized = true;
+    bool tapDownRecognized = false;
+    tap.onTapDown = (_) {
+      tapDownRecognized = true;
     };
 
     bool longPressRecognized = false;
@@ -83,23 +83,23 @@ void main() {
     };
 
     new FakeAsync().run((FakeAsync async) {
-      showPress.addPointer(down);
+      tap.addPointer(down);
       longPress.addPointer(down);
       GestureArena.instance.close(5);
-      expect(showPressRecognized, isFalse);
+      expect(tapDownRecognized, isFalse);
       expect(longPressRecognized, isFalse);
       router.route(down);
-      expect(showPressRecognized, isFalse);
+      expect(tapDownRecognized, isFalse);
       expect(longPressRecognized, isFalse);
       async.elapse(new Duration(milliseconds: 300));
-      expect(showPressRecognized, isTrue);
+      expect(tapDownRecognized, isTrue);
       expect(longPressRecognized, isFalse);
       async.elapse(new Duration(milliseconds: 700));
-      expect(showPressRecognized, isTrue);
+      expect(tapDownRecognized, isTrue);
       expect(longPressRecognized, isTrue);
     });
 
-    showPress.dispose();
+    tap.dispose();
     longPress.dispose();
   });
 }

packages/unit/test/gestures/show_press_test.dart

-import 'package:quiver/testing/async.dart';
-import 'package:flutter/gestures.dart';
-import 'package:test/test.dart';
-
-final PointerInputEvent down = new PointerInputEvent(
-  pointer: 5,
-  type: 'pointerdown',
-  x: 10.0,
-  y: 10.0
-);
-
-final PointerInputEvent up = new PointerInputEvent(
-  pointer: 5,
-  type: 'pointerup',
-  x: 11.0,
-  y: 9.0
-);
-
-void main() {
-  test('Should recognize show press', () {
-    PointerRouter router = new PointerRouter();
-    ShowPressGestureRecognizer showPress = new ShowPressGestureRecognizer(router: router);
-
-    bool showPressRecognized = false;
-    showPress.onShowPress = () {
-      showPressRecognized = true;
-    };
-
-    new FakeAsync().run((FakeAsync async) {
-      showPress.addPointer(down);
-      GestureArena.instance.close(5);
-      expect(showPressRecognized, isFalse);
-      router.route(down);
-      expect(showPressRecognized, isFalse);
-      async.elapse(new Duration(milliseconds: 300));
-      expect(showPressRecognized, isTrue);
-    });
-
-    showPress.dispose();
-  });
-
-  test('Up cancels show press', () {
-    PointerRouter router = new PointerRouter();
-    ShowPressGestureRecognizer showPress = new ShowPressGestureRecognizer(router: router);
-
-    bool showPressRecognized = false;
-    showPress.onShowPress = () {
-      showPressRecognized = true;
-    };
-
-    new FakeAsync().run((FakeAsync async) {
-      showPress.addPointer(down);
-      GestureArena.instance.close(5);
-      expect(showPressRecognized, isFalse);
-      router.route(down);
-      expect(showPressRecognized, isFalse);
-      async.elapse(new Duration(milliseconds: 50));
-      expect(showPressRecognized, isFalse);
-      router.route(up);
-      expect(showPressRecognized, isFalse);
-      async.elapse(new Duration(seconds: 1));
-      expect(showPressRecognized, isFalse);
-    });
-
-    showPress.dispose();
-  });
-}