Refresh indicator overscroll (#5836)

* Added OverscrollIndicatorEdge et al

* RefreshIndicator only clamps its scrollable edge

* added a test

* Updated the test

* fixed lint-os

* fixed a typo

* Scrollable should restore its viewport dimensions when it reappears

* removed an accidental commit

* updated per review feedback

examples/flutter_gallery/lib/demo/overscroll_demo.dart

@@ -45,46 +45,34 @@ class OverscrollDemoState extends State<OverscrollDemo> {
 
   @override
   Widget build(BuildContext context) {
+    Widget body = new MaterialList(
+      type: MaterialListType.threeLine,
+      padding: const EdgeInsets.all(8.0),
+      scrollableKey: _scrollableKey,
+      children: _items.map((String item) {
+        return new ListItem(
+          isThreeLine: true,
+          leading: new CircleAvatar(child: new Text(item)),
+          title: new Text('This item represents $item.'),
+          subtitle: new Text('Even more additional list item information appears on line three.')
+        );
+      })
+    );
+
     String  indicatorTypeText;
-    switch(_type) {
+    switch (_type) {
       case IndicatorType.overscroll:
         indicatorTypeText = 'Over-scroll indicator';
         break;
-      case IndicatorType.refresh:
-        indicatorTypeText = 'Refresh indicator';
-        break;
-    }
-
-    // The default ScrollConfiguration doesn't include the
-    // OverscrollIndicator. That's what we want, since this demo
-    // adds the OverscrollIndicator itself.
-    Widget body = new ScrollConfiguration(
-      child: new MaterialList(
-        type: MaterialListType.threeLine,
-        padding: const EdgeInsets.all(8.0),
-        scrollableKey: _scrollableKey,
-        children: _items.map((String item) {
-          return new ListItem(
-            isThreeLine: true,
-            leading: new CircleAvatar(child: new Text(item)),
-            title: new Text('This item represents $item.'),
-            subtitle: new Text('Even more additional list item information appears on line three.')
-          );
-        })
-      )
-    );
-    switch(_type) {
-      case IndicatorType.overscroll:
-        body = new OverscrollIndicator(child: body);
-        break;
       case IndicatorType.refresh:
         body = new RefreshIndicator(
           key: _refreshIndicatorKey,
-          child: body,
           refresh: refresh,
           scrollableKey: _scrollableKey,
-          location: RefreshIndicatorLocation.top
+          location: RefreshIndicatorLocation.top,
+          child: body,
         );
+        indicatorTypeText = 'Refresh indicator';
         break;
     }
 

examples/flutter_gallery/lib/demo/pesto_demo.dart

@@ -304,7 +304,7 @@ class _RecipePageState extends State<RecipePage> {
           )
         ),
         new ClampOverscrolls(
-          value: true,
+          edge: ScrollableEdge.both,
           child: new ScrollableViewport(
             scrollableKey: _scrollableKey,
             child: new RepaintBoundary(

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

@@ -173,8 +173,29 @@ class _ScrollLikeMountainViewDelegate extends ScrollConfigurationDelegate {
   @override
   ExtentScrollBehavior createScrollBehavior() => new OverscrollWhenScrollableBehavior(platform: TargetPlatform.android);
 
+  ScrollableEdge _overscrollIndicatorEdge(ScrollableEdge edge) {
+    switch (edge) {
+      case ScrollableEdge.leading:
+        return ScrollableEdge.trailing;
+      case ScrollableEdge.trailing:
+        return ScrollableEdge.leading;
+      case ScrollableEdge.both:
+        return ScrollableEdge.none;
+      case ScrollableEdge.none:
+        return ScrollableEdge.both;
+    }
+    return ScrollableEdge.both;
+  }
+
   @override
-  Widget wrapScrollWidget(Widget scrollWidget) => new OverscrollIndicator(child: scrollWidget);
+  Widget wrapScrollWidget(BuildContext context, Widget scrollWidget) {
+    // Only introduce an overscroll indicator for the edges of the scrollable
+    // that aren't already clamped.
+    return new OverscrollIndicator(
+      edge: _overscrollIndicatorEdge(ClampOverscrolls.of(context)?.edge),
+      child: scrollWidget
+    );
+  }
 
   @override
   bool updateShouldNotify(ScrollConfigurationDelegate old) => false;

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

@@ -89,7 +89,9 @@ class _DropDownScrollConfigurationDelegate extends ScrollConfigurationDelegate {
   ExtentScrollBehavior createScrollBehavior() => new OverscrollWhenScrollableBehavior(platform: platform);
 
   @override
-  Widget wrapScrollWidget(Widget scrollWidget) => new ClampOverscrolls(value: true, child: scrollWidget);
+  Widget wrapScrollWidget(BuildContext context, Widget scrollWidget) {
+    return new ClampOverscrolls(edge: ScrollableEdge.both, child: scrollWidget);
+  }
 
   @override
   bool updateShouldNotify(ScrollConfigurationDelegate old) => platform != old.platform;

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

@@ -50,7 +50,7 @@ class _Painter extends CustomPainter {
     final double width = size.width;
     final double height = size.height;
 
-    switch(scrollDirection) {
+    switch (scrollDirection) {
       case Axis.vertical:
         final double radius = width * _kSizeToRadius;
         final double centerX = width / 2.0;
@@ -97,9 +97,11 @@ class OverscrollIndicator extends StatefulWidget {
   OverscrollIndicator({
     Key key,
     this.scrollableKey,
+    this.edge: ScrollableEdge.both,
     this.child
   }) : super(key: key) {
     assert(child != null);
+    assert(edge != null);
   }
 
   /// Identifies the [Scrollable] descendant of child that the overscroll
@@ -107,6 +109,9 @@ class OverscrollIndicator extends StatefulWidget {
   /// descendant.
   final Key scrollableKey;
 
+  /// Where the overscroll indicator should appear.
+  final ScrollableEdge edge;
+
   /// The overscroll indicator will be stacked on top of this child. The
   /// indicator will appear when child's [Scrollable] descendant is
   /// over-scrolled.
@@ -167,10 +172,12 @@ class _OverscrollIndicatorState extends State<OverscrollIndicator> {
       // Hide the indicator as soon as user starts scrolling in the reverse direction of overscroll.
       if (_isReverseScroll(value)) {
         _hide(_kNormalHideDuration);
-      } else {
+      } else if (_isMatchingOverscrollEdge(value)) {
         // Changing the animation's value causes an implicit setState().
         _dragPosition = details?.globalPosition ?? Point.origin;
         _extentAnimation.value = value < _minScrollOffset ? _minScrollOffset - value : value - _maxScrollOffset;
+      } else {
+        _hide(_kNormalHideDuration);
       }
     }
     _updateState(scrollable);
@@ -194,6 +201,20 @@ class _OverscrollIndicatorState extends State<OverscrollIndicator> {
       ((scrollOffset - _scrollOffset).abs() > kPixelScrollTolerance.distance);
   }
 
+  bool _isMatchingOverscrollEdge(double scrollOffset) {
+    switch (config.edge) {
+      case ScrollableEdge.both:
+        return true;
+      case ScrollableEdge.leading:
+        return scrollOffset < _minScrollOffset;
+      case ScrollableEdge.trailing:
+        return scrollOffset > _maxScrollOffset;
+      case ScrollableEdge.none:
+        return false;
+    }
+    return false;
+  }
+
   bool _isReverseScroll(double scrollOffset) {
     final double delta = _scrollOffset - scrollOffset;
     return scrollOffset < _minScrollOffset ? delta < 0.0 : delta > 0.0;
@@ -208,7 +229,7 @@ class _OverscrollIndicatorState extends State<OverscrollIndicator> {
     }
 
     final ScrollableState scrollable = notification.scrollable;
-    switch(notification.kind) {
+    switch (notification.kind) {
       case ScrollNotificationKind.started:
         _onScrollStarted(scrollable);
         break;
@@ -256,9 +277,10 @@ class _OverscrollIndicatorState extends State<OverscrollIndicator> {
             child: child
           );
         },
-        child: new ClampOverscrolls(
+        child: new ClampOverscrolls.inherit(
+          context: context,
+          edge: config.edge,
           child: config.child,
-          value: true
         )
       )
     );

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

@@ -331,6 +331,18 @@ class RefreshIndicatorState extends State<RefreshIndicator> {
     }
   }
 
+  ScrollableEdge get _clampOverscrollsEdge {
+    switch (config.location) {
+      case RefreshIndicatorLocation.top:
+        return ScrollableEdge.leading;
+      case RefreshIndicatorLocation.bottom:
+        return ScrollableEdge.trailing;
+      case RefreshIndicatorLocation.both:
+        return ScrollableEdge.both;
+    }
+    return ScrollableEdge.none;
+  }
+
   @override
   Widget build(BuildContext context) {
     final ThemeData theme = Theme.of(context);
@@ -353,9 +365,10 @@ class RefreshIndicatorState extends State<RefreshIndicator> {
       onPointerUp: _handlePointerUp,
       child: new Stack(
         children: <Widget>[
-          new ClampOverscrolls(
+          new ClampOverscrolls.inherit(
+            context: context,
+            edge: _clampOverscrollsEdge,
             child: config.child,
-            value: true
           ),
           new Positioned(
             top: _isIndicatorAtTop ? 0.0 : null,

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

@@ -609,10 +609,9 @@ class ScaffoldState extends State<Scaffold> {
     if ((scrollable.config.scrollDirection == Axis.vertical) &&
         (config.scrollableKey == null || config.scrollableKey == scrollable.config.key)) {
       double newScrollOffset = scrollable.scrollOffset;
-      if (ClampOverscrolls.of(scrollable.context)) {
-        ExtentScrollBehavior limits = scrollable.scrollBehavior;
-        newScrollOffset = newScrollOffset.clamp(limits.minScrollOffset, limits.maxScrollOffset);
-      }
+      final ClampOverscrolls clampOverscrolls = ClampOverscrolls.of(context);
+      if (clampOverscrolls != null)
+        newScrollOffset = clampOverscrolls.clampScrollOffset(scrollable);
       if (_scrollOffset != newScrollOffset) {
         setState(() {
           _scrollOffsetDelta = _scrollOffset - newScrollOffset;

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

@@ -13,39 +13,103 @@ import 'scrollable.dart';
 /// scrolled to the given `scrollOffset`.
 typedef Widget ViewportBuilder(BuildContext context, ScrollableState state, double scrollOffset);
 
-/// A widget that controls whether [Scrollable] descendants will overscroll.
+/// A widget that controls whether viewport descendants will overscroll their contents.
+/// Overscrolling is clamped at the beginning or end or both according to the
+/// [edge] parameter.
 ///
-/// If `true`, the ClampOverscroll's [Scrollable] descendant will clamp its
-/// viewport's scrollOffsets to the [ScrollBehavior]'s min and max values.
-/// In this case the Scrollable's scrollOffset will still over- and undershoot
-/// the ScrollBehavior's limits, but the viewport itself will not.
+/// Scroll offset limits are defined by the enclosing Scrollable's [ScrollBehavior].
 class ClampOverscrolls extends InheritedWidget {
-  /// Creates a widget that controls whether [Scrollable] descendants will overscroll.
+  /// Creates a widget that controls whether viewport descendants will overscroll
+  /// their contents.
   ///
-  /// The [value] and [child] arguments must not be null.
+  /// The [edge] and [child] arguments must not be null.
   ClampOverscrolls({
     Key key,
-    @required this.value,
-    @required Widget child
+    this.edge: ScrollableEdge.none,
+    @required Widget child,
   }) : super(key: key, child: child) {
-    assert(value != null);
+    assert(edge != null);
     assert(child != null);
   }
 
-  /// Whether [Scrollable] descendants should clamp their viewport's
-  /// scrollOffset values when they are less than the [ScrollBehavior]'s minimum
-  /// or greater than its maximum.
-  final bool value;
+  /// Creates a widget that controls whether viewport descendants will overscroll
+  /// based on the given [edge] and the inherited ClampOverscrolls widget for
+  /// the given [context]. For example if edge is ScrollableEdge.leading
+  /// and a ClampOverscrolls ancestor exists that specified ScrollableEdge.trailing,
+  /// then this widget would clamp both scrollable edges.
+  ///
+  /// The [context], [edge] and [child] arguments must not be null.
+  factory ClampOverscrolls.inherit({
+    Key key,
+    @required BuildContext context,
+    @required ScrollableEdge edge: ScrollableEdge.none,
+    @required Widget child
+  }) {
+    assert(context != null);
+    assert(edge != null);
+    assert(child != null);
 
-  /// Whether a [Scrollable] widget within the given context should overscroll.
-  static bool of(BuildContext context) {
-    final ClampOverscrolls result = context.inheritFromWidgetOfExactType(ClampOverscrolls);
-    return result?.value ?? false;
+    // The child's clamped edge is the union of the given edge and the
+    // parent's clamped edge.
+    ScrollableEdge parentEdge = ClampOverscrolls.of(context)?.edge ?? ScrollableEdge.none;
+    ScrollableEdge childEdge = edge;
+    switch (parentEdge) {
+      case ScrollableEdge.leading:
+        if (edge == ScrollableEdge.trailing || edge == ScrollableEdge.both)
+          childEdge = ScrollableEdge.both;
+        break;
+      case ScrollableEdge.trailing:
+        if (edge == ScrollableEdge.leading || edge == ScrollableEdge.both)
+          childEdge = ScrollableEdge.both;
+        break;
+      case ScrollableEdge.both:
+        childEdge = ScrollableEdge.both;
+        break;
+      case ScrollableEdge.none:
+        break;
+    }
+
+    return new ClampOverscrolls(
+      key: key,
+      edge: childEdge,
+      child: child
+    );
   }
 
-  /// If ClampOverscrolls is true, clamps the ScrollableState's scrollOffset to the
-  /// [ScrollBehavior] minimum and maximum values and then constructs the viewport
-  /// with the clamped scrollOffset. ClampOverscrolls is reset to false for viewport
+  /// Defines when viewport scrollOffsets are clamped in terms of the scrollDirection.
+  /// If edge is `leading` the viewport's scrollOffset will be clamped at its minimum
+  /// value (often 0.0). If edge is `trailing` then the scrollOffset will be clamped
+  /// to its maximum value.  If edge is `both` then both the leading and trailing
+  /// constraints are applied.
+  final ScrollableEdge edge;
+
+  /// Return the [scrollable]'s scrollOffset clamped  according to [edge].
+  double clampScrollOffset(ScrollableState scrollable) {
+    final double scrollOffset = scrollable.scrollOffset;
+    final double minScrollOffset = scrollable.scrollBehavior.minScrollOffset;
+    final double maxScrollOffset = scrollable.scrollBehavior.maxScrollOffset;
+    switch (edge) {
+      case ScrollableEdge.both:
+        return scrollOffset.clamp(minScrollOffset, maxScrollOffset);
+      case ScrollableEdge.leading:
+        return scrollOffset.clamp(minScrollOffset, double.INFINITY);
+      case ScrollableEdge.trailing:
+        return scrollOffset.clamp(double.NEGATIVE_INFINITY, maxScrollOffset);
+      case ScrollableEdge.none:
+        return scrollOffset;
+    }
+    return scrollOffset;
+  }
+
+  /// The closest instance of this class that encloses the given context.
+  static ClampOverscrolls of(BuildContext context) {
+    return context.inheritFromWidgetOfExactType(ClampOverscrolls);
+  }
+
+  /// Clamps the new viewport's scroll offset according to the value of
+  /// `ClampOverscrolls.of(context).edge`.
+  ///
+  /// The clamped overscroll edge is reset to [ScrollableEdge.none] for the viewport's
   /// descendants.
   ///
   /// This utility function is typically used by [Scrollable.builder] callbacks.
@@ -54,22 +118,23 @@ class ClampOverscrolls extends InheritedWidget {
     // by the container and content size. But we don't know those until we
     // layout the viewport, which happens after build phase. We need to rethink
     // this.
-    final bool clampOverscrolls = ClampOverscrolls.of(context);
-    final double clampedScrollOffset = clampOverscrolls
-      ? state.scrollOffset.clamp(state.scrollBehavior.minScrollOffset, state.scrollBehavior.maxScrollOffset)
-      : state.scrollOffset;
+    final ClampOverscrolls clampOverscrolls = ClampOverscrolls.of(context);
+    if (clampOverscrolls == null)
+      return builder(context, state, state.scrollOffset);
+
+    final double clampedScrollOffset = clampOverscrolls.clampScrollOffset(state);
     Widget viewport = builder(context, state, clampedScrollOffset);
-    if (clampOverscrolls)
-      viewport = new ClampOverscrolls(value: false, child: viewport);
+    if (clampOverscrolls.edge != ScrollableEdge.none)
+      viewport = new ClampOverscrolls(edge: ScrollableEdge.none, child: viewport);
     return viewport;
   }
 
   @override
-  bool updateShouldNotify(ClampOverscrolls old) => value != old.value;
+  bool updateShouldNotify(ClampOverscrolls old) => edge != old.edge;
 
   @override
   void debugFillDescription(List<String> description) {
     super.debugFillDescription(description);
-    description.add('value: $value');
+    description.add('edge: $edge');
   }
 }

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

@@ -1,4 +1,4 @@
-// Copyright 2016 The Chromium Authors. All rights reserved.
+/// Copyright 2016 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.
 
@@ -28,7 +28,7 @@ abstract class ScrollConfigurationDelegate {
   /// Scrollable they create. It can be used to add widgets that wrap the
   /// Scrollable, like scrollbars or overscroll indicators. By default the
   /// [scrollWidget] parameter is returned unchanged.
-  Widget wrapScrollWidget(Widget scrollWidget) => scrollWidget;
+  Widget wrapScrollWidget(BuildContext context, Widget scrollWidget) => scrollWidget;
 
   /// Overrides should return true if this ScrollConfigurationDelegate differs
   /// from the provided old delegate in a way that requires rebuilding its
@@ -87,7 +87,7 @@ class ScrollConfiguration extends InheritedWidget {
 
   /// A utility function that calls [ScrollConfigurationDelegate.wrapScrollWidget].
   static Widget wrap(BuildContext context, Widget scrollWidget) {
-    return ScrollConfiguration.of(context).wrapScrollWidget(scrollWidget);
+    return ScrollConfiguration.of(context).wrapScrollWidget(context, scrollWidget);
   }
 
   @override

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

@@ -19,6 +19,24 @@ import 'page_storage.dart';
 import 'scroll_behavior.dart';
 import 'scroll_configuration.dart';
 
+/// Identifies one or both limits of a [Scrollable] in terms of its scrollDirection.
+enum ScrollableEdge {
+  /// The top and bottom of the scrollable if its scrollDirection is vertical
+  /// or the left and right if its scrollDirection is horizontal.
+  both,
+
+  /// Only the top of the scrollable if its scrollDirection is vertical,
+  /// or only the left if its scrollDirection is horizontal.
+  leading,
+
+  /// Only the bottom of the scrollable if its scroll-direction is vertical,
+  /// or only the right if its scrollDirection is horizontal.
+  trailing,
+
+  /// The overscroll indicator should not appear at all.
+  none,
+}
+
 /// The accuracy to which scrolling is computed.
 final Tolerance kPixelScrollTolerance = new Tolerance(
   // TODO(ianh): Handle the case of the device pixel ratio changing.

packages/flutter/test/widget/clamp_overscrolls_test.dart

@@ -0,0 +1,73 @@
+// Copyright 2016 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 'package:flutter_test/flutter_test.dart';
+import 'package:flutter/widgets.dart';
+
+// Assuming that the test container is 800x600. The height of the
+// viewport's contents is 650.0, the top and bottom text children
+// are 100 pixels high and top/left edge of both widgets are visible.
+// The top of the bottom widget is at 550 (the top of the top widget
+// is at 0). The top of the bottom widget is 500 when it has been
+// scrolled completely into view.
+Widget buildFrame(ScrollableEdge clampedEdge) {
+  return new ClampOverscrolls(
+    edge: clampedEdge,
+    child: new ScrollableViewport(
+      scrollableKey: new UniqueKey(),
+      child: new SizedBox(
+        height: 650.0,
+        child: new Column(
+          crossAxisAlignment: CrossAxisAlignment.start,
+          children: <Widget>[
+            new SizedBox(height: 100.0, child: new Text('top')),
+            new Flexible(child: new Container()),
+            new SizedBox(height: 100.0, child: new Text('bottom')),
+          ]
+        )
+      )
+    )
+  );
+}
+
+void main() {
+  testWidgets('ClampOverscrolls', (WidgetTester tester) async {
+
+    // Scroll the target text widget by offset and then return its origin
+    // in global coordinates.
+    Future<Point> locationAfterScroll(String target, Offset offset) async {
+      await tester.scrollAt(tester.getTopLeft(find.text(target)), offset);
+      await tester.pump();
+      final RenderBox textBox = tester.renderObject(find.text(target));
+      final Point widgetOrigin = textBox.localToGlobal(Point.origin);
+      await tester.pump(const Duration(seconds: 1)); // Allow overscroll to settle
+      return new Future<Point>.value(widgetOrigin);
+    }
+
+    await tester.pumpWidget(buildFrame(ScrollableEdge.none));
+    Point origin = await locationAfterScroll('top', const Offset(0.0, 400.0));
+    expect(origin.y, greaterThan(0.0));
+    origin = await locationAfterScroll('bottom', const Offset(0.0, -400.0));
+    expect(origin.y, lessThan(500.0));
+
+
+    await tester.pumpWidget(buildFrame(ScrollableEdge.both));
+    origin = await locationAfterScroll('top', const Offset(0.0, 400.0));
+    expect(origin.y, equals(0.0));
+    origin = await locationAfterScroll('bottom', const Offset(0.0, -400.0));
+    expect(origin.y, equals(500.0));
+
+    await tester.pumpWidget(buildFrame(ScrollableEdge.leading));
+    origin = await locationAfterScroll('top', const Offset(0.0, 400.0));
+    expect(origin.y, equals(0.0));
+    origin = await locationAfterScroll('bottom', const Offset(0.0, -400.0));
+    expect(origin.y, lessThan(500.0));
+
+    await tester.pumpWidget(buildFrame(ScrollableEdge.trailing));
+    origin = await locationAfterScroll('top', const Offset(0.0, 400.0));
+    expect(origin.y, greaterThan(0.0));
+    origin = await locationAfterScroll('bottom', const Offset(0.0, -400.0));
+    expect(origin.y, equals(500.0));
+  });
+}