From 8f3c498f2d9819632ce5f1b8cca6eb760db88a18 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Thu, 4 Aug 2016 15:46:24 -0400 Subject: [PATCH] Initialize ScrollBehavior's content size to infinite rather than 0. (#5199) Why this matters: If you navigate back to a page with a Scrollable that has a nonzero scrollOffset, we will restore that scrollOffset. We clamp the scrollOffset to the contentExtent before the first layout, before contentExtent is updated to its proper value. Initializing contentExtent to INFINITY effectively disables the first clamp, until we can get a valid value from layout. Since the previous scrollOffset was valid, it seems safe to assume it's still valid. BUG=https://github.com/flutter/flutter/issues/4883 BUG=https://github.com/flutter/flutter/issues/4797 --- .../flutter/lib/src/widgets/scroll_behavior.dart | 12 +++++++----- .../test/widget/remember_scroll_position_test.dart | 6 ++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/flutter/lib/src/widgets/scroll_behavior.dart b/packages/flutter/lib/src/widgets/scroll_behavior.dart index 26d1b1f283..f286be3cd0 100644 --- a/packages/flutter/lib/src/widgets/scroll_behavior.dart +++ b/packages/flutter/lib/src/widgets/scroll_behavior.dart @@ -62,7 +62,9 @@ abstract class ScrollBehavior { /// that only scrolls along one axis). abstract class ExtentScrollBehavior extends ScrollBehavior { /// Creates a scroll behavior for a scrollable widget with linear extent. - ExtentScrollBehavior({ double contentExtent: 0.0, double containerExtent: 0.0 }) + /// We start with an INFINITE contentExtent so that we don't accidentally + /// clamp a scrollOffset until we receive an accurate value in updateExtents. + ExtentScrollBehavior({ double contentExtent: double.INFINITY, double containerExtent: 0.0 }) : _contentExtent = contentExtent, _containerExtent = containerExtent; /// The linear extent of the content inside the scrollable widget. @@ -111,7 +113,7 @@ abstract class ExtentScrollBehavior extends ScrollBehavior { class BoundedBehavior extends ExtentScrollBehavior { /// Creates a scroll behavior that does not overscroll. BoundedBehavior({ - double contentExtent: 0.0, + double contentExtent: double.INFINITY, double containerExtent: 0.0, double minScrollOffset: 0.0 }) : _minScrollOffset = minScrollOffset, @@ -161,7 +163,7 @@ Simulation _createSnapScrollSimulation(double startOffset, double endOffset, dou /// A scroll behavior that does not prevent the user from exeeding scroll bounds. class UnboundedBehavior extends ExtentScrollBehavior { /// Creates a scroll behavior with no scrolling limits. - UnboundedBehavior({ double contentExtent: 0.0, double containerExtent: 0.0 }) + UnboundedBehavior({ double contentExtent: double.INFINITY, double containerExtent: 0.0 }) : super(contentExtent: contentExtent, containerExtent: containerExtent); @override @@ -191,7 +193,7 @@ class UnboundedBehavior extends ExtentScrollBehavior { /// A scroll behavior that lets the user scroll beyond the scroll bounds with some resistance. class OverscrollBehavior extends BoundedBehavior { /// Creates a scroll behavior that resists, but does not prevent, scrolling beyond its limits. - OverscrollBehavior({ double contentExtent: 0.0, double containerExtent: 0.0, double minScrollOffset: 0.0 }) + OverscrollBehavior({ double contentExtent: double.INFINITY, double containerExtent: 0.0, double minScrollOffset: 0.0 }) : super(contentExtent: contentExtent, containerExtent: containerExtent, minScrollOffset: minScrollOffset); @override @@ -225,7 +227,7 @@ class OverscrollBehavior extends BoundedBehavior { /// A scroll behavior that lets the user scroll beyond the scroll bounds only when the bounds are disjoint. class OverscrollWhenScrollableBehavior extends OverscrollBehavior { /// Creates a scroll behavior that allows overscrolling only when some amount of scrolling is already possible. - OverscrollWhenScrollableBehavior({ double contentExtent: 0.0, double containerExtent: 0.0, double minScrollOffset: 0.0 }) + OverscrollWhenScrollableBehavior({ double contentExtent: double.INFINITY, double containerExtent: 0.0, double minScrollOffset: 0.0 }) : super(contentExtent: contentExtent, containerExtent: containerExtent, minScrollOffset: minScrollOffset); @override diff --git a/packages/flutter/test/widget/remember_scroll_position_test.dart b/packages/flutter/test/widget/remember_scroll_position_test.dart index a413fdf0d7..0baf927ba3 100644 --- a/packages/flutter/test/widget/remember_scroll_position_test.dart +++ b/packages/flutter/test/widget/remember_scroll_position_test.dart @@ -89,6 +89,12 @@ void main() { navigatorKey.currentState.pop(); await tester.pump(); // navigating always takes two frames + + // Ensure we don't clamp the scroll offset even during the navigation. + // https://github.com/flutter/flutter/issues/4883 + LazyListViewport viewport = tester.firstWidget(find.byType(LazyListViewport)); + expect(viewport.scrollOffset, equals(1000.0)); + await tester.pump(new Duration(seconds: 1)); // we're 600 pixels high, each item is 100 pixels high, scroll position is