Fix: Press-and-hold gesture in PageView doesn't stop momentum scrolli…

…ng (#219)

## Fixes / Closes (optional)
<!-- List any issues or pull requests that this PR fixes or closes. Use
the format: "Fixes #123" or "Closes #456". -->

Fixes #214.

## Description

This PR adds `holdWithScrollPosition()` to `ScrollableSheetExtent` to
handle press-and-hold gestures in scrollable sheet content. It starts a
`HoldScrollDrivenSheetActivity`, which stops momentum scrolling (if it
is running).

The absence of this handling logic hasn't been problematic for simple
ListViews or PageViews with only one page. This is because the SDK
triggers `ScrollPosition.drag()` immediately after calling
`ScrollPosition.hold()` when the user performs a press-and-hold gesture.
However, in a PageView with multiple ListViews, `drag()` won't be called
after `hold()` until the user actually moves their finger.

## Summary (check all that apply)
<!-- Mark the boxes that apply to this PR. Add details if necessary. -->
- [x] Modified / added code
- [x] Modified / added tests
- [x] Modified / added examples
- [ ] Modified / added others (pubspec.yaml, workflows, etc...)
- [ ] Updated README
- [ ] Contains breaking changes
  - [ ] Created / updated migration guide
- [x] Incremented version number
  - [x] Updated CHANGELOG

.vscode/launch.json

@@ -143,6 +143,13 @@
       "type": "dart",
       "program": "lib/tutorial/navigation_sheet_and_keyboard.dart",
       "cwd": "./cookbook"
-    }
+    },
+    {
+      "name": "ScrollableSheet and PageView",
+      "request": "launch",
+      "type": "dart",
+      "program": "lib/tutorial/scrollable_pageview_sheet.dart",
+      "cwd": "./cookbook"
+    },
   ]
 }

cookbook/lib/tutorial/scrollable_pageview_sheet.dart

@@ -0,0 +1,86 @@
+import 'package:flutter/material.dart';
+import 'package:smooth_sheets/smooth_sheets.dart';
+
+void main() {
+  runApp(const _ScrollablePageViewSheetExample());
+}
+
+/// An example of [ScrollableSheet] + [PageView].
+class _ScrollablePageViewSheetExample extends StatelessWidget {
+  const _ScrollablePageViewSheetExample();
+
+  @override
+  Widget build(BuildContext context) {
+    return MaterialApp(
+      home: Scaffold(
+        body: Center(
+          child: Builder(
+            builder: (context) {
+              return ElevatedButton(
+                onPressed: () {
+                  Navigator.push(
+                    context,
+                    ModalSheetRoute(builder: (_) => const _MySheet()),
+                  );
+                },
+                child: const Text('Show Sheet'),
+              );
+            },
+          ),
+        ),
+      ),
+    );
+  }
+}
+
+final pageController = PageController();
+
+class _MySheet extends StatelessWidget {
+  const _MySheet();
+
+  @override
+  Widget build(BuildContext context) {
+    return ScrollableSheet(
+      child: Material(
+        child: SizedBox(
+          height: 600,
+          child: PageView(
+            controller: pageController,
+            children: const [
+              _PageViewItem(),
+              _PageViewItem(),
+              _PageViewItem(),
+            ],
+          ),
+        ),
+      ),
+    );
+  }
+}
+
+class _PageViewItem extends StatefulWidget {
+  const _PageViewItem();
+
+  @override
+  State<_PageViewItem> createState() => _PageViewItemState();
+}
+
+class _PageViewItemState extends State<_PageViewItem>
+    with AutomaticKeepAliveClientMixin {
+  @override
+  bool get wantKeepAlive => true;
+
+  @override
+  Widget build(BuildContext context) {
+    super.build(context);
+    return ListView.builder(
+      itemCount: 100,
+      itemBuilder: (context, index) {
+        return ListTile(
+          onTap: () {},
+          title: Text('Item $index'),
+        );
+      },
+    );
+  }
+}

