Fix `StretchingOverscrollIndicator` not handling directional changes correctly (#116548)

* Fix overscroll behaviour of `StretchingOverscrollIndicator` on directional change while scrolling

* Remove trailing spaces

* Change naming of `_StretchDirection` values to `trailing` and `leading`

* Remove trailing space

* Apply suggestions from code review

* Fix stretching overscroll indicator direction on fling

* Fix issue when changing direction while recede animation is playing

* Remove trailing space

* Add test for changing direction during recede animation

AUTHORS

@@ -102,3 +102,4 @@ Tomasz Gucio <tgucio@gmail.com>
 Jason C.H <ctrysbita@outlook.com>
 Hubert Jóźwiak <hjozwiakdx@gmail.com>
 Eli Albert <crasowas@gmail.com>
+Jan Kuß <jan@kuss.dev>

packages/flutter/lib/src/widgets/overscroll_indicator.dart

@@ -601,6 +601,15 @@ class _GlowingOverscrollIndicatorPainter extends CustomPainter {
   }
 }
 
+enum _StretchDirection {
+  /// The [trailing] direction indicates that the content will be stretched toward
+  /// the trailing edge.
+  trailing,
+  /// The [leading] direction indicates that the content will be stretched toward
+  /// the leading edge.
+  leading,
+}
+
 /// A Material Design visual indication that a scroll view has overscrolled.
 ///
 /// A [StretchingOverscrollIndicator] listens for [ScrollNotification]s in order
@@ -689,6 +698,9 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
   late final _StretchController _stretchController = _StretchController(vsync: this);
   ScrollNotification? _lastNotification;
   OverscrollNotification? _lastOverscrollNotification;
+
+  double _totalOverscroll = 0.0;
+
   bool _accepted = true;
 
   bool _handleScrollNotification(ScrollNotification notification) {
@@ -706,9 +718,11 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
 
       assert(notification.metrics.axis == widget.axis);
       if (_accepted) {
+        _totalOverscroll += notification.overscroll;
+
         if (notification.velocity != 0.0) {
           assert(notification.dragDetails == null);
-          _stretchController.absorbImpact(notification.velocity.abs());
+          _stretchController.absorbImpact(notification.velocity.abs(), _totalOverscroll);
         } else {
           assert(notification.overscroll != 0.0);
           if (notification.dragDetails != null) {
@@ -716,38 +730,40 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
             // which is the furthest distance a single pointer could pull on the
             // screen. This is because more than one pointer will multiply the
             // amount of overscroll - https://github.com/flutter/flutter/issues/11884
+
             final double viewportDimension = notification.metrics.viewportDimension;
-            final double distanceForPull =
-              (notification.overscroll.abs() / viewportDimension) + _stretchController.pullDistance;
+            final double distanceForPull = _totalOverscroll.abs() / viewportDimension;
             final double clampedOverscroll = clampDouble(distanceForPull, 0, 1.0);
-            _stretchController.pull(clampedOverscroll);
+            _stretchController.pull(clampedOverscroll, _totalOverscroll);
           }
         }
       }
     } else if (notification is ScrollEndNotification || notification is ScrollUpdateNotification) {
+      // Since the overscrolling ended, we reset the total overscroll amount.
+      _totalOverscroll = 0;
       _stretchController.scrollEnd();
     }
     _lastNotification = notification;
     return false;
   }
 
