A tap is not a scroll (#5038)

packages/flutter/lib/src/material/overscroll_indicator.dart

@@ -132,7 +132,6 @@ class _OverscrollIndicatorState extends State<OverscrollIndicator> {
   void _hide() {
     _hideTimer?.cancel();
     _hideTimer = null;
-    // Gaurding _hide() being called while indicator is already animating.
     if (!_extentAnimation.isAnimating) {
       _extentAnimation.reverse();
     }
@@ -188,21 +187,24 @@ class _OverscrollIndicatorState extends State<OverscrollIndicator> {
   }
 
   bool _handleScrollNotification(ScrollNotification notification) {
-    if (notification.depth != 0)
+    if (config.scrollableKey == null) {
+        if (notification.depth != 0)
+          return false;
+    } else if (config.scrollableKey != notification.scrollable.config.key) {
       return false;
-    if (config.scrollableKey == null || config.scrollableKey == notification.scrollable.config.key) {
-      final ScrollableState scrollable = notification.scrollable;
-      switch(notification.kind) {
-        case ScrollNotificationKind.started:
-          _onScrollStarted(scrollable);
-          break;
-        case ScrollNotificationKind.updated:
-          _onScrollUpdated(scrollable);
-          break;
-        case ScrollNotificationKind.ended:
-          _onScrollEnded(scrollable);
-          break;
-      }
+    }
+
+    final ScrollableState scrollable = notification.scrollable;
+    switch(notification.kind) {
+      case ScrollNotificationKind.started:
+        _onScrollStarted(scrollable);
+        break;
+      case ScrollNotificationKind.updated:
+        _onScrollUpdated(scrollable);
+        break;
+      case ScrollNotificationKind.ended:
+        _onScrollEnded(scrollable);
+        break;
     }
     return false;
   }

packages/flutter/lib/src/material/scrollbar.dart

@@ -98,6 +98,7 @@ class Scrollbar extends StatefulWidget {
 class _ScrollbarState extends State<Scrollbar> {
   final AnimationController _fade = new AnimationController(duration: _kScrollbarThumbFadeDuration);
   CurvedAnimation _opacity;
+  double _scrollOffsetAnchor;
   double _scrollOffset;
   Axis _scrollDirection;
   double _containerExtent;
@@ -109,6 +110,12 @@ class _ScrollbarState extends State<Scrollbar> {
     _opacity = new CurvedAnimation(parent: _fade, curve: Curves.ease);
   }
 
+  @override
+  void dispose() {
+    _fade.stop();
+    super.dispose();
+  }
+
   void _updateState(ScrollableState scrollable) {
     if (scrollable.scrollBehavior is! ExtentScrollBehavior)
       return;
@@ -121,13 +128,19 @@ class _ScrollbarState extends State<Scrollbar> {
 
   void _onScrollStarted(ScrollableState scrollable) {
     _updateState(scrollable);
-   _fade.forward();
+    _scrollOffsetAnchor = _scrollOffset;
   }
 
   void _onScrollUpdated(ScrollableState scrollable) {
-    setState(() {
-      _updateState(scrollable);
-    });
+    _updateState(scrollable);
+    if (!_fade.isAnimating) {
+      if (_scrollOffsetAnchor != _scrollOffset && _fade.value == 0.0)
+        _fade.forward(); // Lazily start the scrollbar fade-in.
+      setState(() {
+        // If the scrollbar has faded in, rebuild it per the new scrollable state.
+        // If the fade-in is underway this setState() will have no effect.
+      });
+    }
   }
 
   void _onScrollEnded(ScrollableState scrollable) {
@@ -136,19 +149,24 @@ class _ScrollbarState extends State<Scrollbar> {
   }
 
   bool _handleScrollNotification(ScrollNotification notification) {
-    if (config.scrollableKey == null || config.scrollableKey == notification.scrollable.config.key) {
-      final ScrollableState scrollable = notification.scrollable;
-      switch(notification.kind) {
-        case ScrollNotificationKind.started:
-          _onScrollStarted(scrollable);
-          break;
-        case ScrollNotificationKind.updated:
-          _onScrollUpdated(scrollable);
-          break;
-        case ScrollNotificationKind.ended:
-          _onScrollEnded(scrollable);
-          break;
-      }
+    if (config.scrollableKey == null) {
+        if (notification.depth != 0)
+          return false;
+    } else if (config.scrollableKey != notification.scrollable.config.key) {
+      return false;
+    }
+
+    final ScrollableState scrollable = notification.scrollable;
+    switch(notification.kind) {
+      case ScrollNotificationKind.started:
+        _onScrollStarted(scrollable);
+        break;
+      case ScrollNotificationKind.updated:
+        _onScrollUpdated(scrollable);
+        break;
+      case ScrollNotificationKind.ended:
+        _onScrollEnded(scrollable);
+        break;
     }
     return false;
   }

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

@@ -7,7 +7,6 @@ import 'dart:math' as math;
 import 'package:flutter/physics.dart';
 import 'package:meta/meta.dart';
 
-const double _kSecondsPerMillisecond = 1000.0;
 const double _kScrollDrag = 0.025;
 
 // TODO(hansmuller): Simplify these classes. We're no longer using the ScrollBehavior<T, U>
@@ -151,14 +150,12 @@ class BoundedBehavior extends ExtentScrollBehavior {
 }
 
 Simulation _createScrollSimulation(double position, double velocity, double minScrollOffset, double maxScrollOffset) {
-  final double startVelocity = velocity * _kSecondsPerMillisecond;
   final SpringDescription spring = new SpringDescription.withDampingRatio(mass: 1.0, springConstant: 170.0, ratio: 1.1);
-  return new ScrollSimulation(position, startVelocity, minScrollOffset, maxScrollOffset, spring, _kScrollDrag);
+  return new ScrollSimulation(position, velocity, minScrollOffset, maxScrollOffset, spring, _kScrollDrag);
 }
 
 Simulation _createSnapScrollSimulation(double startOffset, double endOffset, double startVelocity, double endVelocity) {
-  final double velocity = startVelocity * _kSecondsPerMillisecond;
-  return new FrictionSimulation.through(startOffset, endOffset, velocity, endVelocity);
+  return new FrictionSimulation.through(startOffset, endOffset, startVelocity, endVelocity);
 }
 
 /// A scroll behavior that does not prevent the user from exeeding scroll bounds.
@@ -169,9 +166,8 @@ class UnboundedBehavior extends ExtentScrollBehavior {
 
   @override
   Simulation createScrollSimulation(double position, double velocity) {
-    double velocityPerSecond = velocity * 1000.0;
     return new BoundedFrictionSimulation(
-      _kScrollDrag, position, velocityPerSecond, double.NEGATIVE_INFINITY, double.INFINITY
+      _kScrollDrag, position, velocity, double.NEGATIVE_INFINITY, double.INFINITY
     );
   }
 

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

@@ -430,8 +430,7 @@ class ScrollableState<T extends Scrollable> extends State<T> {
     if (_numberOfInProgressScrolls > 0) {
       if (_simulation != null) {
         double dx = _simulation.dx(_controller.lastElapsedDuration.inMicroseconds / Duration.MICROSECONDS_PER_SECOND);
-        // TODO(abarth): We should be consistent about the units we use for velocity (i.e., per second).
-        _startToEndAnimation(dx / Duration.MILLISECONDS_PER_SECOND);
+        _startToEndAnimation(dx); // dx - logical pixels / second
       }
       return;
     }
@@ -456,14 +455,15 @@ class ScrollableState<T extends Scrollable> extends State<T> {
     updateGestureDetector();
   }
 
-  /// Fling the scroll offset with the given velocity.
+  /// Fling the scroll offset with the given velocity in logical pixels/second.
   ///
   /// Calling this function starts a physics-based animation of the scroll
   /// offset with the given value as the initial velocity. The physics
-  /// simulation used is determined by the scroll behavior.
+  /// simulation is determined by the scroll behavior.
   Future<Null> fling(double scrollVelocity) {
-    if (scrollVelocity != 0.0 || !_controller.isAnimating)
+    if (scrollVelocity.abs() > kPixelScrollTolerance.velocity || !_controller.isAnimating)
       return _startToEndAnimation(scrollVelocity);
+
     return new Future<Null>.value();
   }
 
@@ -587,9 +587,9 @@ class ScrollableState<T extends Scrollable> extends State<T> {
     scrollBy(pixelOffsetToScrollOffset(details.primaryDelta));
   }
 
-  Future<Null> _handleDragEnd(DragEndDetails details) {
-    double scrollVelocity = pixelDeltaToScrollOffset(details.velocity.pixelsPerSecond) / Duration.MILLISECONDS_PER_SECOND;
-    return fling(scrollVelocity).then(_endScroll);
+  void _handleDragEnd(DragEndDetails details) {
+    final double scrollVelocity = pixelDeltaToScrollOffset(details.velocity.pixelsPerSecond);
+    fling(scrollVelocity).then(_endScroll);
   }
 
   Null _endScroll([Null _]) {

packages/flutter/test/widget/snap_scrolling_test.dart

@@ -51,7 +51,7 @@ Future<Null> fling(double velocity) {
 }
 
 void main() {
-  testWidgets('ScrollableList snap scrolling, fling(0.8)', (WidgetTester tester) async {
+  testWidgets('ScrollableList snap scrolling', (WidgetTester tester) async {
     await tester.pumpWidget(buildFrame());
 
     scrollOffset = 0.0;
@@ -60,7 +60,7 @@ void main() {
 
     Duration dt = const Duration(seconds: 2);
 
-    fling(1.0);
+    fling(1000.0);
     await tester.pump(); // Start the scheduler at 0.0
     await tester.pump(dt);
     expect(scrollOffset, closeTo(200.0, 1.0));
@@ -69,7 +69,7 @@ void main() {
     await tester.pump();
     expect(scrollOffset, 0.0);
 
-    fling(2.0);
+    fling(2000.0);
     await tester.pump();
     await tester.pump(dt);
     expect(scrollOffset, closeTo(400.0, 1.0));
@@ -78,7 +78,7 @@ void main() {
     await tester.pump();
     expect(scrollOffset, 400.0);
 
-    fling(-0.8);
+    fling(-800.0);
     await tester.pump();
     await tester.pump(dt);
     expect(scrollOffset, closeTo(0.0, 1.0));
@@ -87,7 +87,7 @@ void main() {
     await tester.pump();
     expect(scrollOffset, 800.0);
 
-    fling(-2.0);
+    fling(-2000.0);
     await tester.pump();
     await tester.pump(dt);
     expect(scrollOffset, closeTo(200.0, 1.0));
@@ -97,7 +97,7 @@ void main() {
     expect(scrollOffset, 800.0);
 
     bool completed = false;
-    fling(-2.0).then((_) {
+    fling(-2000.0).then((_) {
       completed = true;
       expectSync(scrollOffset, closeTo(200.0, 1.0));
     });