From 84f5b1cb09ea8c485fe11da686b5ceb7dfcf2660 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 2 Jul 2018 17:20:26 -0700 Subject: [PATCH] Round properly to nearest page in TabBarView. (#16868) This logic is described in the test as looking for a scroll ending very close to a new page, but in fact its behavior is more like "very close to a page to the right": if we're not very, very close to any page, it will pick the page to the left, not an old page. There's no reason this should be left-right asymmetrical. Instead, pick the nearest page. In practice, the case where this makes a difference never arises when the scroll runs undisturbed to completion; but when the user taps on the page to hold or drag, the scroll will be interrupted before it gets within tolerance of a particular page, and this case does arise. This fixes a glitch that is hard to trigger without time dilation, but is quite conspicuous with it: * Open a tab view with at least 4 tabs, e.g. the Buttons screen of the gallery (with "Animate Slowly" on.) * Starting at tab 0, tap tab 2. * When the animation is nearly complete, tap the page a couple of times, as if to drag it around to scroll. Then let the page view settle ballistically toward page 2. * Before it finishes, tap tab 3. * Suddenly page 1 fills the view, replacing page 2, before we scroll from there to page 3. With this fix, the animation in the last step moves smoothly from where we are when it starts onward to page 3. --- packages/flutter/lib/src/material/tabs.dart | 5 +--- packages/flutter/test/material/tabs_test.dart | 28 +++++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/flutter/lib/src/material/tabs.dart b/packages/flutter/lib/src/material/tabs.dart index d89d440821..ceb07f705a 100644 --- a/packages/flutter/lib/src/material/tabs.dart +++ b/packages/flutter/lib/src/material/tabs.dart @@ -1150,10 +1150,7 @@ class _TabBarViewState extends State { } _controller.offset = (_pageController.page - _controller.index).clamp(-1.0, 1.0); } else if (notification is ScrollEndNotification) { - final ScrollPosition position = _pageController.position; - final double pageTolerance = position.physics.tolerance.distance - / (position.viewportDimension * _pageController.viewportFraction); - _controller.index = (_pageController.page + pageTolerance).floor(); + _controller.index = _pageController.page.round(); _currentIndex = _controller.index; } _warpUnderwayCount -= 1; diff --git a/packages/flutter/test/material/tabs_test.dart b/packages/flutter/test/material/tabs_test.dart index 0960c66451..c56a9ef694 100644 --- a/packages/flutter/test/material/tabs_test.dart +++ b/packages/flutter/test/material/tabs_test.dart @@ -804,7 +804,7 @@ void main() { await tester.pump(const Duration(milliseconds: 300)); }); - testWidgets('TabBarView scrolls end very VERY close to a new page', (WidgetTester tester) async { + testWidgets('TabBarView scrolls end close to a new page', (WidgetTester tester) async { // This is a regression test for https://github.com/flutter/flutter/issues/9375 final TabController tabController = new TabController( @@ -845,15 +845,23 @@ void main() { expect(position.pixels, 400.0); // Not close enough to switch to page 2 - pageController.jumpTo(800.0 - 1.25 * position.physics.tolerance.distance); + pageController.jumpTo(500.0); expect(tabController.index, 1); // Close enough to switch to page 2 - pageController.jumpTo(800.0 - 0.75 * position.physics.tolerance.distance); + pageController.jumpTo(700.0); expect(tabController.index, 2); + + // Same behavior going left: not left enough to get to page 0 + pageController.jumpTo(300.0); + expect(tabController.index, 1); + + // Left enough to get to page 0 + pageController.jumpTo(100.0); + expect(tabController.index, 0); }); - testWidgets('TabBarView scrolls end very close to a new page with custom physics', (WidgetTester tester) async { + testWidgets('TabBarView scrolls end close to a new page with custom physics', (WidgetTester tester) async { final TabController tabController = new TabController( vsync: const TestVSync(), initialIndex: 1, @@ -893,12 +901,20 @@ void main() { expect(position.pixels, 400.0); // Not close enough to switch to page 2 - pageController.jumpTo(800.0 - 1.25 * position.physics.tolerance.distance); + pageController.jumpTo(500.0); expect(tabController.index, 1); // Close enough to switch to page 2 - pageController.jumpTo(800.0 - 0.75 * position.physics.tolerance.distance); + pageController.jumpTo(700.0); expect(tabController.index, 2); + + // Same behavior going left: not left enough to get to page 0 + pageController.jumpTo(300.0); + expect(tabController.index, 1); + + // Left enough to get to page 0 + pageController.jumpTo(100.0); + expect(tabController.index, 0); }); testWidgets('Scrollable TabBar with a non-zero TabController initialIndex', (WidgetTester tester) async {