package/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.9.3 Aug 19, 2024
+
+- Fix: Press-and-hold gesture in PageView doesn't stop momentum scrolling (#219)
+
 ## 0.9.2 Aug 14, 2024
 
 - Fix: Keyboard visibility changes disrupt route transition animation in NavigationSheet (#215)

package/lib/src/foundation/sheet_drag.dart

@@ -275,8 +275,7 @@ class SheetDragController implements Drag, ScrollActivityDelegate {
   /// to avoid duplicating the code of [ScrollDragController].
   late final ScrollDragController _impl;
 
-  // TODO: Remove unnecessary nullability.
-  SheetDragControllerTarget? _target;
+  late SheetDragControllerTarget _target;
 
   // TODO: Rename to _gestureProxy.
   SheetGestureTamperer? _gestureTamperer;
@@ -318,22 +317,22 @@ class SheetDragController implements Drag, ScrollActivityDelegate {
   void goBallistic(double velocity) {
     if (_impl.lastDetails case final DragEndDetails rawDetails) {
       var endDetails = SheetDragEndDetails(
-        axisDirection: _target!.dragAxisDirection,
+        axisDirection: _target.dragAxisDirection,
         velocityX: rawDetails.velocity.pixelsPerSecond.dx,
         velocityY: -1 * velocity,
       );
       if (_gestureTamperer case final tamper?) {
         endDetails = tamper.tamperWithDragEnd(endDetails);
       }
       _lastDetails = endDetails;
-      _target!.applyUserDragEnd(endDetails);
+      _target.applyUserDragEnd(endDetails);
     } else {
       final cancelDetails = SheetDragCancelDetails(
-        axisDirection: _target!.dragAxisDirection,
+        axisDirection: _target.dragAxisDirection,
       );
       _lastDetails = cancelDetails;
       _gestureTamperer?.onDragCancel(cancelDetails);
-      _target!.onDragCancel(cancelDetails);
+      _target.onDragCancel(cancelDetails);
     }
   }
 
@@ -344,7 +343,7 @@ class SheetDragController implements Drag, ScrollActivityDelegate {
     final rawDetails = _impl.lastDetails as DragUpdateDetails;
     var details = SheetDragUpdateDetails(
       sourceTimeStamp: rawDetails.sourceTimeStamp,
-      axisDirection: _target!.dragAxisDirection,
+      axisDirection: _target.dragAxisDirection,
       localPositionX: rawDetails.localPosition.dx,
       localPositionY: rawDetails.localPosition.dy,
       globalPositionX: rawDetails.globalPosition.dx,
@@ -355,7 +354,7 @@ class SheetDragController implements Drag, ScrollActivityDelegate {
 
     if (_gestureTamperer case final tamper?) {
       final minPotentialDeltaConsumption =
-          _target!.computeMinPotentialDeltaConsumption(details.delta);
+          _target.computeMinPotentialDeltaConsumption(details.delta);
       assert(minPotentialDeltaConsumption.dx.abs() <= details.delta.dx.abs());
       assert(minPotentialDeltaConsumption.dy.abs() <= details.delta.dy.abs());
       details = tamper.tamperWithDragUpdate(
@@ -365,12 +364,12 @@ class SheetDragController implements Drag, ScrollActivityDelegate {
     }
 
     _lastDetails = details;
-    _target!.applyUserDragUpdate(details);
+    _target.applyUserDragUpdate(details);
   }
 
   @override
   AxisDirection get axisDirection {
-    return switch (_target!.dragAxisDirection) {
+    return switch (_target.dragAxisDirection) {
       VerticalDirection.up => AxisDirection.up,
       VerticalDirection.down => AxisDirection.down,
     };
@@ -389,7 +388,6 @@ class SheetDragController implements Drag, ScrollActivityDelegate {
 
   @mustCallSuper
   void dispose() {
-    _target = null;
     _gestureTamperer = null;
     _impl.dispose();
   }

package/lib/src/foundation/sheet_extent.dart

@@ -392,36 +392,15 @@ abstract class SheetExtent extends ChangeNotifier
 
   @mustCallSuper
   void beginActivity(SheetActivity activity) {
+    assert((_activity is SheetDragControllerTarget) == (currentDrag != null));
+    currentDrag?.dispose();
+    currentDrag = null;
+
     final oldActivity = _activity;
     // Update the current activity before initialization.
     _activity = activity;
     activity.init(this);
-
-    if (oldActivity == null) {
-      return;
-    }
-
-    final wasDragging = oldActivity.status == SheetStatus.dragging;
-    final isDragging = activity.status == SheetStatus.dragging;
-
-    // TODO: Make more typesafe
-    assert(() {
-      final wasActuallyDragging =
-          currentDrag != null && oldActivity is SheetDragControllerTarget;
-      final isActuallyDragging =
-          currentDrag != null && activity is SheetDragControllerTarget;
-      return wasDragging == wasActuallyDragging &&
-          isDragging == isActuallyDragging;
-    }());
-
-    if (wasDragging && isDragging) {
-      currentDrag!.updateTarget(activity as SheetDragControllerTarget);
-    } else if (wasDragging && !isDragging) {
-      currentDrag!.dispose();
-      currentDrag = null;
-    }
-
-    oldActivity.dispose();
+    oldActivity?.dispose();
   }
 
   void goIdle() {
@@ -469,7 +448,7 @@ abstract class SheetExtent extends ChangeNotifier
       startDetails = tamperer.tamperWithDragStart(startDetails);
     }
 
-    final drag = currentDrag = SheetDragController(
+    final drag = SheetDragController(
       target: dragActivity,
       gestureTamperer: _gestureTamperer,
       details: startDetails,
@@ -479,6 +458,7 @@ abstract class SheetExtent extends ChangeNotifier
       motionStartDistanceThreshold: physics.dragStartDistanceMotionThreshold,
     );
     beginActivity(dragActivity);
+    currentDrag = drag;
     didDragStart(startDetails);
     return drag;
   }

package/lib/src/foundation/sheet_status.dart

@@ -1,3 +1,4 @@
+// TODO: Consider removing this API.
 /// The status of a sheet.
 enum SheetStatus {
   /// The sheet is resting at a natural position.

package/lib/src/scrollable/scrollable_sheet_activity.dart

@@ -1,11 +1,13 @@
 import 'dart:math';
 
+import 'package:flutter/gestures.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter/widgets.dart';
 import 'package:meta/meta.dart';
 
 import '../foundation/sheet_activity.dart';
 import '../foundation/sheet_drag.dart';
+import '../foundation/sheet_status.dart';
 import '../internal/float_comp.dart';
 import 'scrollable_sheet.dart';
 import 'scrollable_sheet_extent.dart';
@@ -296,3 +298,39 @@ class BallisticScrollDrivenSheetActivity extends ScrollableSheetActivity
     );
   }
 }
+
+/// A [SheetActivity] that does nothing but can be released to resume
+/// normal idle behavior.
+///
+/// This is used while the user is touching the scrollable content but before
+/// the touch has become a [Drag]. The [scrollPosition], which is associated
+/// with the scrollable content must have a [SheetContentHoldScrollActivity]
+/// as its activity throughout the lifetime of this activity.
+class HoldScrollDrivenSheetActivity extends ScrollableSheetActivity
+    implements ScrollHoldController {
+  HoldScrollDrivenSheetActivity(
+    super.scrollPosition, {
+    required this.heldPreviousVelocity,
+    required this.onHoldCanceled,
+  });
+
+  final double heldPreviousVelocity;
+  final VoidCallback? onHoldCanceled;
+
+  @override
+  SheetStatus get status => SheetStatus.dragging;
+
+  @override
+  void cancel() {
+    owner.goBallisticWithScrollPosition(
+      velocity: 0,
+      scrollPosition: scrollPosition,
+    );
+  }
+
+  @override
+  void dispose() {
+    onHoldCanceled?.call();
+    super.dispose();
+  }
+}

package/lib/src/scrollable/scrollable_sheet_extent.dart

@@ -116,6 +116,24 @@ class ScrollableSheetExtent extends SheetExtent
     goIdle();
   }
 
+  @override
+  ScrollHoldController holdWithScrollPosition({
+    required double heldPreviousVelocity,
+    required VoidCallback holdCancelCallback,
+    required SheetContentScrollPosition scrollPosition,
+  }) {
+    final holdActivity = HoldScrollDrivenSheetActivity(
+      scrollPosition,
+      onHoldCanceled: holdCancelCallback,
+      heldPreviousVelocity: heldPreviousVelocity,
+    );
+    scrollPosition.beginActivity(
+      SheetContentHoldScrollActivity(delegate: scrollPosition),
+    );
+    beginActivity(holdActivity);
+    return holdActivity;
+  }
+
   @override
   Drag dragWithScrollPosition({
     required DragStartDetails details,
@@ -136,24 +154,30 @@ class ScrollableSheetExtent extends SheetExtent
     if (gestureTamperer case final tamperer?) {
       startDetails = tamperer.tamperWithDragStart(startDetails);
     }
-    final drag = currentDrag = SheetDragController(
+    final heldPreviousVelocity = switch (activity) {
+      final HoldScrollDrivenSheetActivity holdActivity =>
+        holdActivity.heldPreviousVelocity,
+      _ => 0.0,
+    };
+    final drag = SheetDragController(
       target: dragActivity,
       gestureTamperer: gestureTamperer,
       details: startDetails,
       onDragCanceled: dragCancelCallback,
-      carriedVelocity: scrollPosition.physics
-          .carriedMomentum(scrollPosition.heldPreviousVelocity),
+      carriedVelocity:
+          scrollPosition.physics.carriedMomentum(heldPreviousVelocity),
       motionStartDistanceThreshold:
           scrollPosition.physics.dragStartDistanceMotionThreshold,
     );
     scrollPosition.beginActivity(
       SheetContentDragScrollActivity(
         delegate: scrollPosition,
-        getLastDragDetails: () => currentDrag?.lastRawDetails,
-        getPointerDeviceKind: () => currentDrag?.pointerDeviceKind,
+        getLastDragDetails: () => drag.lastRawDetails,
+        getPointerDeviceKind: () => drag.pointerDeviceKind,
       ),
     );
     beginActivity(dragActivity);
+    currentDrag = drag;
     didDragStart(startDetails);
     return drag;
   }

package/lib/src/scrollable/sheet_content_scroll_activity.dart

@@ -171,3 +171,24 @@ class SheetContentBallisticScrollActivity extends ScrollActivity {
   @override
   double get velocity => getVelocity();
 }
+
+/// A [ScrollActivity] for the [SheetContentScrollPosition] that is associated
+/// with a [HoldScrollDrivenSheetActivity].
+///
+/// This activity is like a placeholder, meaning it doesn't actually modify the
+/// scroll position and the actual scrolling is done by the associated
+/// [HoldScrollDrivenSheetActivity].
+class SheetContentHoldScrollActivity extends ScrollActivity {
+  SheetContentHoldScrollActivity({
+    required ScrollActivityDelegate delegate,
+  }) : super(delegate);
+
+  @override
+  bool get shouldIgnorePointer => false;
+
+  @override
+  bool get isScrolling => false;
+
+  @override
+  double get velocity => 0.0;
+}