-  AlignmentGeometry _getAlignmentForAxisDirection(double overscroll) {
+  AlignmentGeometry _getAlignmentForAxisDirection(_StretchDirection stretchDirection) {
     // Accounts for reversed scrollables by checking the AxisDirection
     switch (widget.axisDirection) {
       case AxisDirection.up:
-        return overscroll > 0
+        return stretchDirection == _StretchDirection.trailing
             ? AlignmentDirectional.topCenter
             : AlignmentDirectional.bottomCenter;
       case AxisDirection.right:
-        return overscroll > 0
+        return stretchDirection == _StretchDirection.trailing
             ? Alignment.centerRight
             : Alignment.centerLeft;
       case AxisDirection.down:
-        return overscroll > 0
+        return stretchDirection == _StretchDirection.trailing
             ? AlignmentDirectional.bottomCenter
             : AlignmentDirectional.topCenter;
       case AxisDirection.left:
-        return overscroll > 0
+        return stretchDirection == _StretchDirection.trailing
             ? Alignment.centerLeft
             : Alignment.centerRight;
     }
@@ -784,7 +800,7 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
           }
 
           final AlignmentGeometry alignment = _getAlignmentForAxisDirection(
-            _lastOverscrollNotification?.overscroll ?? 0.0
+            _stretchController.stretchDirection,
           );
 
           final double viewportDimension = _lastOverscrollNotification?.metrics.viewportDimension ?? mainAxisSize;
@@ -836,6 +852,9 @@ class _StretchController extends ChangeNotifier {
   double get pullDistance => _pullDistance;
   double _pullDistance = 0.0;
 
+  _StretchDirection get stretchDirection => _stretchDirection;
+  _StretchDirection _stretchDirection = _StretchDirection.trailing;
+
   // Constants from Android.
   static const double _exponentialScalar = math.e / 0.33;
   static const double _stretchIntensity = 0.016;
@@ -847,7 +866,7 @@ class _StretchController extends ChangeNotifier {
   /// Handle a fling to the edge of the viewport at a particular velocity.
   ///
   /// The velocity must be positive.
-  void absorbImpact(double velocity) {
+  void absorbImpact(double velocity, double totalOverscroll) {
     assert(velocity >= 0.0);
     velocity = clampDouble(velocity, 1, 10000);
     _stretchSizeTween.begin = _stretchSize.value;
@@ -855,6 +874,7 @@ class _StretchController extends ChangeNotifier {
     _stretchController.duration = Duration(milliseconds: (velocity * 0.02).round());
     _stretchController.forward(from: 0.0);
     _state = _StretchState.absorb;
+    _stretchDirection = totalOverscroll > 0 ? _StretchDirection.trailing : _StretchDirection.leading;
   }
 
   /// Handle a user-driven overscroll.
@@ -862,8 +882,19 @@ class _StretchController extends ChangeNotifier {
   /// The `normalizedOverscroll` argument should be the absolute value of the
   /// scroll distance in logical pixels, divided by the extent of the viewport
   /// in the main axis.
-  void pull(double normalizedOverscroll) {
+  void pull(double normalizedOverscroll, double totalOverscroll) {
     assert(normalizedOverscroll >= 0.0);
+
+    final _StretchDirection newStretchDirection = totalOverscroll > 0 ? _StretchDirection.trailing : _StretchDirection.leading;
+    if (_stretchDirection != newStretchDirection && _state == _StretchState.recede) {
+      // When the stretch direction changes while we are in the recede state, we need to ignore the change.
+      // If we don't, the stretch will instantly jump to the new direction with the recede animation still playing, which causes
+      // a unwanted visual abnormality (https://github.com/flutter/flutter/pull/116548#issuecomment-1414872567).
+      // By ignoring the directional change until the recede state is finished, we can avoid this.
+      return;
+    }
+
+    _stretchDirection = newStretchDirection;
     _pullDistance = normalizedOverscroll;
     _stretchSizeTween.begin = _stretchSize.value;
     final double linearIntensity =_stretchIntensity * _pullDistance;

packages/flutter/test/widgets/overscroll_stretch_indicator_test.dart

@@ -20,6 +20,9 @@ void main() {
     Axis axis = Axis.vertical,
     bool reverse = false,
     TextDirection textDirection = TextDirection.ltr,
+    double boxHeight = 250.0,
+    double boxWidth = 300.0,
+    ScrollPhysics? physics,
   }) {
     final AxisDirection axisDirection;
     switch (axis) {
@@ -44,6 +47,7 @@ void main() {
           child: StretchingOverscrollIndicator(
             axisDirection: axisDirection,
             child: CustomScrollView(
+              physics: physics,
               reverse: reverse,
               scrollDirection: axis,
               controller: controller,
@@ -51,20 +55,20 @@ void main() {
                 SliverToBoxAdapter(child: Container(
                   color: const Color(0xD0FF0000),
                   key: box1Key,
-                  height: 250.0,
-                  width: 300.0,
+                  height: boxHeight,
+                  width: boxWidth,
                 )),
                 SliverToBoxAdapter(child: Container(
                   color: const Color(0xFFFFFF00),
                   key: box2Key,
-                  height: 250.0,
-                  width: 300.0,
+                  height: boxHeight,
+                  width: boxWidth,
                 )),
                 SliverToBoxAdapter(child: Container(
                   color: const Color(0xFF6200EA),
                   key: box3Key,
-                  height: 250.0,
-                  width: 300.0,
+                  height: boxHeight,
+                  width: boxWidth,
                 )),
               ],
             ),
@@ -387,7 +391,7 @@ void main() {
     expect(box3.localToGlobal(Offset.zero).dx, 500.0);
   });
 
-  testWidgets('Stretch overscroll horizontally RTl', (WidgetTester tester) async {
+  testWidgets('Stretch overscroll horizontally RTL', (WidgetTester tester) async {
     final GlobalKey box1Key = GlobalKey();
     final GlobalKey box2Key = GlobalKey();
     final GlobalKey box3Key = GlobalKey();
@@ -701,7 +705,7 @@ void main() {
     await tester.pumpAndSettle();
   });
 
-  testWidgets('Multiple pointers wll not exceed stretch limit', (WidgetTester tester) async {
+  testWidgets('Multiple pointers will not exceed stretch limit', (WidgetTester tester) async {
     // Regression test for https://github.com/flutter/flutter/issues/99264
     await tester.pumpWidget(
       Directionality(
@@ -770,4 +774,297 @@ void main() {
     await pointer4.up();
     await tester.pumpAndSettle();
   });
+
+  testWidgets('Stretch overscroll vertically, change direction mid scroll', (WidgetTester tester) async {
+    final GlobalKey box1Key = GlobalKey();
+    final GlobalKey box2Key = GlobalKey();
+    final GlobalKey box3Key = GlobalKey();
+    final ScrollController controller = ScrollController();
+    await tester.pumpWidget(
+      buildTest(
+        box1Key,
+        box2Key,
+        box3Key,
+        controller,
+        // Setting the `boxHeight` to 100.0 will make the boxes fit in the
+        // scrollable viewport.
+        boxHeight: 100,
+        // To make the scroll view in the test still scrollable, we need to add
+        // the `AlwaysScrollableScrollPhysics`.
+        physics: const AlwaysScrollableScrollPhysics(),
+      ),
+    );
+
+    expect(find.byType(StretchingOverscrollIndicator), findsOneWidget);
+    expect(find.byType(GlowingOverscrollIndicator), findsNothing);
+    final RenderBox box1 = tester.renderObject(find.byKey(box1Key));
+    final RenderBox box2 = tester.renderObject(find.byKey(box2Key));
+    final RenderBox box3 = tester.renderObject(find.byKey(box3Key));
+
+    expect(controller.offset, 0.0);
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero), const Offset(0.0, 100.0));
+    expect(box3.localToGlobal(Offset.zero), const Offset(0.0, 200.0));
+
+    final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(CustomScrollView)));
+    // Overscroll the start
+    await gesture.moveBy(const Offset(0.0, 600.0));
+    await tester.pumpAndSettle();
+
+    // The boxes should now be at different locations because of the scaling.
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero).dy, greaterThan(103.0));
+    expect(box3.localToGlobal(Offset.zero).dy, greaterThan(206.0));
+
+    // Move the pointer up a miniscule amount to trigger a directional change.
+    await gesture.moveBy(const Offset(0.0, -20.0));
+    await tester.pumpAndSettle();
+
+    // The boxes should remain roughly at the same locations, since the pointer
+    // didn't move far.
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero).dy, greaterThan(103.0));
+    expect(box3.localToGlobal(Offset.zero).dy, greaterThan(206.0));
+
+    // Now make the pointer overscroll to the end
+    await gesture.moveBy(const Offset(0.0, -1200.0));
+    await tester.pumpAndSettle();
+
+    expect(box1.localToGlobal(Offset.zero).dy, lessThan(-19.0));
+    expect(box2.localToGlobal(Offset.zero).dy, lessThan(85.0));
+    expect(box3.localToGlobal(Offset.zero).dy, lessThan(188.0));
+
+    // Release the pointer
+    await gesture.up();
+    await tester.pumpAndSettle();
+
+    // Now the boxes should be back to their original locations.
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero), const Offset(0.0, 100.0));
+    expect(box3.localToGlobal(Offset.zero), const Offset(0.0, 200.0));
+  });
+
+  testWidgets('Stretch overscroll horizontally, change direction mid scroll', (WidgetTester tester) async {
+    final GlobalKey box1Key = GlobalKey();
+    final GlobalKey box2Key = GlobalKey();
+    final GlobalKey box3Key = GlobalKey();
+    final ScrollController controller = ScrollController();
+    await tester.pumpWidget(
+      buildTest(
+        box1Key,
+        box2Key,
+        box3Key,
+        controller,
+        // Setting the `boxWidth` to 100.0 will make the boxes fit in the
+        // scrollable viewport.
+        boxWidth: 100,
+        // To make the scroll view in the test still scrollable, we need to add
+        // the `AlwaysScrollableScrollPhysics`.
+        physics: const AlwaysScrollableScrollPhysics(),
+        axis: Axis.horizontal,
+      ),
+    );
+
+    expect(find.byType(StretchingOverscrollIndicator), findsOneWidget);
+    expect(find.byType(GlowingOverscrollIndicator), findsNothing);
+    final RenderBox box1 = tester.renderObject(find.byKey(box1Key));
+    final RenderBox box2 = tester.renderObject(find.byKey(box2Key));
+    final RenderBox box3 = tester.renderObject(find.byKey(box3Key));
+
+    expect(controller.offset, 0.0);
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero), const Offset(100.0, 0.0));
+    expect(box3.localToGlobal(Offset.zero), const Offset(200.0, 0.0));
+
+    final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(CustomScrollView)));
+    // Overscroll the start
+    await gesture.moveBy(const Offset(600.0, 0.0));
+    await tester.pumpAndSettle();
+
+    // The boxes should now be at different locations because of the scaling.
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero).dx, greaterThan(102.0));
+    expect(box3.localToGlobal(Offset.zero).dx, greaterThan(205.0));
+
+    // Move the pointer up a miniscule amount to trigger a directional change.
+    await gesture.moveBy(const Offset(-20.0, 0.0));
+    await tester.pumpAndSettle();
+
+    // The boxes should remain roughly at the same locations, since the pointer
+    // didn't move far.
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero).dx, greaterThan(102.0));
+    expect(box3.localToGlobal(Offset.zero).dx, greaterThan(205.0));
+
+    // Now make the pointer overscroll to the end
+    await gesture.moveBy(const Offset(-1200.0, 0.0));
+    await tester.pumpAndSettle();
+
+    expect(box1.localToGlobal(Offset.zero).dx, lessThan(-19.0));
+    expect(box2.localToGlobal(Offset.zero).dx, lessThan(85.0));
+    expect(box3.localToGlobal(Offset.zero).dx, lessThan(188.0));
+
+    // Release the pointer
+    await gesture.up();
+    await tester.pumpAndSettle();
+
+    // Now the boxes should be back to their original locations.
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero), const Offset(100.0, 0.0));
+    expect(box3.localToGlobal(Offset.zero), const Offset(200.0, 0.0));
+  });
+
+  testWidgets('Fling toward the trailing edge causes stretch toward the leading edge', (WidgetTester tester) async {
+    final GlobalKey box1Key = GlobalKey();
+    final GlobalKey box2Key = GlobalKey();
+    final GlobalKey box3Key = GlobalKey();
+    final ScrollController controller = ScrollController();
+    await tester.pumpWidget(
+      buildTest(box1Key, box2Key, box3Key, controller),
+    );
+
+    expect(find.byType(StretchingOverscrollIndicator), findsOneWidget);
+    expect(find.byType(GlowingOverscrollIndicator), findsNothing);
+    final RenderBox box1 = tester.renderObject(find.byKey(box1Key));
+    final RenderBox box2 = tester.renderObject(find.byKey(box2Key));
+    final RenderBox box3 = tester.renderObject(find.byKey(box3Key));
+
+    expect(controller.offset, 0.0);
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero), const Offset(0.0, 250.0));
+    expect(box3.localToGlobal(Offset.zero), const Offset(0.0, 500.0));
+
+    await tester.fling(find.byType(CustomScrollView), const Offset(0.0, -50.0), 10000.0);
+    await tester.pump(const Duration(milliseconds: 100));
+    await tester.pump(const Duration(milliseconds: 100));
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // The boxes should now be at different locations because of the scaling.
+    expect(controller.offset, 150.0);
+    expect(box1.localToGlobal(Offset.zero).dy, lessThan(-160.0));
+    expect(box2.localToGlobal(Offset.zero).dy, lessThan(93.0));
+    expect(box3.localToGlobal(Offset.zero).dy, lessThan(347.0));
+
+    await tester.pumpAndSettle();
+
+    // The boxes should now be at their final position.
+    expect(controller.offset, 150.0);
+    expect(box1.localToGlobal(Offset.zero).dy, -150.0);
+    expect(box2.localToGlobal(Offset.zero).dy, 100.0);
+    expect(box3.localToGlobal(Offset.zero).dy, 350.0);
+  });
+
+  testWidgets('Fling toward the leading edge causes stretch toward the trailing edge', (WidgetTester tester) async {
+    final GlobalKey box1Key = GlobalKey();
+    final GlobalKey box2Key = GlobalKey();
+    final GlobalKey box3Key = GlobalKey();
+    final ScrollController controller = ScrollController();
+    await tester.pumpWidget(
+      buildTest(box1Key, box2Key, box3Key, controller),
+    );
+
+    expect(find.byType(StretchingOverscrollIndicator), findsOneWidget);
+    expect(find.byType(GlowingOverscrollIndicator), findsNothing);
+    final RenderBox box1 = tester.renderObject(find.byKey(box1Key));
+    final RenderBox box2 = tester.renderObject(find.byKey(box2Key));
+    final RenderBox box3 = tester.renderObject(find.byKey(box3Key));
+
+    expect(controller.offset, 0.0);
+    expect(box1.localToGlobal(Offset.zero), Offset.zero);
+    expect(box2.localToGlobal(Offset.zero), const Offset(0.0, 250.0));
+    expect(box3.localToGlobal(Offset.zero), const Offset(0.0, 500.0));
+
+    // We fling to the trailing edge and let it settle.
+    await tester.fling(find.byType(CustomScrollView), const Offset(0.0, -50.0), 10000.0);
+    await tester.pumpAndSettle();
+
+    // We are now at the trailing edge
+    expect(controller.offset, 150.0);
+    expect(box1.localToGlobal(Offset.zero).dy, -150.0);
+    expect(box2.localToGlobal(Offset.zero).dy, 100.0);
+    expect(box3.localToGlobal(Offset.zero).dy, 350.0);
+
+    // Now fling to the leading edge
+    await tester.fling(find.byType(CustomScrollView), const Offset(0.0, 50.0), 10000.0);
+    await tester.pump(const Duration(milliseconds: 100));
+    await tester.pump(const Duration(milliseconds: 100));
+    await tester.pump(const Duration(milliseconds: 100));
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // The boxes should now be at different locations because of the scaling.
+    expect(controller.offset, 0.0);
+    expect(box1.localToGlobal(Offset.zero).dy, 0.0);
+    expect(box2.localToGlobal(Offset.zero).dy, greaterThan(254.0));
+    expect(box3.localToGlobal(Offset.zero).dy, greaterThan(508.0));
+
+    await tester.pumpAndSettle();
+
+    // The boxes should now be at their final position.
+    expect(controller.offset, 0.0);
+    expect(box1.localToGlobal(Offset.zero).dy, 0.0);
+    expect(box2.localToGlobal(Offset.zero).dy, 250.0);
+    expect(box3.localToGlobal(Offset.zero).dy, 500.0);
+  });
+
+  testWidgets('changing scroll direction during recede animation will not change the stretch direction', (WidgetTester tester) async {
+    final GlobalKey box1Key = GlobalKey();
+    final GlobalKey box2Key = GlobalKey();
+    final GlobalKey box3Key = GlobalKey();
+    final ScrollController controller = ScrollController();
+    await tester.pumpWidget(
+      buildTest(box1Key, box2Key, box3Key, controller, boxHeight: 205.0),
+    );
+
+    expect(find.byType(StretchingOverscrollIndicator), findsOneWidget);
+    expect(find.byType(GlowingOverscrollIndicator), findsNothing);
+    final RenderBox box1 = tester.renderObject(find.byKey(box1Key));
+    final RenderBox box2 = tester.renderObject(find.byKey(box2Key));
+    final RenderBox box3 = tester.renderObject(find.byKey(box3Key));
+
+    // Fling to the trailing edge
+    await tester.fling(find.byType(CustomScrollView), const Offset(0.0, -50.0), 10000.0);
+    await tester.pumpAndSettle();
+
+    expect(box1.localToGlobal(Offset.zero).dy, -15.0);
+    expect(box2.localToGlobal(Offset.zero).dy, 190.0);
+    expect(box3.localToGlobal(Offset.zero).dy, 395.0);
+
+    final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(CustomScrollView)));
+    // Overscroll to the trailing edge
+    await gesture.moveBy(const Offset(0.0, -200.0));
+    await tester.pumpAndSettle();
+
+    expect(box1.localToGlobal(Offset.zero).dy, lessThan(-25.0));
+    expect(box2.localToGlobal(Offset.zero).dy, lessThan(185.0));
+    expect(box3.localToGlobal(Offset.zero).dy, lessThan(392.0));
+
+    // This will trigger the recede animation
+    // The y offset of the boxes should be increasing, since the boxes were stretched
+    // toward the leading edge.
+    await gesture.moveBy(const Offset(0.0, 150.0));
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // Explicitly check that the box1 offset is not 0.0, since this would probably mean that
+    // the stretch direction is wrong.
+    expect(box1.localToGlobal(Offset.zero).dy, isNot(0.0));
+
+    expect(box1.localToGlobal(Offset.zero).dy, lessThan(-12.0));
+    expect(box2.localToGlobal(Offset.zero).dy, lessThan(197.0));
+    expect(box3.localToGlobal(Offset.zero).dy, lessThan(407.0));
+
+    await tester.pump(const Duration(milliseconds: 100));
+
+    expect(box1.localToGlobal(Offset.zero).dy, lessThan(-6.0));
+    expect(box2.localToGlobal(Offset.zero).dy, lessThan(201.0));
+    expect(box3.localToGlobal(Offset.zero).dy, lessThan(408.0));
+
+    await tester.pumpAndSettle();
+
+    // The recede animation is done now, we should now be at the leading edge.
+    expect(box1.localToGlobal(Offset.zero).dy, 0.0);
+    expect(box2.localToGlobal(Offset.zero).dy, 205.0);
+    expect(box3.localToGlobal(Offset.zero).dy, 410.0);
+
+    await gesture.up();
+  });
 }