diff --git a/packages/flutter/lib/src/widgets/scroll_physics.dart b/packages/flutter/lib/src/widgets/scroll_physics.dart index 1e9b9901234..d67c23649c0 100644 --- a/packages/flutter/lib/src/widgets/scroll_physics.dart +++ b/packages/flutter/lib/src/widgets/scroll_physics.dart @@ -412,6 +412,45 @@ class ScrollPhysics { /// These physics should be combined with other scroll physics, e.g. /// [BouncingScrollPhysics] or [ClampingScrollPhysics], to obtain a complete /// description of typical scroll physics. See [applyTo]. +/// +/// ## Implementation details +/// +/// Specifically, these physics perform two adjustments. +/// +/// The first is to maintain overscroll when the position is out of range. +/// +/// The second is to enforce the boundary when the position is in range. +/// +/// If the current velocity is zero, neither adjustment is made. The +/// assumption is that there is an ongoing animation and therefore +/// further changing the scroll position would disrupt the experience. +/// +/// If the extents haven't changed, then the overscroll adjustment is +/// not made. The assumption is that if the position is overscrolled, +/// it is intentional, otherwise the position could not have reached +/// that position. (Consider [ClampingScrollPhysics] vs +/// [BouncingScrollPhysics] for example.) +/// +/// If the position itself changed since the last animation frame, +/// then the overscroll is not maintained. The assumption is similar +/// to the previous case: the position would not have been placed out +/// of range unless it was intentional. +/// +/// In addition, if the position changed and the boundaries were and +/// still are finite, then the boundary isn't enforced either, for +/// the same reason. However, if any of the boundaries were or are +/// now infinite, the boundary _is_ enforced, on the assumption that +/// infinite boundaries indicate a lazy-loading scroll view, which +/// cannot enforce boundaries while the full list has not loaded. +/// +/// If the range was out of range, then the boundary is not enforced +/// even if the range is not maintained. If the range is maintained, +/// then the distance between the old position and the old boundary is +/// applied to the new boundary to obtain the new position. +/// +/// If the range was in range, and the boundary is to be enforced, +/// then the new position is obtained by deferring to the other physics, +/// if any, and then clamped to the new range. class RangeMaintainingScrollPhysics extends ScrollPhysics { /// Creates scroll physics that maintain the scroll position in range. const RangeMaintainingScrollPhysics({ ScrollPhysics parent }) : super(parent: parent); @@ -428,18 +467,60 @@ class RangeMaintainingScrollPhysics extends ScrollPhysics { @required bool isScrolling, @required double velocity, }) { - if (velocity != 0.0 || ((oldPosition.minScrollExtent == newPosition.minScrollExtent) && (oldPosition.maxScrollExtent == newPosition.maxScrollExtent))) { - return super.adjustPositionForNewDimensions(oldPosition: oldPosition, newPosition: newPosition, isScrolling: isScrolling, velocity: velocity); + bool maintainOverscroll = true; + bool enforceBoundary = true; + if (velocity != 0.0) { + // Don't try to adjust an animating position, the jumping around + // would be distracting. + maintainOverscroll = false; + enforceBoundary = false; } - if (oldPosition.pixels < oldPosition.minScrollExtent) { - final double oldDelta = oldPosition.minScrollExtent - oldPosition.pixels; - return newPosition.minScrollExtent - oldDelta; + if ((oldPosition.minScrollExtent == newPosition.minScrollExtent) && + (oldPosition.maxScrollExtent == newPosition.maxScrollExtent)) { + // If the extents haven't changed then ignore overscroll. + maintainOverscroll = false; } - if (oldPosition.pixels > oldPosition.maxScrollExtent) { - final double oldDelta = oldPosition.pixels - oldPosition.maxScrollExtent; - return newPosition.maxScrollExtent + oldDelta; + if (oldPosition.pixels != newPosition.pixels) { + // If the position has been changed already, then it might have + // been adjusted to expect new overscroll, so don't try to + // maintain the relative overscroll. + maintainOverscroll = false; + if (oldPosition.minScrollExtent.isFinite && oldPosition.maxScrollExtent.isFinite && + newPosition.minScrollExtent.isFinite && newPosition.maxScrollExtent.isFinite) { + // In addition, if the position changed then we only enforce + // the new boundary if the previous boundary was not entirely + // finite. A common case where the position changes while one + // of the extents is infinite is a lazily-loaded list. (If the + // boundaries were finite, and the position changed, then we + // assume it was intentional.) + enforceBoundary = false; + } } - return newPosition.pixels.clamp(newPosition.minScrollExtent, newPosition.maxScrollExtent) as double; + if ((oldPosition.pixels < oldPosition.minScrollExtent) && + (oldPosition.pixels > oldPosition.maxScrollExtent)) { + // If the old position was out of range, then we should + // not try to keep the new position in range. + enforceBoundary = false; + } + if (maintainOverscroll) { + // Force the new position to be no more out of range + // than it was before, if it was overscrolled. + if (oldPosition.pixels < oldPosition.minScrollExtent) { + final double oldDelta = oldPosition.minScrollExtent - oldPosition.pixels; + return newPosition.minScrollExtent - oldDelta; + } + if (oldPosition.pixels > oldPosition.maxScrollExtent) { + final double oldDelta = oldPosition.pixels - oldPosition.maxScrollExtent; + return newPosition.maxScrollExtent + oldDelta; + } + } + // If we're not forcing the overscroll, defer to other physics. + double result = super.adjustPositionForNewDimensions(oldPosition: oldPosition, newPosition: newPosition, isScrolling: isScrolling, velocity: velocity); + if (enforceBoundary) { + // ...but if they put us out of range then reinforce the boundary. + result = result.clamp(newPosition.minScrollExtent, newPosition.maxScrollExtent) as double; + } + return result; } } diff --git a/packages/flutter/lib/src/widgets/scroll_position.dart b/packages/flutter/lib/src/widgets/scroll_position.dart index f58d83ab2d4..cb8088fed95 100644 --- a/packages/flutter/lib/src/widgets/scroll_position.dart +++ b/packages/flutter/lib/src/widgets/scroll_position.dart @@ -226,7 +226,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { /// If there is any overscroll, it is reported using [didOverscrollBy]. double setPixels(double newPixels) { assert(_pixels != null); - assert(SchedulerBinding.instance.schedulerPhase.index <= SchedulerPhase.transientCallbacks.index); + assert(SchedulerBinding.instance.schedulerPhase != SchedulerPhase.persistentCallbacks, 'A scrollable\'s position should not change during the build, layout, and paint phases, otherwise the rendering will be confused.'); if (newPixels != pixels) { final double overscroll = applyBoundaryConditions(newPixels); assert(() { @@ -471,27 +471,37 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { return true; } + bool _pendingDimensions = false; + ScrollMetrics _lastMetrics; + @override bool applyContentDimensions(double minScrollExtent, double maxScrollExtent) { assert(minScrollExtent != null); assert(maxScrollExtent != null); + assert(haveDimensions == (_lastMetrics != null)); if (!nearEqual(_minScrollExtent, minScrollExtent, Tolerance.defaultTolerance.distance) || !nearEqual(_maxScrollExtent, maxScrollExtent, Tolerance.defaultTolerance.distance) || _didChangeViewportDimensionOrReceiveCorrection) { assert(minScrollExtent != null); assert(maxScrollExtent != null); assert(minScrollExtent <= maxScrollExtent); - final ScrollMetrics oldPosition = haveDimensions ? copyWith() : null; _minScrollExtent = minScrollExtent; _maxScrollExtent = maxScrollExtent; - final ScrollMetrics newPosition = haveDimensions ? copyWith() : null; + final ScrollMetrics currentMetrics = haveDimensions ? copyWith() : null; _didChangeViewportDimensionOrReceiveCorrection = false; - if (haveDimensions && !correctForNewDimensions(oldPosition, newPosition)) + _pendingDimensions = true; + if (haveDimensions && !correctForNewDimensions(_lastMetrics, currentMetrics)) { return false; + } _haveDimensions = true; + } + assert(haveDimensions); + if (_pendingDimensions) { applyNewDimensions(); + _pendingDimensions = false; } assert(!_didChangeViewportDimensionOrReceiveCorrection, 'Use correctForNewDimensions() (and return true) to change the scroll offset during applyContentDimensions().'); + _lastMetrics = copyWith(); return true; } @@ -546,6 +556,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { @mustCallSuper void applyNewDimensions() { assert(pixels != null); + assert(_pendingDimensions); activity.applyNewDimensions(); _updateSemanticActions(); // will potentially request a semantics update. } diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index 7a7790a71b6..72b735e0f71 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -692,7 +692,7 @@ class ScrollableState extends State with TickerProviderStateMixin, R key: _scrollSemanticsKey, child: result, position: position, - allowImplicitScrolling: widget?.physics?.allowImplicitScrolling ?? _physics.allowImplicitScrolling, + allowImplicitScrolling: _physics.allowImplicitScrolling, semanticChildCount: widget.semanticChildCount, ); } @@ -704,6 +704,7 @@ class ScrollableState extends State with TickerProviderStateMixin, R void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); properties.add(DiagnosticsProperty('position', position)); + properties.add(DiagnosticsProperty('effective physics', _physics)); } @override @@ -971,7 +972,7 @@ class ScrollAction extends Action { assert(state.position.viewportDimension != null); assert(state.position.maxScrollExtent != null); assert(state.position.minScrollExtent != null); - assert(state.widget.physics == null || state.widget.physics.shouldAcceptUserOffset(state.position)); + assert(state._physics == null || state._physics.shouldAcceptUserOffset(state.position)); if (state.widget.incrementCalculator != null) { return state.widget.incrementCalculator( ScrollIncrementDetails( @@ -1060,7 +1061,7 @@ class ScrollAction extends Action { assert(state.position.minScrollExtent != null); // Don't do anything if the user isn't allowed to scroll. - if (state.widget.physics != null && !state.widget.physics.shouldAcceptUserOffset(state.position)) { + if (state._physics != null && !state._physics.shouldAcceptUserOffset(state.position)) { return; } final double increment = _getIncrement(state, intent); diff --git a/packages/flutter/test/widgets/range_maintaining_scroll_physics_test.dart b/packages/flutter/test/widgets/range_maintaining_scroll_physics_test.dart index 8beabd87f24..256e87a0e46 100644 --- a/packages/flutter/test/widgets/range_maintaining_scroll_physics_test.dart +++ b/packages/flutter/test/widgets/range_maintaining_scroll_physics_test.dart @@ -219,4 +219,45 @@ void main() { expect(position.maxScrollExtent, 100.0); expect(position.pixels, 0.0); }); + + testWidgets('expanding page views', (WidgetTester tester) async { + await tester.pumpWidget(Padding(padding: const EdgeInsets.only(right: 200.0), child: TabBarDemo())); + await tester.tap(find.text('bike')); + await tester.pump(); + await tester.pump(const Duration(seconds: 1)); + final Rect bike1 = tester.getRect(find.byIcon(Icons.directions_bike)); + await tester.pumpWidget(Padding(padding: EdgeInsets.zero, child: TabBarDemo())); + final Rect bike2 = tester.getRect(find.byIcon(Icons.directions_bike)); + expect(bike2.center, bike1.shift(const Offset(100.0, 0.0)).center); + }); } + +class TabBarDemo extends StatelessWidget { + @override + Widget build(BuildContext context) { + return MaterialApp( + home: DefaultTabController( + length: 3, + child: Scaffold( + appBar: AppBar( + bottom: const TabBar( + tabs: [ + Tab(text: 'car'), + Tab(text: 'transit'), + Tab(text: 'bike'), + ], + ), + title: const Text('Tabs Demo'), + ), + body: const TabBarView( + children: [ + Icon(Icons.directions_car), + Icon(Icons.directions_transit), + Icon(Icons.directions_bike), + ], + ), + ), + ), + ); + } +} \ No newline at end of file diff --git a/packages/flutter/test/widgets/scroll_activity_test.dart b/packages/flutter/test/widgets/scroll_activity_test.dart new file mode 100644 index 00000000000..9d05801e9c9 --- /dev/null +++ b/packages/flutter/test/widgets/scroll_activity_test.dart @@ -0,0 +1,264 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// @dart = 2.8 + +import 'package:flutter/material.dart'; +import 'package:flutter/scheduler.dart'; +import 'package:flutter_test/flutter_test.dart'; + +List children(int n) { + return List.generate(n, (int i) { + return Container(height: 100.0, child: Text('$i')); + }); +} + +void main() { + testWidgets('Scrolling with list view changes', (WidgetTester tester) async { + final ScrollController controller = ScrollController(); + await tester.pumpWidget(MaterialApp(home: ListView(children: children(30), controller: controller))); + final double thirty = controller.position.maxScrollExtent; + controller.jumpTo(thirty); + await tester.pump(); + controller.jumpTo(thirty + 100.0); // past the end + await tester.pump(); + await tester.pumpWidget(MaterialApp(home: ListView(children: children(31), controller: controller))); + expect(controller.position.pixels, thirty + 200.0); // same distance past the end + expect(await tester.pumpAndSettle(), 7); // now it goes ballistic... + expect(controller.position.pixels, thirty + 100.0); // and ends up at the end + }); + + testWidgets('Ability to keep a PageView at the end manually (issue 62209)', (WidgetTester tester) async { + await tester.pumpWidget(const MaterialApp(home: PageView62209())); + expect(find.text('Page 1'), findsOneWidget); + expect(find.text('Page 100'), findsNothing); + await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 2'), findsOneWidget); + expect(find.text('Page 100'), findsNothing); + await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 3'), findsOneWidget); + expect(find.text('Page 100'), findsNothing); + await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 4'), findsOneWidget); + expect(find.text('Page 100'), findsNothing); + await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 5'), findsOneWidget); + expect(find.text('Page 100'), findsNothing); + await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 5'), findsNothing); + expect(find.text('Page 100'), findsOneWidget); + await tester.tap(find.byType(FlatButton)); // 6 + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 6'), findsNothing); + expect(find.text('Page 5'), findsNothing); + expect(find.text('Page 100'), findsOneWidget); + await tester.tap(find.byType(FlatButton)); // 7 + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 6'), findsNothing); + expect(find.text('Page 7'), findsNothing); + expect(find.text('Page 5'), findsNothing); + expect(find.text('Page 100'), findsOneWidget); + await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 5'), findsOneWidget); + expect(find.text('Page 100'), findsNothing); + await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 4'), findsOneWidget); + expect(find.text('Page 5'), findsNothing); + expect(find.text('Page 100'), findsNothing); + await tester.tap(find.byType(FlatButton)); // 8 + await tester.pump(); + expect(find.text('Page 1'), findsNothing); + expect(find.text('Page 8'), findsNothing); + expect(find.text('Page 4'), findsOneWidget); + expect(find.text('Page 5'), findsNothing); + expect(find.text('Page 100'), findsNothing); + await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 3'), findsOneWidget); + await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 2'), findsOneWidget); + await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 6'), findsOneWidget); + await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 7'), findsOneWidget); + await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 8'), findsOneWidget); + await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 1'), findsOneWidget); + await tester.tap(find.byType(FlatButton)); // 9 + await tester.pump(); + expect(find.text('Page 1'), findsOneWidget); + expect(find.text('Page 9'), findsNothing); + await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0)); + await tester.pump(); + expect(find.text('Page 9'), findsOneWidget); + }); +} + +class PageView62209 extends StatefulWidget { + const PageView62209(); + + @override + _PageView62209State createState() => _PageView62209State(); +} + +class _PageView62209State extends State { + int _nextPageNum = 1; + final List _pages = []; + + @override + void initState() { + super.initState(); + for (int i = 0; i < 5; i++) { + _pages.add(Carousel62209Page( + key: Key('$_nextPageNum'), + number: _nextPageNum++, + )); + } + _pages.add(const Carousel62209Page(number: 100)); + } + + @override + Widget build(BuildContext context) { + return Scaffold( + body: Column( + children: [ + Expanded(child: Carousel62209(pages: _pages)), + FlatButton( + child: const Text('ADD PAGE'), + onPressed: () { + setState(() { + _pages.insert( + 1, + Carousel62209Page( + key: Key('$_nextPageNum'), + number: _nextPageNum++, + ), + ); + }); + }, + ) + ], + ), + ); + } +} + +class Carousel62209Page extends StatelessWidget { + const Carousel62209Page({this.number, Key key}) : super(key: key); + + final int number; + + @override + Widget build(BuildContext context) { + return Center(child: Text('Page $number')); + } +} + +class Carousel62209 extends StatefulWidget { + const Carousel62209({Key key, this.pages}) : super(key: key); + + final List pages; + + @override + _Carousel62209State createState() => _Carousel62209State(); +} + +class _Carousel62209State extends State { + // page variables + PageController _pageController; + int _currentPage = 0; + + // controls updates outside of user interaction + List _pages; + bool _jumpingToPage = false; + + @override + void initState() { + super.initState(); + _pages = widget.pages.toList(); + _pageController = PageController(initialPage: 0, keepPage: false); + } + + @override + void didUpdateWidget(Carousel62209 oldWidget) { + super.didUpdateWidget(oldWidget); + if (!_jumpingToPage) { + int newPage = -1; + for (int i = 0; i < widget.pages.length; i++) { + if (widget.pages[i].number == _pages[_currentPage].number) { + newPage = i; + } + } + if (newPage == _currentPage) { + _pages = widget.pages.toList(); + } else { + _jumpingToPage = true; + SchedulerBinding.instance.addPostFrameCallback((_) { + if (mounted) { + setState(() { + _pages = widget.pages.toList(); + _currentPage = newPage; + _pageController.jumpToPage(_currentPage); + SchedulerBinding.instance.addPostFrameCallback((_) { + _jumpingToPage = false; + }); + }); + } + }); + } + } + } + + @override + void dispose() { + _pageController.dispose(); + super.dispose(); + } + + bool _handleScrollNotification(ScrollNotification notification) { + if (notification is ScrollUpdateNotification) { + final int page = _pageController.page.round(); + if (!_jumpingToPage && _currentPage != page) { + _currentPage = page; + } + } + return true; + } + + @override + Widget build(BuildContext context) { + return NotificationListener( + onNotification: _handleScrollNotification, + child: PageView.builder( + controller: _pageController, + itemCount: _pages.length, + itemBuilder: (BuildContext context, int index) { + return _pages[index]; + }, + ), + ); + } +}