diff --git a/.vscode/launch.json b/.vscode/launch.json index 7c68f21..62d3151 100644 --- a/.vscode/launch.json +++ b/.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" + }, ] } diff --git a/cookbook/lib/tutorial/scrollable_pageview_sheet.dart b/cookbook/lib/tutorial/scrollable_pageview_sheet.dart new file mode 100644 index 0000000..c00aee3 --- /dev/null +++ b/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'), + ); + }, + ); + } +} diff --git a/package/CHANGELOG.md b/package/CHANGELOG.md index 15964f7..a01c8db 100644 --- a/package/CHANGELOG.md +++ b/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) diff --git a/package/lib/src/foundation/sheet_drag.dart b/package/lib/src/foundation/sheet_drag.dart index e5470ce..8f72270 100644 --- a/package/lib/src/foundation/sheet_drag.dart +++ b/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,7 +317,7 @@ 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, ); @@ -326,14 +325,14 @@ class SheetDragController implements Drag, ScrollActivityDelegate { 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(); } diff --git a/package/lib/src/foundation/sheet_extent.dart b/package/lib/src/foundation/sheet_extent.dart index e26255b..19a3059 100644 --- a/package/lib/src/foundation/sheet_extent.dart +++ b/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; } diff --git a/package/lib/src/foundation/sheet_status.dart b/package/lib/src/foundation/sheet_status.dart index 6bd89a8..cdbeb69 100644 --- a/package/lib/src/foundation/sheet_status.dart +++ b/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. diff --git a/package/lib/src/scrollable/scrollable_sheet_activity.dart b/package/lib/src/scrollable/scrollable_sheet_activity.dart index d282d69..bac85a9 100644 --- a/package/lib/src/scrollable/scrollable_sheet_activity.dart +++ b/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(); + } +} diff --git a/package/lib/src/scrollable/scrollable_sheet_extent.dart b/package/lib/src/scrollable/scrollable_sheet_extent.dart index 02ea2ec..857da63 100644 --- a/package/lib/src/scrollable/scrollable_sheet_extent.dart +++ b/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; } diff --git a/package/lib/src/scrollable/sheet_content_scroll_activity.dart b/package/lib/src/scrollable/sheet_content_scroll_activity.dart index 8ae2d5e..0e58fa4 100644 --- a/package/lib/src/scrollable/sheet_content_scroll_activity.dart +++ b/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; +} diff --git a/package/lib/src/scrollable/sheet_content_scroll_position.dart b/package/lib/src/scrollable/sheet_content_scroll_position.dart index 6671d68..49d4f7a 100644 --- a/package/lib/src/scrollable/sheet_content_scroll_position.dart +++ b/package/lib/src/scrollable/sheet_content_scroll_position.dart @@ -10,7 +10,7 @@ import 'scrollable_sheet.dart'; /// An owner of [SheetContentScrollPosition]s. /// /// The associated scroll positions delegate their behavior of -/// `goIdle`, `drag`, and `goBallistic` to this owner. +/// `goIdle`, `hold`, `drag`, and `goBallistic` to this owner. @internal abstract class SheetContentScrollPositionOwner { bool get hasPrimaryScrollPosition; @@ -24,6 +24,12 @@ abstract class SheetContentScrollPositionOwner { void goIdleWithScrollPosition(); + ScrollHoldController holdWithScrollPosition({ + required double heldPreviousVelocity, + required VoidCallback holdCancelCallback, + required SheetContentScrollPosition scrollPosition, + }); + Drag dragWithScrollPosition({ required DragStartDetails details, required VoidCallback dragCancelCallback, @@ -59,15 +65,6 @@ class SheetContentScrollPosition extends ScrollPositionWithSingleContext { /// this object to the controller, and it is unset when detaching. ValueGetter? _getOwner; - /// Velocity from a previous activity temporarily held by [hold] - /// to potentially transfer to a next activity. - /// - /// This mirrors the value of `_heldPreviousVelocity` in - /// [ScrollPositionWithSingleContext] and is exposed here for - /// being used from outside of this object. - 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; @@ -91,13 +88,6 @@ class SheetContentScrollPosition extends ScrollPositionWithSingleContext { super.absorb(other); } - @override - void beginActivity(ScrollActivity? newActivity) { - _heldPreviousVelocity = - newActivity is HoldScrollActivity ? activity!.velocity : 0.0; - super.beginActivity(newActivity); - } - @override void goIdle({bool calledByOwner = false}) { final owner = _getOwner?.call(); @@ -108,6 +98,18 @@ class SheetContentScrollPosition extends ScrollPositionWithSingleContext { } } + @override + ScrollHoldController hold(VoidCallback holdCancelCallback) { + return switch (_getOwner?.call()) { + null => super.hold(holdCancelCallback), + final owner => owner.holdWithScrollPosition( + scrollPosition: this, + holdCancelCallback: holdCancelCallback, + heldPreviousVelocity: activity!.velocity, + ), + }; + } + @override Drag drag(DragStartDetails details, VoidCallback dragCancelCallback) { return switch (_getOwner?.call()) { diff --git a/package/pubspec.yaml b/package/pubspec.yaml index c21fbff..215b675 100644 --- a/package/pubspec.yaml +++ b/package/pubspec.yaml @@ -1,6 +1,6 @@ name: smooth_sheets description: Sheet widgets with smooth motion and great flexibility. Also supports nested navigation in both imperative and declarative ways. -version: 0.9.2 +version: 0.9.3 repository: https://github.com/fujidaiti/smooth_sheets screenshots: - description: Practical examples of smooth_sheets. diff --git a/package/test/scrollable/scrollable_sheet_test.dart b/package/test/scrollable/scrollable_sheet_test.dart index d783262..df28af9 100644 --- a/package/test/scrollable/scrollable_sheet_test.dart +++ b/package/test/scrollable/scrollable_sheet_test.dart @@ -144,28 +144,95 @@ 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; + group('Press-and-hold gesture should stop momentum scrolling', () { + // Regression test for https://github.com/fujidaiti/smooth_sheets/issues/190 + testWidgets( + 'in a plain ListView', + (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 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.', + ); + }, + ); + + // Regression test for https://github.com/fujidaiti/smooth_sheets/issues/214 + testWidgets('in a PageView with multiple ListViews', (tester) async { + late final 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) {}, + return Material( + child: PageView( + controller: PageController(), + children: [ + for (var i = 0; i < 2; i++) + ListView.builder( + key: Key('ListView #$i'), + itemCount: 100, + itemBuilder: (context, index) { + return ListTile( + onTap: () {}, + title: Text('Item $index'), + ); + }, + ), + ], + ), ); }, ), @@ -173,10 +240,11 @@ void main() { ), ); + const listViewKey = Key('ListView #0'); const dragDistance = 200.0; const flingSpeed = 2000.0; await tester.fling( - find.byKey(targetKey), + find.byKey(listViewKey), const Offset(0, -1 * dragDistance), // Fling up flingSpeed, ); @@ -191,18 +259,18 @@ void main() { reason: 'Momentum scrolling should be in progress.'); // Press and hold the finger on the target widget. - await tester.press(find.byKey(targetKey)); + await tester.press(find.byKey(listViewKey)); // 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' + reason: 'Momentum scrolling should be stopped immediately ' 'by pressing and holding.', ); - }, - ); + }); + }); group('SheetKeyboardDismissible', () { late FocusNode focusNode;