From ba734ed63c2e95f29565e7488bc8dacd79ec1b71 Mon Sep 17 00:00:00 2001 From: xubaolin Date: Fri, 10 Sep 2021 08:37:04 +0800 Subject: [PATCH] PageView resize from zero-size viewport should not lose state (#65015) --- .../flutter/lib/src/widgets/page_view.dart | 37 ++++++-- .../flutter/test/widgets/page_view_test.dart | 85 +++++++++++++++++++ 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/packages/flutter/lib/src/widgets/page_view.dart b/packages/flutter/lib/src/widgets/page_view.dart index dc778e5a996..58a5c87af5c 100644 --- a/packages/flutter/lib/src/widgets/page_view.dart +++ b/packages/flutter/lib/src/widgets/page_view.dart @@ -193,6 +193,11 @@ class PageController extends ScrollController { required Curve curve, }) { final _PagePosition position = this.position as _PagePosition; + if (position._cachedPage != null) { + position._cachedPage = page.toDouble(); + return Future.value(); + } + return position.animateTo( position.getPixelsFromPage(page.toDouble()), duration: duration, @@ -206,6 +211,11 @@ class PageController extends ScrollController { /// without animation, and without checking if the new value is in range. void jumpToPage(int page) { final _PagePosition position = this.position as _PagePosition; + if (position._cachedPage != null) { + position._cachedPage = page.toDouble(); + return; + } + position.jumpTo(position.getPixelsFromPage(page.toDouble())); } @@ -325,6 +335,10 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri final int initialPage; double _pageToUseOnStartup; + // When the viewport has a zero-size, the `page` can not + // be retrieved by `getPageFromPixels`, so we need to cache the page + // for use when resizing the viewport to non-zero next time. + double? _cachedPage; @override Future ensureVisible( @@ -370,7 +384,8 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri double get _initialPageOffset => math.max(0, viewportDimension * (viewportFraction - 1) / 2); double getPageFromPixels(double pixels, double viewportDimension) { - final double actual = math.max(0.0, pixels - _initialPageOffset) / math.max(1.0, viewportDimension * viewportFraction); + assert(viewportDimension > 0.0); + final double actual = math.max(0.0, pixels - _initialPageOffset) / (viewportDimension * viewportFraction); final double round = actual.roundToDouble(); if ((actual - round).abs() < precisionErrorTolerance) { return round; @@ -390,12 +405,12 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri ); return !hasPixels || !hasContentDimensions ? null - : getPageFromPixels(pixels.clamp(minScrollExtent, maxScrollExtent), viewportDimension); + : _cachedPage ?? getPageFromPixels(pixels.clamp(minScrollExtent, maxScrollExtent), viewportDimension); } @override void saveScrollOffset() { - PageStorage.of(context.storageContext)?.writeState(context.storageContext, getPageFromPixels(pixels, viewportDimension)); + PageStorage.of(context.storageContext)?.writeState(context.storageContext, _cachedPage ?? getPageFromPixels(pixels, viewportDimension)); } @override @@ -409,7 +424,7 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri @override void saveOffset() { - context.saveOffset(getPageFromPixels(pixels, viewportDimension)); + context.saveOffset(_cachedPage ?? getPageFromPixels(pixels, viewportDimension)); } @override @@ -431,9 +446,21 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri } final bool result = super.applyViewportDimension(viewportDimension); final double? oldPixels = hasPixels ? pixels : null; - final double page = (oldPixels == null || oldViewportDimensions == 0.0) ? _pageToUseOnStartup : getPageFromPixels(oldPixels, oldViewportDimensions!); + double page; + if (oldPixels == null) { + page = _pageToUseOnStartup; + } else if (oldViewportDimensions == 0.0) { + // If resize from zero, we should use the _cachedPage to recover the state. + page = _cachedPage!; + } else { + page = getPageFromPixels(oldPixels, oldViewportDimensions!); + } final double newPixels = getPixelsFromPage(page); + // If the viewportDimension is zero, cache the page + // in case the viewport is resized to be non-zero. + _cachedPage = (viewportDimension == 0.0) ? page : null; + if (newPixels != oldPixels) { correctPixels(newPixels); return false; diff --git a/packages/flutter/test/widgets/page_view_test.dart b/packages/flutter/test/widgets/page_view_test.dart index 1da4e486ed9..de7ab628c5b 100644 --- a/packages/flutter/test/widgets/page_view_test.dart +++ b/packages/flutter/test/widgets/page_view_test.dart @@ -14,6 +14,91 @@ import 'states.dart'; const Duration _frameDuration = Duration(milliseconds: 100); void main() { + testWidgets('PageView resize from zero-size viewport should not lose state', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/88956 + final PageController controller = PageController( + initialPage: 1, + ); + + Widget build(Size size) { + return Directionality( + textDirection: TextDirection.ltr, + child: Center( + child: SizedBox.fromSize( + size: size, + child: PageView( + children: kStates.map((String state) => Text(state)).toList(), + controller: controller, + onPageChanged: (int page) { }, + ), + ), + ), + ); + } + + // The pageView have a zero viewport, so nothing display. + await tester.pumpWidget(build(Size.zero)); + expect(find.text('Alabama'), findsNothing); + expect(find.text('Alabama', skipOffstage: false), findsOneWidget); + + // Resize from zero viewport to non-zero, the controller's initialPage 1 will display. + await tester.pumpWidget(build(const Size(200.0, 200.0))); + expect(find.text('Alaska'), findsOneWidget); + + // Jump to page 'Iowa'. + controller.jumpToPage(kStates.indexOf('Iowa')); + await tester.pump(); + expect(find.text('Iowa'), findsOneWidget); + + // Resize to zero viewport again, nothing display. + await tester.pumpWidget(build(Size.zero)); + expect(find.text('Iowa'), findsNothing); + + // Resize from zero to non-zero, the pageView should not lose state, so the page 'Iowa' show again. + await tester.pumpWidget(build(const Size(200.0, 200.0))); + expect(find.text('Iowa'), findsOneWidget); + }); + + testWidgets('Change the page through the controller when zero-size viewport', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/88956 + final PageController controller = PageController( + initialPage: 1, + ); + + Widget build(Size size) { + return Directionality( + textDirection: TextDirection.ltr, + child: Center( + child: SizedBox.fromSize( + size: size, + child: PageView( + children: kStates.map((String state) => Text(state)).toList(), + controller: controller, + onPageChanged: (int page) { }, + ), + ), + ), + ); + } + + // The pageView have a zero viewport, so nothing display. + await tester.pumpWidget(build(Size.zero)); + expect(find.text('Alabama'), findsNothing); + expect(find.text('Alabama', skipOffstage: false), findsOneWidget); + + // Change the page through the page controller when zero viewport + controller.animateToPage(kStates.indexOf('Iowa'), duration: kTabScrollDuration, curve: Curves.ease); + expect(controller.page, kStates.indexOf('Iowa')); + + controller.jumpToPage(kStates.indexOf('Illinois')); + expect(controller.page, kStates.indexOf('Illinois')); + + // Resize from zero viewport to non-zero, the latest state should not lost. + await tester.pumpWidget(build(const Size(200.0, 200.0))); + expect(controller.page, kStates.indexOf('Illinois')); + expect(find.text('Illinois'), findsOneWidget); + }); + testWidgets('PageController cannot return page while unattached', (WidgetTester tester) async { final PageController controller = PageController();