Interactable ScrollView content when settling a scroll activity (#145848)

Currently when the user: - flings a scrollable - overscrolls, and the scrollable is trying to settle - applies `DrivenScrollActivity` - swipes from one tab to the next All inputs are discarded or forwarded directly to the Scrollable widget (scroll activity). This leads to situations like these: https://github.com/flutter/flutter/assets/12874766/51b7876f-5a91-4a86-aa21-c72f0b2c4263 https://github.com/flutter/flutter/assets/12874766/2f756a45-5e42-47d7-98a0-12f071d34e7c https://github.com/flutter/flutter/assets/12874766/5eb998a1-b3b8-42a1-8b04-543f68823c2b Which leads to poor experience on iOS. The native behavior of iOS is to allow touches while a scrollable is settling: https://github.com/flutter/flutter/assets/12874766/e1ae61f8-d59c-40ae-a4c4-ad919f0dc6bf This PR alters the `shouldIgnoreTouches` of `BallisticScrollAvtivity` and `DrivenScrollActivity` to not make the child of the scrollable ignore touches. Fixes #145330 Currently tests that test tap to stop are not working as the taps now register when they should not. Because there is no distinction between flings inside and flings that go out of range.
2024-07-03 01:01:44 +02:00 · 2024-07-03 01:01:44 +02:00 · 46030f1eff
commit 46030f1eff
parent ce0e5c4330
5 changed files with 545 additions and 5 deletions
--- a/packages/flutter/lib/src/widgets/nested_scroll_view.dart
+++ b/packages/flutter/lib/src/widgets/nested_scroll_view.dart
@ -633,6 +633,10 @@ class _NestedScrollCoordinator implements ScrollActivityDelegate, ScrollHoldCont
  late _NestedScrollController _outerController;
  late _NestedScrollController _innerController;

+  bool get outOfRange {
+    return (_outerPosition?.outOfRange ?? false) || _innerPositions.any((_NestedScrollPosition position) => position.outOfRange);
+  }
+
  _NestedScrollPosition? get _outerPosition {
    if (!_outerController.hasClients) {
      return null;
@ -1415,6 +1419,7 @@ class _NestedScrollPosition extends ScrollPosition implements ScrollActivityDele
    if (simulation == null) {
      return IdleScrollActivity(this);
    }
+
    switch (mode) {
      case _NestedBallisticScrollActivityMode.outer:
        assert(metrics != null);
@ -1427,7 +1432,7 @@ class _NestedScrollPosition extends ScrollPosition implements ScrollActivityDele
          metrics,
          simulation,
          context.vsync,
-          activity?.shouldIgnorePointer ?? true,
+          shouldIgnorePointer,
        );
      case _NestedBallisticScrollActivityMode.inner:
        return _NestedInnerBallisticScrollActivity(
@ -1435,10 +1440,15 @@ class _NestedScrollPosition extends ScrollPosition implements ScrollActivityDele
          this,
          simulation,
          context.vsync,
-          activity?.shouldIgnorePointer ?? true,
+          shouldIgnorePointer,
        );
      case _NestedBallisticScrollActivityMode.independent:
-        return BallisticScrollActivity(this, simulation, context.vsync, activity?.shouldIgnorePointer ?? true);
+        return BallisticScrollActivity(
+          this,
+          simulation,
+          context.vsync,
+          shouldIgnorePointer
+        );
    }
  }

--- a/packages/flutter/lib/src/widgets/scroll_position.dart
+++ b/packages/flutter/lib/src/widgets/scroll_position.dart
@ -270,6 +270,15 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
  bool get haveDimensions => _haveDimensions;
  bool _haveDimensions = false;

+  /// Whether scrollables should absorb pointer events at this position.
+  ///
+  /// This is value relates to the current [ScrollActivity], which determines
+  /// if additional touch input should be received by the scroll view or its children.
+  /// If the position is overscrolled, as is allowed by [BouncingScrollPhysics],
+  /// children of the scroll view will receive pointer events as the scroll view
+  /// settles back from the overscrolled state.
+  bool get shouldIgnorePointer => !outOfRange && (activity?.shouldIgnorePointer ?? true);
+
  /// Take any current applicable state from the given [ScrollPosition].
  ///
  /// This method is called by the constructor if it is given an `oldPosition`.
@ -363,6 +372,9 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
      final double oldPixels = pixels;
      _pixels = newPixels - overscroll;
      if (_pixels != oldPixels) {
+        if (outOfRange) {
+          context.setIgnorePointer(false);
+        }
        notifyListeners();
        didUpdateScrollPositionBy(pixels - oldPixels);
      }
--- a/packages/flutter/lib/src/widgets/scroll_position_with_single_context.dart
+++ b/packages/flutter/lib/src/widgets/scroll_position_with_single_context.dart
@ -147,7 +147,7 @@ class ScrollPositionWithSingleContext extends ScrollPosition implements ScrollAc
        this,
        simulation,
        context.vsync,
-        activity?.shouldIgnorePointer ?? true,
+        shouldIgnorePointer,
      ));
    } else {
      goIdle();
--- a/packages/flutter/test/widgets/list_view_test.dart
+++ b/packages/flutter/test/widgets/list_view_test.dart
@ -545,6 +545,221 @@ void main() {
    expect(find.byType(Viewport), paints..clipRect());
  });

