diff --git a/packages/flutter/lib/src/widgets/page_view.dart b/packages/flutter/lib/src/widgets/page_view.dart index 8febea5cbb..3ed0f548ef 100644 --- a/packages/flutter/lib/src/widgets/page_view.dart +++ b/packages/flutter/lib/src/widgets/page_view.dart @@ -12,6 +12,7 @@ import 'package:flutter/rendering.dart'; import 'basic.dart'; import 'framework.dart'; import 'notification_listener.dart'; +import 'page_storage.dart'; import 'scroll_context.dart'; import 'scroll_controller.dart'; import 'scroll_metrics.dart'; @@ -187,6 +188,20 @@ class _PagePosition extends ScrollPositionWithSingleContext { double get page => pixels == null ? null : getPageFromPixels(pixels.clamp(minScrollExtent, maxScrollExtent), viewportDimension); + @override + void saveScrollOffset() { + PageStorage.of(context.storageContext)?.writeState(context.storageContext, getPageFromPixels(pixels, viewportDimension)); + } + + @override + void restoreScrollOffset() { + if (pixels == null) { + final double value = PageStorage.of(context.storageContext)?.readState(context.storageContext); + if (value != null) + correctPixels(getPixelsFromPage(value)); + } + } + @override bool applyViewportDimension(double viewportDimension) { final double oldViewportDimensions = this.viewportDimension; diff --git a/packages/flutter/lib/src/widgets/scroll_context.dart b/packages/flutter/lib/src/widgets/scroll_context.dart index 10ba7b79a8..71fec6d219 100644 --- a/packages/flutter/lib/src/widgets/scroll_context.dart +++ b/packages/flutter/lib/src/widgets/scroll_context.dart @@ -27,6 +27,14 @@ abstract class ScrollContext { /// [ScrollBehavior.buildViewportChrome]. BuildContext get notificationContext; + /// The [BuildContext] that should be used when searching for a [PageStorage]. + /// + /// This context is typically the context of the scrollable widget itself. In + /// particular, it should involve any [GlobalKey]s that are dynamically + /// created as part of creating the scrolling widget, since those would be + /// different each time the widget is created. + BuildContext get storageContext; + /// A [TickerProvider] to use when animating the scroll position. TickerProvider get vsync; diff --git a/packages/flutter/lib/src/widgets/scroll_controller.dart b/packages/flutter/lib/src/widgets/scroll_controller.dart index 1d671551cb..1b711bedfa 100644 --- a/packages/flutter/lib/src/widgets/scroll_controller.dart +++ b/packages/flutter/lib/src/widgets/scroll_controller.dart @@ -180,7 +180,24 @@ class ScrollController extends ChangeNotifier { /// resizes. /// /// By default, returns a [ScrollPositionWithSingleContext]. - ScrollPosition createScrollPosition( + /// + /// The arguments are generally passed to the [ScrollPosition] being created: + /// + /// * `physics`: An instance of [ScrollPhysics] that determines how the + /// [ScrollPosition] should react to user interactions, how it should + /// simulate scrolling when released or flung, etc. The value will not be + /// null. It typically comes from the [ScrollView] or other widget that + /// creates the [Scrollable], or, if none was provided, from the ambient + /// [ScrollConfiguration]. + /// * `context`: A [ScrollContext] used for communicating with the object + /// that is to own the [ScrollPosition] (typically, this is the + /// [Scrollable] itself). + /// * `oldPosition`: If this is not the first time a [ScrollPosition] has + /// been created for this [Scrollable], this will be the previous instance. + /// This is used when the environment has changed and the [Scrollable] + /// needs to recreate the [ScrollPosition] object. It is null the first + /// time the [ScrollPosition] is created. + ScrollPosition createScrollPosition( ScrollPhysics physics, ScrollContext context, ScrollPosition oldPosition, diff --git a/packages/flutter/lib/src/widgets/scroll_position.dart b/packages/flutter/lib/src/widgets/scroll_position.dart index c963ceadf9..bacb6f2ae0 100644 --- a/packages/flutter/lib/src/widgets/scroll_position.dart +++ b/packages/flutter/lib/src/widgets/scroll_position.dart @@ -12,6 +12,7 @@ import 'package:flutter/scheduler.dart'; import 'basic.dart'; import 'framework.dart'; import 'gesture_detector.dart'; +import 'page_storage.dart'; import 'scroll_activity.dart'; import 'scroll_context.dart'; import 'scroll_metrics.dart'; @@ -71,6 +72,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { assert(context.vsync != null); if (oldPosition != null) absorb(oldPosition); + restoreScrollOffset(); } /// How the scroll position should respond to user input. @@ -259,15 +261,54 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { /// To cause the position to jump or animate to a new value, consider [jumpTo] /// or [animateTo]. /// - /// This should not be called during layout. Consider [correctPixels] if you - /// find you need to adjust the position during layout. + /// This should not be called during layout (e.g. when setting the initial + /// scroll offset). Consider [correctPixels] if you find you need to adjust + /// the position during layout. @protected void forcePixels(double value) { - assert(_pixels != null); + assert(pixels != null); _pixels = value; notifyListeners(); } + /// Called whenever scrolling ends, to store the current scroll offset in a + /// storage mechanism with a lifetime that matches the app's lifetime. + /// + /// The stored value will be used by [restoreScrollOffset] when the + /// [ScrollPosition] is recreated, in the case of the [Scrollable] being + /// disposed then recreated in the same session. This might happen, for + /// instance, if a [ListView] is on one of the pages inside a [TabBarView], + /// and that page is displayed, then hidden, then displayed again. + /// + /// The default implementation writes the [pixels] using the nearest + /// [PageStorage] found from the [context]'s [ScrollContext.storageContext] + /// property. + @protected + void saveScrollOffset() { + PageStorage.of(context.storageContext)?.writeState(context.storageContext, pixels); + } + + /// Called whenever the [ScrollPosition] is created, to restore the scroll + /// offset if possible. + /// + /// The value is stored by [saveScrollOffset] when the scroll position + /// changes, so that it can be restored in the case of the [Scrollable] being + /// disposed then recreated in the same session. This might happen, for + /// instance, if a [ListView] is on one of the pages inside a [TabBarView], + /// and that page is displayed, then hidden, then displayed again. + /// + /// The default implementation reads the value from the nearest [PageStorage] + /// found from the [context]'s [ScrollContext.storageContext] property, and + /// sets it using [correctPixels], if [pixels] is still null. + @protected + void restoreScrollOffset() { + if (pixels == null) { + final double value = PageStorage.of(context.storageContext)?.readState(context.storageContext); + if (value != null) + correctPixels(value); + } + } + /// Returns the overscroll by applying the boundary conditions. /// /// If the given value is in bounds, returns 0.0. Otherwise, returns the @@ -467,7 +508,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { oldIgnorePointer = _activity.shouldIgnorePointer; wasScrolling = _activity.isScrolling; if (wasScrolling && !newActivity.isScrolling) - didEndScroll(); + didEndScroll(); // notifies and then saves the scroll offset _activity.dispose(); } else { oldIgnorePointer = false; @@ -495,8 +536,11 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { } /// Called by [beginActivity] to report when an activity has ended. + /// + /// This also saves the scroll offset using [saveScrollOffset]. void didEndScroll() { activity.dispatchScrollEndNotification(cloneMetrics(), context.notificationContext); + saveScrollOffset(); } /// Called by [setPixels] to report overscroll when an attempt is made to diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index 9c91495ebb..c1762fc984 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -358,6 +358,9 @@ class ScrollableState extends State with TickerProviderStateMixin @override BuildContext get notificationContext => _gestureDetectorKey.currentContext; + @override + BuildContext get storageContext => context; + // TOUCH HANDLERS Drag _drag; diff --git a/packages/flutter/test/widgets/remember_scroll_position_test.dart b/packages/flutter/test/widgets/remember_scroll_position_test.dart index 0d5a2db107..393addb450 100644 --- a/packages/flutter/test/widgets/remember_scroll_position_test.dart +++ b/packages/flutter/test/widgets/remember_scroll_position_test.dart @@ -8,6 +8,10 @@ import 'package:meta/meta.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter/material.dart'; +ScrollController _controller = new ScrollController( + initialScrollOffset: 110.0, +); + class ThePositiveNumbers extends StatelessWidget { const ThePositiveNumbers({ @required this.from }); final int from; @@ -15,6 +19,7 @@ class ThePositiveNumbers extends StatelessWidget { Widget build(BuildContext context) { return new ListView.builder( itemExtent: 100.0, + controller: _controller, itemBuilder: (BuildContext context, int index) { return new Text('${index + from}', key: new ValueKey(index)); } @@ -22,98 +27,112 @@ class ThePositiveNumbers extends StatelessWidget { } } -Future performTest(WidgetTester tester) async { +Future performTest(WidgetTester tester, bool maintainState) async { + final GlobalKey navigatorKey = new GlobalKey(); + await tester.pumpWidget(new Navigator( + key: navigatorKey, + onGenerateRoute: (RouteSettings settings) { + if (settings.name == '/') { + return new MaterialPageRoute( + settings: settings, + builder: (_) => new Container(child: const ThePositiveNumbers(from: 0)), + maintainState: maintainState, + ); + } else if (settings.name == '/second') { + return new MaterialPageRoute( + settings: settings, + builder: (_) => new Container(child: const ThePositiveNumbers(from: 10000)), + maintainState: maintainState, + ); + } + return null; + } + )); + + // we're 600 pixels high, each item is 100 pixels high, scroll position is + // 110.0, so we should have 7 items, 1..7. + expect(find.text('0'), findsNothing); + expect(find.text('1'), findsOneWidget); + expect(find.text('2'), findsOneWidget); + expect(find.text('3'), findsOneWidget); + expect(find.text('4'), findsOneWidget); + expect(find.text('5'), findsOneWidget); + expect(find.text('6'), findsOneWidget); + expect(find.text('7'), findsOneWidget); + expect(find.text('8'), findsNothing); + expect(find.text('10'), findsNothing); + expect(find.text('100'), findsNothing); + + tester.state(find.byType(Scrollable)).position.jumpTo(1000.0); + await tester.pump(const Duration(seconds: 1)); + + // we're 600 pixels high, each item is 100 pixels high, scroll position is + // 1000, so we should have exactly 6 items, 10..15. + + expect(find.text('0'), findsNothing); + expect(find.text('1'), findsNothing); + expect(find.text('8'), findsNothing); + expect(find.text('9'), findsNothing); + expect(find.text('10'), findsOneWidget); + expect(find.text('11'), findsOneWidget); + expect(find.text('12'), findsOneWidget); + expect(find.text('13'), findsOneWidget); + expect(find.text('14'), findsOneWidget); + expect(find.text('15'), findsOneWidget); + expect(find.text('16'), findsNothing); + expect(find.text('100'), findsNothing); + + navigatorKey.currentState.pushNamed('/second'); + await tester.pump(); // navigating always takes two frames, one to start... + await tester.pump(const Duration(seconds: 1)); // ...and one to end the transition + + // the second list is now visible, starting at 10001 + expect(find.text('0'), findsNothing); + expect(find.text('1'), findsNothing); + expect(find.text('10'), findsNothing); + expect(find.text('11'), findsNothing); + expect(find.text('10000'), findsNothing); + expect(find.text('10001'), findsOneWidget); + expect(find.text('10002'), findsOneWidget); + expect(find.text('10003'), findsOneWidget); + expect(find.text('10004'), findsOneWidget); + expect(find.text('10005'), findsOneWidget); + expect(find.text('10006'), findsOneWidget); + expect(find.text('10007'), findsOneWidget); + expect(find.text('10008'), findsNothing); + expect(find.text('10010'), findsNothing); + expect(find.text('10100'), findsNothing); + + navigatorKey.currentState.pop(); + await tester.pump(); // again, navigating always takes two frames + + // Ensure we don't clamp the scroll offset even during the navigation. + // https://github.com/flutter/flutter/issues/4883 + final ScrollableState state = tester.state(find.byType(Scrollable).first); + expect(state.position.pixels, equals(1000.0)); + + await tester.pump(const Duration(seconds: 1)); + + // we're 600 pixels high, each item is 100 pixels high, scroll position is + // 1000, so we should have exactly 6 items, 10..15. + + expect(find.text('0'), findsNothing); + expect(find.text('1'), findsNothing); + expect(find.text('8'), findsNothing); + expect(find.text('9'), findsNothing); + expect(find.text('10'), findsOneWidget); + expect(find.text('11'), findsOneWidget); + expect(find.text('12'), findsOneWidget); + expect(find.text('13'), findsOneWidget); + expect(find.text('14'), findsOneWidget); + expect(find.text('15'), findsOneWidget); + expect(find.text('16'), findsNothing); + expect(find.text('100'), findsNothing); } void main() { testWidgets('whether we remember our scroll position', (WidgetTester tester) async { - final GlobalKey navigatorKey = new GlobalKey(); - await tester.pumpWidget(new Navigator( - key: navigatorKey, - onGenerateRoute: (RouteSettings settings) { - if (settings.name == '/') { - return new MaterialPageRoute( - settings: settings, - builder: (_) => new Container(child: const ThePositiveNumbers(from: 0)), - ); - } else if (settings.name == '/second') { - return new MaterialPageRoute( - settings: settings, - builder: (_) => new Container(child: const ThePositiveNumbers(from: 10000)), - ); - } - return null; - } - )); - - // we're 600 pixels high, each item is 100 pixels high, scroll position is - // zero, so we should have exactly 6 items, 0..5. - expect(find.text('0'), findsOneWidget); - expect(find.text('1'), findsOneWidget); - expect(find.text('2'), findsOneWidget); - expect(find.text('3'), findsOneWidget); - expect(find.text('4'), findsOneWidget); - expect(find.text('5'), findsOneWidget); - expect(find.text('6'), findsNothing); - expect(find.text('10'), findsNothing); - expect(find.text('100'), findsNothing); - - tester.state(find.byType(Scrollable)).position.jumpTo(1000.0); - await tester.pump(const Duration(seconds: 1)); - - // we're 600 pixels high, each item is 100 pixels high, scroll position is - // 1000, so we should have exactly 6 items, 10..15. - - expect(find.text('0'), findsNothing); - expect(find.text('8'), findsNothing); - expect(find.text('9'), findsNothing); - expect(find.text('10'), findsOneWidget); - expect(find.text('11'), findsOneWidget); - expect(find.text('12'), findsOneWidget); - expect(find.text('13'), findsOneWidget); - expect(find.text('14'), findsOneWidget); - expect(find.text('15'), findsOneWidget); - expect(find.text('16'), findsNothing); - expect(find.text('100'), findsNothing); - - navigatorKey.currentState.pushNamed('/second'); - await tester.pump(); // navigating always takes two frames, one to start... - await tester.pump(const Duration(seconds: 1)); // ...and one to end the transition - - // the second list is now visible, starting at 10000 - expect(find.text('10000'), findsOneWidget); - expect(find.text('10001'), findsOneWidget); - expect(find.text('10002'), findsOneWidget); - expect(find.text('10003'), findsOneWidget); - expect(find.text('10004'), findsOneWidget); - expect(find.text('10005'), findsOneWidget); - expect(find.text('10006'), findsNothing); - expect(find.text('10010'), findsNothing); - expect(find.text('10100'), findsNothing); - - navigatorKey.currentState.pop(); - await tester.pump(); // again, navigating always takes two frames - - // Ensure we don't clamp the scroll offset even during the navigation. - // https://github.com/flutter/flutter/issues/4883 - final ScrollableState state = tester.state(find.byType(Scrollable).first); - expect(state.position.pixels, equals(1000.0)); - - await tester.pump(const Duration(seconds: 1)); - - // we're 600 pixels high, each item is 100 pixels high, scroll position is - // 1000, so we should have exactly 6 items, 10..15. - - expect(find.text('0'), findsNothing); - expect(find.text('8'), findsNothing); - expect(find.text('9'), findsNothing); - expect(find.text('10'), findsOneWidget); - expect(find.text('11'), findsOneWidget); - expect(find.text('12'), findsOneWidget); - expect(find.text('13'), findsOneWidget); - expect(find.text('14'), findsOneWidget); - expect(find.text('15'), findsOneWidget); - expect(find.text('16'), findsNothing); - expect(find.text('100'), findsNothing); + await performTest(tester, true); + await performTest(tester, false); }); }