From f4f68dff1b0cb84fc8ea78c609a6fd16a4ee4801 Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Tue, 9 May 2017 15:32:54 -0700 Subject: [PATCH] Make the page-view remembering logic actually work. (#9905) It wasn't tested, which is why it didn't work before. --- .../flutter/lib/src/widgets/page_view.dart | 9 ++- .../lib/src/widgets/scroll_position.dart | 3 + .../flutter/test/widgets/page_view_test.dart | 59 +++++++++++++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/packages/flutter/lib/src/widgets/page_view.dart b/packages/flutter/lib/src/widgets/page_view.dart index baa04998f6..353a200bdb 100644 --- a/packages/flutter/lib/src/widgets/page_view.dart +++ b/packages/flutter/lib/src/widgets/page_view.dart @@ -154,7 +154,9 @@ class _PagePosition extends ScrollPositionWithSingleContext { this.initialPage: 0, double viewportFraction: 1.0, ScrollPosition oldPosition, - }) : _viewportFraction = viewportFraction, super( + }) : _viewportFraction = viewportFraction, + _pageToUseOnStartup = initialPage.toDouble(), + super( physics: physics, context: context, initialPixels: null, @@ -166,6 +168,7 @@ class _PagePosition extends ScrollPositionWithSingleContext { } final int initialPage; + double _pageToUseOnStartup; double get viewportFraction => _viewportFraction; double _viewportFraction; @@ -198,7 +201,7 @@ class _PagePosition extends ScrollPositionWithSingleContext { if (pixels == null) { final double value = PageStorage.of(context.storageContext)?.readState(context.storageContext); if (value != null) - correctPixels(getPixelsFromPage(value)); + _pageToUseOnStartup = value; } } @@ -207,7 +210,7 @@ class _PagePosition extends ScrollPositionWithSingleContext { final double oldViewportDimensions = this.viewportDimension; final bool result = super.applyViewportDimension(viewportDimension); final double oldPixels = pixels; - final double page = (oldPixels == null || oldViewportDimensions == 0.0) ? initialPage.toDouble() : getPageFromPixels(oldPixels, oldViewportDimensions); + final double page = (oldPixels == null || oldViewportDimensions == 0.0) ? _pageToUseOnStartup : getPageFromPixels(oldPixels, oldViewportDimensions); final double newPixels = getPixelsFromPage(page); if (newPixels != oldPixels) { correctPixels(newPixels); diff --git a/packages/flutter/lib/src/widgets/scroll_position.dart b/packages/flutter/lib/src/widgets/scroll_position.dart index e065adba63..360b7a814e 100644 --- a/packages/flutter/lib/src/widgets/scroll_position.dart +++ b/packages/flutter/lib/src/widgets/scroll_position.dart @@ -300,6 +300,9 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { /// 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. + /// + /// This method is called from the constructor, so layout has not yet + /// occurred, and the viewport dimensions aren't yet known when it is called. @protected void restoreScrollOffset() { if (pixels == null) { diff --git a/packages/flutter/test/widgets/page_view_test.dart b/packages/flutter/test/widgets/page_view_test.dart index 2f0bff442c..29a4d17b7f 100644 --- a/packages/flutter/test/widgets/page_view_test.dart +++ b/packages/flutter/test/widgets/page_view_test.dart @@ -398,4 +398,63 @@ void main() { await tester.pump(); expect(changeIndex, 0); }); + + testWidgets('PageView can restore page', + (WidgetTester tester) async { + final PageController controller = new PageController(); + final PageStorageBucket bucket = new PageStorageBucket(); + await tester.pumpWidget( + new PageStorage( + bucket: bucket, + child: new PageView( + controller: controller, + children: [ + const Placeholder(), + const Placeholder(), + const Placeholder(), + ], + ), + ), + ); + expect(controller.page, 0); + controller.jumpToPage(2); + expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 1); + expect(controller.page, 2); + await tester.pumpWidget( + new PageStorage( + bucket: bucket, + child: new Container(), + ), + ); + expect(() => controller.page, throwsAssertionError); + await tester.pumpWidget( + new PageStorage( + bucket: bucket, + child: new PageView( + controller: controller, + children: [ + const Placeholder(), + const Placeholder(), + const Placeholder(), + ], + ), + ), + ); + expect(controller.page, 2); + await tester.pumpWidget( + new PageStorage( + bucket: bucket, + child: new PageView( + key: const Key('Check it again against your list and see consistency!'), + controller: controller, + children: [ + const Placeholder(), + const Placeholder(), + const Placeholder(), + ], + ), + ), + ); + expect(controller.page, 0); + }); }