Fix: Momentum scrolling continues despite press and hold in list view (…

…#196)

## Fixes / Closes (optional)

Fixes #190.

## Description

This PR fixes an incorrect use of the
`ScrollActivity.shouldIgnorePointer` flag that has caused the issue.

Previously, `DragScrollDrivenSheetActivity` used to start a
`SheetContentBallisticScrollActivity` with `shouldIgnorePointer` set to
`false` when a drag gesture ended. However, it should actually be `true`
except in a special case. This is because setting
`ScrollActivity.shouldIgnorePointer` to `true` means that clickable
items in the scroll view cannot receive pointer events while performing
that activity, and the scroll view may receive them instead. So if the
flag is set to `false` during a ballistic scroll animation and there are
any clickable items in the scroll view, the scroll view may not be able
to receive the pointer events, resulting in the ballistic scroll
animation not stopping in response to user gestures.

After this PR is merged, `SheetContentBallisticScrollActivity` will be
started with the same value of `shouldIgnorePointer` as the previous
`ScrollActivity`. Note that we can't use a hardcoded value of `true`
here, as it may need to be `false` in a special case where the pointer
device kind is `PointerDeviceKind.trackpad`.

## Summary (check all that apply)

- [x] Modified / added code
- [x] Modified / added tests
- [ ] Modified / added examples
- [ ] Modified / added others (pubspec.yaml, workflows, etc...)
- [ ] Updated README
- [ ] Contains breaking changes
  - [ ] Created / updated migration guide
- [ ] Incremented version number
  - [ ] Updated CHANGELOG

package/lib/src/scrollable/scrollable_sheet_activity.dart

@@ -12,6 +12,18 @@ import 'scrollable_sheet_extent.dart';
 import 'sheet_content_scroll_activity.dart';
 import 'sheet_content_scroll_position.dart';
 
+/// A [SheetActivity] that is associated with a [SheetContentScrollPosition].
+///
+/// This activity is responsible for both scrolling a scrollable content
+/// in the sheet and dragging the sheet itself.
+///
+/// [shouldIgnorePointer] and [SheetContentScrollPosition.shouldIgnorePointer]
+/// of the associated scroll position may be synchronized, but not always.
+/// For example, [BallisticScrollDrivenSheetActivity]'s [shouldIgnorePointer]
+/// is always `false` while the associated scroll position sets it to `true`
+/// in most cases to ensure that the pointer events, which potentially
+/// interrupt the ballistic scroll animation, are not stolen by clickable
+/// items in the scroll view.
 @internal
 abstract class ScrollableSheetActivity
     extends SheetActivity<ScrollableSheetExtent> {
@@ -196,7 +208,6 @@ class DragScrollDrivenSheetActivity extends ScrollableSheetActivity
       ..didDragEnd(details)
       ..goBallisticWithScrollPosition(
         velocity: -1 * details.velocityY,
-        shouldIgnorePointer: false,
         scrollPosition: scrollPosition,
       );
   }
@@ -215,14 +226,10 @@ class BallisticScrollDrivenSheetActivity extends ScrollableSheetActivity
     super.scrollPosition, {
     required this.simulation,
     required double initialPixels,
-    required this.shouldIgnorePointer,
   }) : _oldPixels = initialPixels;
 
   final Simulation simulation;
 
-  @override
-  final bool shouldIgnorePointer;
-
   double _oldPixels;
 
   @override
@@ -271,7 +278,6 @@ class BallisticScrollDrivenSheetActivity extends ScrollableSheetActivity
   void _end() {
     owner.goBallisticWithScrollPosition(
       velocity: 0,
-      shouldIgnorePointer: shouldIgnorePointer,
       scrollPosition: scrollPosition,
     );
   }

package/lib/src/scrollable/scrollable_sheet_extent.dart

@@ -161,7 +161,6 @@ class ScrollableSheetExtent extends SheetExtent
   @override
   void goBallisticWithScrollPosition({
     required double velocity,
-    required bool shouldIgnorePointer,
     required SheetContentScrollPosition scrollPosition,
   }) {
     assert(metrics.hasDimensions);
@@ -196,13 +195,12 @@ class ScrollableSheetExtent extends SheetExtent
           scrollPosition,
           simulation: scrollSimulation,
           initialPixels: scrollPixelsForScrollPhysics,
-          shouldIgnorePointer: shouldIgnorePointer,
         ),
       );
       scrollPosition.beginActivity(
         SheetContentBallisticScrollActivity(
           delegate: scrollPosition,
-          shouldIgnorePointer: shouldIgnorePointer,
+          shouldIgnorePointer: scrollPosition.shouldIgnorePointer,
           getVelocity: () => activity.velocity,
         ),
       );

