Fixes PageView.onPageChanged to only report changes in range (#8553) * clamp the pixels used in the page calculation to be within min + max extents to avoid pages outside of the range of children * add test to check onPageChanged not fired on overscroll
diff --git a/packages/flutter/lib/src/widgets/page_view.dart b/packages/flutter/lib/src/widgets/page_view.dart index c737404..811d9f8 100644 --- a/packages/flutter/lib/src/widgets/page_view.dart +++ b/packages/flutter/lib/src/widgets/page_view.dart
@@ -176,7 +176,7 @@ return page * viewportDimension * viewportFraction; } - double get page => pixels == null ? null : getPageFromPixels(pixels, viewportDimension); + double get page => pixels == null ? null : getPageFromPixels(pixels.clamp(minScrollExtent, maxScrollExtent), viewportDimension); @override bool applyViewportDimension(double viewportDimension) {
diff --git a/packages/flutter/test/widgets/page_view_test.dart b/packages/flutter/test/widgets/page_view_test.dart index fb01565..8d57a77 100644 --- a/packages/flutter/test/widgets/page_view_test.dart +++ b/packages/flutter/test/widgets/page_view_test.dart
@@ -28,7 +28,7 @@ child: new Text(state), ), ); - }).toList() + }).toList(), )); await tester.tap(find.text('Alabama')); @@ -60,7 +60,8 @@ expect(log, equals(<String>['Alaska'])); log.clear(); - await tester.fling(find.byType(PageView), const Offset(-200.0, 0.0), 1000.0); + await tester.fling( + find.byType(PageView), const Offset(-200.0, 0.0), 1000.0); await tester.pumpUntilNoTransientCallbacks(_frameDuration); expect(find.text('Alabama'), findsNothing); @@ -75,7 +76,8 @@ expect(find.text('Arizona'), findsNothing); }); - testWidgets('PageView does not squish when overscrolled', (WidgetTester tester) async { + testWidgets('PageView does not squish when overscrolled', + (WidgetTester tester) async { await tester.pumpWidget(new MaterialApp( theme: new ThemeData(platform: TargetPlatform.iOS), home: new PageView( @@ -91,7 +93,8 @@ )); Size sizeOf(int i) => tester.getSize(find.byKey(new ValueKey<int>(i))); - double leftOf(int i) => tester.getTopLeft(find.byKey(new ValueKey<int>(i))).x; + double leftOf(int i) => + tester.getTopLeft(find.byKey(new ValueKey<int>(i))).x; expect(leftOf(0), equals(0.0)); expect(sizeOf(0), equals(const Size(800.0, 600.0))); @@ -118,15 +121,18 @@ height: 400.0, child: new PageView( controller: controller, - children: kStates.map<Widget>((String state) => new Text(state)).toList(), + children: + kStates.map<Widget>((String state) => new Text(state)).toList(), ), ), )); expect(find.text('California'), findsOneWidget); - controller.nextPage(duration: const Duration(milliseconds: 150), curve: Curves.ease); - await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100)); + controller.nextPage( + duration: const Duration(milliseconds: 150), curve: Curves.ease); + await tester + .pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100)); expect(find.text('Colorado'), findsOneWidget); @@ -136,15 +142,18 @@ height: 400.0, child: new PageView( controller: controller, - children: kStates.map<Widget>((String state) => new Text(state)).toList(), + children: + kStates.map<Widget>((String state) => new Text(state)).toList(), ), ), )); expect(find.text('Colorado'), findsOneWidget); - controller.previousPage(duration: const Duration(milliseconds: 150), curve: Curves.ease); - await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100)); + controller.previousPage( + duration: const Duration(milliseconds: 150), curve: Curves.ease); + await tester + .pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100)); expect(find.text('California'), findsOneWidget); }); @@ -155,7 +164,8 @@ width: 600.0, height: 400.0, child: new PageView( - children: kStates.map<Widget>((String state) => new Text(state)).toList(), + children: + kStates.map<Widget>((String state) => new Text(state)).toList(), ), ), )); @@ -163,7 +173,8 @@ expect(find.text('Alabama'), findsOneWidget); await tester.scroll(find.byType(PageView), const Offset(-1250.0, 0.0)); - await tester.pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100)); + await tester + .pumpUntilNoTransientCallbacks(const Duration(milliseconds: 100)); expect(find.text('Arizona'), findsOneWidget); @@ -172,7 +183,8 @@ width: 250.0, height: 100.0, child: new PageView( - children: kStates.map<Widget>((String state) => new Text(state)).toList(), + children: + kStates.map<Widget>((String state) => new Text(state)).toList(), ), ), )); @@ -184,7 +196,8 @@ width: 450.0, height: 400.0, child: new PageView( - children: kStates.map<Widget>((String state) => new Text(state)).toList(), + children: + kStates.map<Widget>((String state) => new Text(state)).toList(), ), ), )); @@ -198,7 +211,8 @@ width: 0.0, height: 0.0, child: new PageView( - children: kStates.map<Widget>((String state) => new Text(state)).toList(), + children: + kStates.map<Widget>((String state) => new Text(state)).toList(), ), ), )); @@ -210,7 +224,8 @@ width: 200.0, height: 200.0, child: new PageView( - children: kStates.map<Widget>((String state) => new Text(state)).toList(), + children: + kStates.map<Widget>((String state) => new Text(state)).toList(), ), ), )); @@ -221,13 +236,16 @@ testWidgets('Page changes at halfway point', (WidgetTester tester) async { final List<int> log = <int>[]; await tester.pumpWidget(new PageView( - onPageChanged: (int page) { log.add(page); }, + onPageChanged: (int page) { + log.add(page); + }, children: kStates.map<Widget>((String state) => new Text(state)).toList(), )); expect(log, isEmpty); - final TestGesture gesture = await tester.startGesture(const Point(100.0, 100.0)); + final TestGesture gesture = + await tester.startGesture(const Point(100.0, 100.0)); // The page view is 800.0 wide, so this move is just short of halfway. await gesture.moveBy(const Offset(-380.0, 0.0)); @@ -274,10 +292,12 @@ itemBuilder: (BuildContext context, int index) { return new Container( height: 200.0, - color: index % 2 == 0 ? const Color(0xFF0000FF) : const Color(0xFF00FF00), + color: index % 2 == 0 + ? const Color(0xFF0000FF) + : const Color(0xFF00FF00), child: new Text(kStates[index]), ); - } + }, ); } @@ -312,10 +332,12 @@ itemBuilder: (BuildContext context, int index) { return new Container( height: 200.0, - color: index % 2 == 0 ? const Color(0xFF0000FF) : const Color(0xFF00FF00), + color: index % 2 == 0 + ? const Color(0xFF0000FF) + : const Color(0xFF00FF00), child: new Text(kStates[index]), ); - } + }, ); } @@ -330,7 +352,8 @@ controller.jumpToPage(10); await tester.pump(); - expect(tester.getTopLeft(find.text('Connecticut')), const Point(-50.0, 0.0)); + expect( + tester.getTopLeft(find.text('Connecticut')), const Point(-50.0, 0.0)); expect(tester.getTopLeft(find.text('Delaware')), const Point(50.0, 0.0)); expect(tester.getTopLeft(find.text('Florida')), const Point(150.0, 0.0)); expect(tester.getTopLeft(find.text('Georgia')), const Point(250.0, 0.0)); @@ -342,7 +365,8 @@ }); testWidgets('PageView large viewportFraction', (WidgetTester tester) async { - final PageController controller = new PageController(viewportFraction: 5/4); + final PageController controller = + new PageController(viewportFraction: 5/4); Widget build(PageController controller) { return new PageView.builder( @@ -351,21 +375,48 @@ itemBuilder: (BuildContext context, int index) { return new Container( height: 200.0, - color: index % 2 == 0 ? const Color(0xFF0000FF) : const Color(0xFF00FF00), + color: index % 2 == 0 + ? const Color(0xFF0000FF) + : const Color(0xFF00FF00), child: new Text(kStates[index]), ); - } + }, ); } await tester.pumpWidget(build(controller)); expect(tester.getTopLeft(find.text('Alabama')), const Point(-100.0, 0.0)); - expect(tester.getBottomRight(find.text('Alabama')), const Point(900.0, 600.0)); + expect( + tester.getBottomRight(find.text('Alabama')), const Point(900.0, 600.0)); controller.jumpToPage(10); await tester.pump(); expect(tester.getTopLeft(find.text('Hawaii')), const Point(-100.0, 0.0)); }); + + testWidgets('PageView does not report page changed on overscroll', + (WidgetTester tester) async { + final PageController controller = + new PageController(initialPage: kStates.length - 1); + int changeIndex = 0; + Widget build() { + return new PageView( + children: + kStates.map<Widget>((String state) => new Text(state)).toList(), + controller: controller, + onPageChanged: (int page) { + changeIndex = page; + }, + ); + } + + await tester.pumpWidget(build()); + controller.jumpToPage(kStates.length * 2); // try to move beyond max range + // change index should be zero, shouldn't fire onPageChanged + expect(changeIndex, 0); + await tester.pump(); + expect(changeIndex, 0); + }); }