From 47dd7639d277602826da37f0bc7c4873d7ea453a Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 29 Apr 2025 18:53:15 -0700 Subject: [PATCH] Let DrivenScrollActivity subclasses customize handling of overscroll (#166731) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This functionality is useful in a custom DrivenScrollActivity subclass, just like it's useful in a BallisticScrollActivity subclass. For example, if one calls `animateTo(0, …)` to scroll to the beginning of a list, there will be no overscroll (provided the chosen curve doesn't go beyond its endpoint). The same is true if one calls `animateTo(position.maxScrollExtent, …)` to scroll to the end... as long as the value of maxScrollExtent at the beginning remains accurate when the scroll view reaches the end. If one wants to scroll to the end and avoid overscroll even when that turns out to be closer than the initial estimate found in maxScrollExtent, then that calls for customizing the overscroll behavior of a DrivenScrollActivity. Fortunately we don't need to invent any new API in order to do this: BallisticScrollActivity already has an API for the exact same need. So copy that API -- name, docs, and all -- to keep these two classes aligned. (It might also be useful to take much of what these have in common and factor that out to a common subclass; but that'd be for another PR.) ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../lib/src/widgets/scroll_activity.dart | 14 +++- .../test/widgets/scroll_activity_test.dart | 84 +++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/packages/flutter/lib/src/widgets/scroll_activity.dart b/packages/flutter/lib/src/widgets/scroll_activity.dart index c66f4e4c5f0..f5072d72ab0 100644 --- a/packages/flutter/lib/src/widgets/scroll_activity.dart +++ b/packages/flutter/lib/src/widgets/scroll_activity.dart @@ -748,11 +748,23 @@ class DrivenScrollActivity extends ScrollActivity { Future get done => _completer.future; void _tick() { - if (delegate.setPixels(_controller.value) != 0.0) { + if (!applyMoveTo(_controller.value)) { delegate.goIdle(); } } + /// Move the position to the given location. + /// + /// If the new position was fully applied, returns true. If there was any + /// overflow, returns false. + /// + /// The default implementation calls [ScrollActivityDelegate.setPixels] + /// and returns true if the overflow was zero. + @protected + bool applyMoveTo(double value) { + return delegate.setPixels(value).abs() < precisionErrorTolerance; + } + void _end() { // Check if the activity was disposed before going ballistic because _end might be called // if _controller is disposed just after completion. diff --git a/packages/flutter/test/widgets/scroll_activity_test.dart b/packages/flutter/test/widgets/scroll_activity_test.dart index d6325b86f5a..1404260bdeb 100644 --- a/packages/flutter/test/widgets/scroll_activity_test.dart +++ b/packages/flutter/test/widgets/scroll_activity_test.dart @@ -61,6 +61,61 @@ void main() { expect(controller.position.pixels, thirty + 100.0); // and ends up at the end }); + testWidgets('DrivenScrollActivity allows overriding applyMoveTo', (WidgetTester tester) async { + final ScrollController controller = ScrollController(); + addTearDown(controller.dispose); + final List notifications = []; + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: NotificationListener( + onNotification: (ScrollNotification notif) { + if (notif is OverscrollNotification) { + notifications.add(notif); + } + return false; + }, + child: ListView(controller: controller, children: children(10)), + ), + ), + ); + final ScrollPositionWithSingleContext position = + controller.position as ScrollPositionWithSingleContext; + final double end = position.maxScrollExtent; + + position.beginActivity( + DrivenScrollActivity( + position, + from: 0, + to: end + 10, + duration: const Duration(milliseconds: 100), + curve: Curves.linear, + vsync: position.context.vsync, + ), + ); + await tester.pumpAndSettle(); + // The base DrivenScrollActivity caused overscroll. + expect(notifications, hasLength(1)); + + notifications.clear(); + controller.jumpTo(0); + await tester.pump(); + + position.beginActivity( + _NoOverscrollDrivenScrollActivity( + position, + from: 0, + to: end + 10, + duration: const Duration(milliseconds: 100), + curve: Curves.linear, + vsync: position.context.vsync, + ), + ); + await tester.pumpAndSettle(); + // The _NoOverscrollDrivenScrollActivity avoided overscroll. + expect(notifications, isEmpty); + }); + testWidgets('Ability to keep a PageView at the end manually (issue 62209)', ( WidgetTester tester, ) async { @@ -440,6 +495,35 @@ class _Carousel62209State extends State { } } +class _NoOverscrollDrivenScrollActivity extends DrivenScrollActivity { + _NoOverscrollDrivenScrollActivity( + ScrollPositionWithSingleContext super.delegate, { + required super.from, + required super.to, + required super.duration, + required super.curve, + required super.vsync, + }); + + ScrollPosition get _position => delegate as ScrollPosition; + + @override + bool applyMoveTo(double value) { + bool done = false; + if (velocity >= 0.0 && value > _position.maxScrollExtent) { + value = _position.maxScrollExtent; + done = true; + } else if (velocity <= 0.0 && value < _position.minScrollExtent) { + value = _position.minScrollExtent; + done = true; + } + if (!super.applyMoveTo(value)) { + return false; + } + return !done; + } +} + class _ScrollActivity extends ScrollActivity { _ScrollActivity(super.delegate);