package/lib/src/scrollable/sheet_content_scroll_position.dart

@@ -32,7 +32,6 @@ abstract class SheetContentScrollPositionOwner {
 
   void goBallisticWithScrollPosition({
     required double velocity,
-    required bool shouldIgnorePointer,
     required SheetContentScrollPosition scrollPosition,
   });
 }
@@ -69,6 +68,10 @@ class SheetContentScrollPosition extends ScrollPositionWithSingleContext {
   double _heldPreviousVelocity = 0.0;
   double get heldPreviousVelocity => _heldPreviousVelocity;
 
+  /// Whether the scroll view should prevent its contents from receiving
+  /// pointer events.
+  bool get shouldIgnorePointer => activity!.shouldIgnorePointer;
+
   /// Sets the user scroll direction.
   ///
   /// This exists only to expose `updateUserScrollDirection`
@@ -123,7 +126,6 @@ class SheetContentScrollPosition extends ScrollPositionWithSingleContext {
     if (owner != null && owner.hasPrimaryScrollPosition && !calledByOwner) {
       owner.goBallisticWithScrollPosition(
         velocity: velocity,
-        shouldIgnorePointer: activity?.shouldIgnorePointer ?? true,
         scrollPosition: this,
       );
       return;

package/test/scrollable/scrollable_sheet_test.dart

@@ -38,12 +38,16 @@ class _TestSheetContent extends StatelessWidget {
   const _TestSheetContent({
     super.key,
     this.height = 500,
+    this.itemCount = 30,
     // Disable the snapping effect by default in tests.
     this.scrollPhysics = const ClampingScrollPhysics(),
+    this.onTapItem,
   });
 
   final double? height;
+  final int itemCount;
   final ScrollPhysics? scrollPhysics;
+  final void Function(int index)? onTapItem;
 
   @override
   Widget build(BuildContext context) {
@@ -54,9 +58,10 @@ class _TestSheetContent extends StatelessWidget {
         child: ListView(
           physics: scrollPhysics,
           children: List.generate(
-            30,
+            itemCount,
             (index) => ListTile(
               title: Text('Item $index'),
+              onTap: onTapItem != null ? () => onTapItem!(index) : null,
             ),
           ),
         ),
@@ -101,11 +106,7 @@ void main() {
             controller: controller,
             minExtent: const Extent.pixels(200),
             initialExtent: const Extent.pixels(200),
-            child: const Material(
-              child: _TestSheetContent(
-                height: 500,
-              ),
-            ),
+            child: const _TestSheetContent(height: 500),
           ),
         ),
       ),
@@ -139,6 +140,66 @@ void main() {
     );
   });
 
+  // Regression test for https://github.com/fujidaiti/smooth_sheets/issues/190
+  testWidgets(
+    'Press and hold scrollable view should stop momentum scrolling',
+    (tester) async {
+      const targetKey = Key('Target');
+      final controller = SheetController();
+      late ScrollController scrollController;
+
+      await tester.pumpWidget(
+        _TestApp(
+          child: ScrollableSheet(
+            controller: controller,
+            child: Builder(
+              builder: (context) {
+                // TODO(fujita): Refactor this line after #116 is resolved.
+                scrollController = PrimaryScrollController.of(context);
+                return _TestSheetContent(
+                  key: targetKey,
+                  itemCount: 1000,
+                  height: null,
+                  // The items need to be clickable to cause the reported issue.
+                  onTapItem: (index) {},
+                );
+              },
+            ),
+          ),
+        ),
+      );
+
+      const dragDistance = 200.0;
+      const flingSpeed = 2000.0;
+      await tester.fling(
+        find.byKey(targetKey),
+        const Offset(0, -1 * dragDistance), // Fling up
+        flingSpeed,
+      );
+
+      final offsetAfterFling = scrollController.offset;
+      // Don't know why, but we need to call `pump` at least 2 times
+      // to forward the animation clock.
+      await tester.pump();
+      await tester.pump(const Duration(milliseconds: 250));
+      final offsetBeforePress = scrollController.offset;
+      expect(offsetBeforePress, greaterThan(offsetAfterFling),
+          reason: 'Momentum scrolling should be in progress.');
+
+      // Press and hold the finger on the target widget.
+      await tester.press(find.byKey(targetKey));
+      // Wait for the momentum scrolling to stop.
+      await tester.pumpAndSettle();
+      final offsetAfterPress = scrollController.offset;
+      expect(
+        offsetAfterPress,
+        equals(offsetBeforePress),
+        reason: 'Momentum scrolling should be stopped immediately'
+            'by pressing and holding.',
+      );
+    },
+  );
+
   group('SheetKeyboardDismissible', () {
     late FocusNode focusNode;
     late Widget testWidget;