+  testWidgets('ListView allows touch on children when reaching an edge and over-scrolling / settling', (WidgetTester tester) async {
+    bool tapped = false;
+    final ScrollController controller = ScrollController();
+    addTearDown(controller.dispose);
+
+    const Duration frame = Duration(milliseconds: 16);
+
+    await tester.pumpWidget(
+      Directionality(
+        textDirection: TextDirection.ltr,
+        child: ListView.builder(
+          controller: controller,
+          physics: const BouncingScrollPhysics(),
+          itemCount: 15,
+          itemBuilder: (BuildContext context, int index) {
+            return GestureDetector(
+              onTap: () {
+                tapped = true;
+              },
+              child: SizedBox(
+                height: 100.0,
+                child: Text('Item $index'),
+              ),
+            );
+          },
+        ),
+      ),
+    );
+
+    // Tapping on an item in an idle scrollable should register the tap
+    await tester.tap(find.text('Item 0'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.fling(find.byType(ListView), const Offset(0.0, 80.0), 1000.0);
+    // Pump a few frames to ensure the scrollable is in an over-scrolled state
+    for (int i = 0; i < 5; i++) {
+      await tester.pump(frame);
+    }
+
+    expect(controller.offset, lessThan(0.0));
+
+    // Tapping on an item in an over-scrolled state should register the tap
+    await tester.tap(find.text('Item 1'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.pumpAndSettle();
+    expect(controller.offset, 0.0);
+
+    // Tapping on an item in an idle scrollable should register the tap
+    await tester.tap(find.text('Item 2'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    // Jump somewhere in the middle of the list
+    controller.jumpTo(101.0);
+    expect(controller.offset, equals(101.0));
+
+    await tester.tap(find.text('Item 3'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.pumpAndSettle();
+
+    // Strong fling down, to over-scroll the list at the top
+    await tester.fling(find.byType(ListView), const Offset(0.0, 500.0), 5000.0);
+
+    for (int i = 0; i < 5; i++) {
+      await tester.pump(frame);
+    }
+
+    // Ensure the scrollable is over-scrolled
+    expect(controller.offset, lessThan(0.0));
+
+    // Now we are settling, all taps should be registered
+    await tester.tap(find.text('Item 2'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.pump(frame);
+
+    await tester.tap(find.text('Item 2'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.pumpAndSettle();
+
+    await tester.tap(find.text('Item 2'));
+    expect(tapped, isTrue);
+    tapped = false;
+  });
+
+  testWidgets('ListView absorbs touch to stop scrolling when not at the edge', (WidgetTester tester) async {
+    bool tapped = false;
+    final ScrollController controller = ScrollController();
+    addTearDown(controller.dispose);
+
+    const Duration frame = Duration(milliseconds: 16);
+
+    await tester.pumpWidget(
+      Directionality(
+        textDirection: TextDirection.ltr,
+        child: ListView.builder(
+          controller: controller,
+          physics: const BouncingScrollPhysics(),
+          itemCount: 15,
+          itemBuilder: (BuildContext context, int index) {
+            return GestureDetector(
+              onTap: () {
+                tapped = true;
+              },
+              child: SizedBox(
+                height: 100.0,
+                child: Text('Item $index'),
+              ),
+            );
+          },
+        ),
+      ),
+    );
+
+    // Jump somewhere in the middle of the list
+    controller.jumpTo(101.0);
+    expect(controller.offset, equals(101.0));
+
+    // Tap on an item, it should register the tap
+    await tester.tap(find.text('Item 3'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    // Fling the list, it should start scrolling. Bot not to the edge
+    await tester.fling(find.byType(ListView), const Offset(0.0, 100.0), 1000.0);
+
+    await tester.pump(frame);
+
+    final double offset = controller.offset;
+
+    // Ensure we are somewhere between 0 and the starting offset
+    expect(controller.offset, lessThan(101.0));
+    expect(controller.offset, greaterThan(0.0));
+
+    await tester.tap(find.text('Item 2'), warnIfMissed: false); // The tap should be absorbed by the ListView. Therefore warnIfMissed is set to false
+    expect(tapped, isFalse);
+
+    // Ensure the scrollable stops in place and doesn't scroll further
+    await tester.pump(frame);
+    expect(offset, equals(controller.offset));
+    await tester.pumpAndSettle();
+    expect(offset, equals(controller.offset));
+
+    // Tapping on an item should register the tap normally, as the scrollable is idle
+    await tester.tap(find.text('Item 2'));
+    expect(tapped, isTrue);
+    tapped = false;
+  });
+
+  testWidgets('Horizontal ListView, when over-scrolled at the end allows touches on children', (WidgetTester tester) async {
+    bool tapped = false;
+    final ScrollController controller = ScrollController();
+    addTearDown(controller.dispose);
+
+    const Duration frame = Duration(milliseconds: 16);
+
+    await tester.pumpWidget(
+      Directionality(
+        textDirection: TextDirection.ltr,
+        child: ListView.builder(
+          itemExtent: 100.0,
+          controller: controller,
+          physics: const BouncingScrollPhysics(),
+          scrollDirection: Axis.horizontal,
+          itemCount: 15,
+          itemBuilder: (BuildContext context, int index) {
+            return GestureDetector(
+              onTap: () {
+                tapped = true;
+              },
+              child: SizedBox(
+                width: 100.0,
+                child: Text('Item $index'),
+              ),
+            );
+          },
+        ),
+      ),
+    );
+
+    // Tap on an item, it should register the tap
+    await tester.tap(find.text('Item 3'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    // Fling the list, it should start scrolling
+    await tester.fling(find.byType(ListView), const Offset(-500.0, 0.0), 10000.0);
+
+    for (int i = 0; i < 5; i++) {
+      await tester.pump(frame);
+    }
+
+    // Ensure the scrollable is over-scrolled at the end
+    expect(controller.offset, greaterThan(controller.position.maxScrollExtent));
+
+    // Tap on an item, it should register the tap
+    await tester.tap(find.text('Item 14'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.pumpAndSettle();
+
+    // Tap on an item, it should register the tap
+    await tester.tap(find.text('Item 14'));
+    expect(tapped, isTrue);
+  });
+
  testWidgets('ListView does not clips if no overflow', (WidgetTester tester) async {
    await tester.pumpWidget(
      Directionality(
--- a/packages/flutter/test/widgets/nested_scroll_view_test.dart
+++ b/packages/flutter/test/widgets/nested_scroll_view_test.dart
@ -325,6 +325,309 @@ void main() {
    expect(inner.offset, 0.0);
  });

+  testWidgets('NestedScrollView allows taps on children while over-scrolled to the top', (WidgetTester tester) async {
+    final Key innerKey = UniqueKey();
+    final GlobalKey<NestedScrollViewState> outerKey = GlobalKey();
+
+    final ScrollController outerController = ScrollController();
+    addTearDown(outerController.dispose);
+
+    const Duration frame = Duration(milliseconds: 16);
+    bool tapped = false;
+
+    Widget build() {
+      return Directionality(
+        textDirection: TextDirection.ltr,
+        child: Scaffold(
+          body: NestedScrollView(
+            key: outerKey,
+            controller: outerController,
+            physics: const BouncingScrollPhysics(),
+            headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) => <Widget>[
+              SliverToBoxAdapter(
+                child: Container(color: Colors.green, height: 300),
+              ),
+              SliverOverlapAbsorber(
+                handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context),
+                sliver: SliverToBoxAdapter(
+                  child: Container(
+                    color: Colors.blue,
+                    height: 64,
+                  ),
+                ),
+              ),
+            ],
+            body: ListView.builder(
+              key: innerKey,
+              physics: const BouncingScrollPhysics(),
+              itemCount: 15,
+              itemBuilder: (BuildContext context, int index) {
+                return ListTile(
+                  title: Text('Item $index'),
+                  onTap: () {
+                    tapped = true;
+                  },
+                );
+              },
+            ),
+          ),
+        ),
+      );
+    }
+
+    await tester.pumpWidget(build());
+
+    final ScrollController outer = outerKey.currentState!.outerController;
+    final ScrollController inner = outerKey.currentState!.innerController;
+
+    // Assert the initial positions
+    expect(outer.offset, 0.0);
+    expect(inner.offset, 0.0);
+
+    // Over-scroll the inner Scrollable to the top
+    await tester.fling(find.byKey(innerKey), const Offset(0, 200), 2000);
+
+    for (int i = 0; i < 5; i++) {
+      await tester.pump(frame);
+    }
+
+    // Ensure the inner Scrollable is over-scrolled
+    expect(inner.offset, lessThan(0.0));
+
+    // Tap on the first item in the ListView
+    await tester.tap(find.text('Item 0'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.pump(frame);
+
+    await tester.tap(find.text('Item 1'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.pumpAndSettle();
+
+    await tester.tap(find.text('Item 0'));
+    expect(tapped, isTrue);
+    tapped = false;
+  });
+
+  testWidgets('NestedScrollView absorbs touch to stop scrolling when not at the edge', (WidgetTester tester) async {
+    final Key innerKey = UniqueKey();
+    final GlobalKey<NestedScrollViewState> outerKey = GlobalKey();
+
+    final ScrollController outerController = ScrollController();
+    addTearDown(outerController.dispose);
+
+    const Duration frame = Duration(milliseconds: 16);
+    bool tapped = false;
+
+    Widget build() {
+      return Directionality(
+        textDirection: TextDirection.ltr,
+        child: Scaffold(
+          body: NestedScrollView(
+            key: outerKey,
+            controller: outerController,
+            physics: const BouncingScrollPhysics(),
+            headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) => <Widget>[
+              SliverToBoxAdapter(
+                child: Container(color: Colors.green, height: 300),
+              ),
+              SliverOverlapAbsorber(
+                handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context),
+                sliver: SliverToBoxAdapter(
+                  child: Container(
+                    color: Colors.blue,
+                    height: 64,
+                  ),
+                ),
+              ),
+            ],
+            body: ListView.builder(
+              key: innerKey,
+              physics: const BouncingScrollPhysics(),
+              itemExtent: 56,
+              itemCount: 15,
+              itemBuilder: (BuildContext context, int index) {
+                return ListTile(
+                  title: Text('Item $index'),
+                  onTap: () {
+                    tapped = true;
+                  },
+                );
+              },
+            ),
+          ),
+        ),
+      );
+    }
+
+    await tester.pumpWidget(build());
+
+    final ScrollController outer = outerKey.currentState!.outerController;
+    final ScrollController inner = outerKey.currentState!.innerController;
+
+    // Assert the initial positions
+    expect(outer.offset, 0.0);
+    expect(inner.offset, 0.0);
+
+    // Fling to somewhere in the middle of the outer Scrollable
+    await tester.fling(find.byKey(innerKey), const Offset(0, -200), 2000);
+
+    for (int i = 0; i < 3; i++) {
+      await tester.pump(frame);
+    }
+
+    // Ensure we are not at the edge
+    expect(outer.offset, greaterThan(0.0));
+    expect(outer.offset, lessThan(outer.position.maxScrollExtent));
+    final double offset = outer.offset;
+
+    // Tap on the first item in the ListView
+    await tester.tap(find.text('Item 2'), warnIfMissed: false);
+    expect(tapped, isFalse);
+
+    await tester.pump(frame);
+
+    // Ensure the outer Scrollable is not moving
+    expect(offset, equals(outer.offset));
+
+    await tester.tap(find.text('Item 2'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    await tester.pumpAndSettle();
+
+    await tester.tap(find.text('Item 2'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    // Fling the scrollable further
+    await tester.fling(find.byKey(innerKey), const Offset(0, -200), 2000);
+
+    for (int i = 0; i < 3; i++) {
+      await tester.pump(frame);
+    }
+
+    // Ensure the outer Scrollable is at edge
+    expect(outer.offset, equals(outer.position.maxScrollExtent));
+    // Ensure the inner Scrollable is not over-scrolled yet
+    expect(inner.offset, lessThan(inner.position.maxScrollExtent));
+
+    final double innerOffset = inner.offset;
+
+    // Tap on an item near the end of the ListView
+    await tester.tap(find.text('Item 10'), warnIfMissed: false);
+    expect(tapped, isFalse);
+
+    await tester.pump(frame);
+
+    // Ensure the inner Scrollable is not moving
+    expect(innerOffset, equals(inner.offset));
+
+    // Tapping on an item should register the tap normally, as the scrollable is idle
+    await tester.tap(find.text('Item 10'));
+    expect(tapped, isTrue);
+  });
+
+  testWidgets('NestedScrollView when over-scrolled at the end allows touches on children', (WidgetTester tester) async {
+    final Key innerKey = UniqueKey();
+    final GlobalKey<NestedScrollViewState> outerKey = GlobalKey();
+
+    final ScrollController outerController = ScrollController();
+    addTearDown(outerController.dispose);
+
+    const Duration frame = Duration(milliseconds: 16);
+    bool tapped = false;
+
+    Widget build() {
+      return Directionality(
+        textDirection: TextDirection.ltr,
+        child: Scaffold(
+          body: NestedScrollView(
+            key: outerKey,
+            controller: outerController,
+            physics: const BouncingScrollPhysics(),
+            headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) => <Widget>[
+              SliverToBoxAdapter(
+                child: Container(color: Colors.green, height: 300),
+              ),
+              SliverOverlapAbsorber(
+                handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context),
+                sliver: SliverToBoxAdapter(
+                  child: Container(
+                    color: Colors.blue,
+                    height: 64,
+                  ),
+                ),
+              ),
+            ],
+            body: ListView.builder(
+              key: innerKey,
+              physics: const BouncingScrollPhysics(),
+              itemExtent: 56,
+              itemCount: 15,
+              itemBuilder: (BuildContext context, int index) {
+                return ListTile(
+                  title: Text('Item $index'),
+                  onTap: () {
+                    tapped = true;
+                  },
+                );
+              },
+            ),
+          ),
+        ),
+      );
+    }
+
+    await tester.pumpWidget(build());
+
+    final ScrollController outer = outerKey.currentState!.outerController;
+    final ScrollController inner = outerKey.currentState!.innerController;
+
+    // Assert the initial positions
+    expect(outer.offset, 0.0);
+    expect(inner.offset, 0.0);
+
+    // Fling to somewhere in the middle of the outer Scrollable
+    await tester.fling(find.byKey(innerKey), const Offset(0, -2000), 2000);
+
+    for (int i = 0; i < 10; i++) {
+      await tester.pump(frame);
+    }
+
+    // Ensure the outer Scrollable is at edge
+    expect(outer.offset, equals(outer.position.maxScrollExtent));
+    // Ensure the inner Scrollable is over-scrolled
+    expect(inner.offset, greaterThan(inner.position.maxScrollExtent));
+
+    // Tap on an item near the end of the ListView
+    await tester.tap(find.text('Item 14'));
+    expect(tapped, isTrue);
+    tapped = false;
+
+    double settleOffset = inner.offset;
+
+    for (int i = 0; i < 5; i++) {
+      await tester.pump(frame);
+      await tester.pump(frame); // Pump a second frame to ensure the Scrollable has a chance to move
+
+      await tester.tap(find.text('Item 14'));
+      expect(tapped, isTrue);
+      tapped = false;
+      // Ensure the inner Scrollable is settling
+      expect(settleOffset, greaterThan(inner.offset));
+      settleOffset = inner.offset;
+    }
+
+    await tester.pumpAndSettle();
+
+    await tester.tap(find.text('Item 14'));
+    expect(tapped, isTrue);
+  });
+
  testWidgets('NestedScrollView overscroll and release and hold', (WidgetTester tester) async {
    await tester.pumpWidget(buildTest());
    expect(find.text('aaa2'), findsOneWidget);
@ -2457,7 +2760,7 @@ void main() {

      // Tap after releasing the overscroll to trigger secondary inner ballistic
      // scroll activity with 0 velocity.
-      await tester.tap(find.text('Item 49'), warnIfMissed: false);
+      await tester.tap(find.text('Item 49'));
      await tester.pumpAndSettle();

      // If handled correctly, the ballistic scroll activity should finish