From a6d62ca8de2e992f6a4e236198b00a2550a1c451 Mon Sep 17 00:00:00 2001 From: Daniel Iglesia Date: Mon, 22 May 2023 16:14:35 -0700 Subject: [PATCH] Support keeping a bottom sheet with a DraggableScrollableSheet from closing on drag/fling to min extent (#127339) --- .../lib/src/material/bottom_sheet.dart | 2 +- .../flutter/lib/src/material/scaffold.dart | 4 +- .../widgets/draggable_scrollable_sheet.dart | 21 ++++++++++ .../persistent_bottom_sheet_test.dart | 38 ++++++++++++++++++ .../draggable_scrollable_sheet_test.dart | 40 ++++++++++++++----- 5 files changed, 94 insertions(+), 11 deletions(-) diff --git a/packages/flutter/lib/src/material/bottom_sheet.dart b/packages/flutter/lib/src/material/bottom_sheet.dart index c42d5daa63..f041d1f865 100644 --- a/packages/flutter/lib/src/material/bottom_sheet.dart +++ b/packages/flutter/lib/src/material/bottom_sheet.dart @@ -315,7 +315,7 @@ class _BottomSheetState extends State { } bool extentChanged(DraggableScrollableNotification notification) { - if (notification.extent == notification.minExtent) { + if (notification.extent == notification.minExtent && notification.shouldCloseOnMinExtent) { widget.onClosing(); } return false; diff --git a/packages/flutter/lib/src/material/scaffold.dart b/packages/flutter/lib/src/material/scaffold.dart index 621c506742..d9d239c529 100644 --- a/packages/flutter/lib/src/material/scaffold.dart +++ b/packages/flutter/lib/src/material/scaffold.dart @@ -3190,7 +3190,9 @@ class _StandardBottomSheetState extends State<_StandardBottomSheet> { scaffold.showBodyScrim(false, 0.0); } // If the Scaffold.bottomSheet != null, we're a persistent bottom sheet. - if (notification.extent == notification.minExtent && scaffold.widget.bottomSheet == null) { + if (notification.extent == notification.minExtent && + scaffold.widget.bottomSheet == null && + notification.shouldCloseOnMinExtent) { close(); } return false; diff --git a/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart b/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart index 342c4ab4dc..0a63839619 100644 --- a/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart +++ b/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart @@ -308,6 +308,7 @@ class DraggableScrollableSheet extends StatefulWidget { this.snapSizes, this.snapAnimationDuration, this.controller, + this.shouldCloseOnMinExtent = true, required this.builder, }) : assert(minChildSize >= 0.0), assert(maxChildSize <= 1.0), @@ -391,6 +392,13 @@ class DraggableScrollableSheet extends StatefulWidget { /// A controller that can be used to programmatically control this sheet. final DraggableScrollableController? controller; + /// Whether the sheet, when dragged (or flung) to its minimum size, should + /// cause its parent sheet to close. + /// + /// Set on emitted [DraggableScrollableNotification]s. It is up to parent + /// classes to properly read and handle this value. + final bool shouldCloseOnMinExtent; + /// The builder that creates a child to display in this widget, which will /// use the provided [ScrollController] to enable dragging and scrolling /// of the contents. @@ -432,6 +440,7 @@ class DraggableScrollableNotification extends Notification with ViewportNotifica required this.maxExtent, required this.initialExtent, required this.context, + this.shouldCloseOnMinExtent = true, }) : assert(0.0 <= minExtent), assert(maxExtent <= 1.0), assert(minExtent <= extent), @@ -458,6 +467,12 @@ class DraggableScrollableNotification extends Notification with ViewportNotifica /// is live when it first gets the notification. final BuildContext context; + /// Whether the widget that fired this notification, when dragged (or flung) + /// to minExtent, should cause its parent sheet to close. + /// + /// It is up to parent classes to properly read and handle this value. + final bool shouldCloseOnMinExtent; + @override void debugFillDescription(List description) { super.debugFillDescription(description); @@ -487,6 +502,7 @@ class _DraggableSheetExtent { ValueNotifier? currentSize, bool? hasDragged, bool? hasChanged, + this.shouldCloseOnMinExtent = true, }) : assert(minSize >= 0), assert(maxSize <= 1), assert(minSize <= initialSize), @@ -504,6 +520,7 @@ class _DraggableSheetExtent { final List snapSizes; final Duration? snapAnimationDuration; final double initialSize; + final bool shouldCloseOnMinExtent; final ValueNotifier _currentSize; double availablePixels; @@ -576,6 +593,7 @@ class _DraggableSheetExtent { extent: currentSize, initialExtent: initialSize, context: context, + shouldCloseOnMinExtent: shouldCloseOnMinExtent, ).dispatch(context); } @@ -598,6 +616,7 @@ class _DraggableSheetExtent { required List snapSizes, required double initialSize, Duration? snapAnimationDuration, + bool shouldCloseOnMinExtent = true, }) { return _DraggableSheetExtent( minSize: minSize, @@ -613,6 +632,7 @@ class _DraggableSheetExtent { : initialSize), hasDragged: hasDragged, hasChanged: hasChanged, + shouldCloseOnMinExtent: shouldCloseOnMinExtent, ); } } @@ -631,6 +651,7 @@ class _DraggableScrollableSheetState extends State { snapSizes: _impliedSnapSizes(), snapAnimationDuration: widget.snapAnimationDuration, initialSize: widget.initialChildSize, + shouldCloseOnMinExtent: widget.shouldCloseOnMinExtent, ); _scrollController = _DraggableScrollableSheetScrollController(extent: _extent); widget.controller?._attach(_scrollController); diff --git a/packages/flutter/test/material/persistent_bottom_sheet_test.dart b/packages/flutter/test/material/persistent_bottom_sheet_test.dart index 7aa890aa16..670a3d1105 100644 --- a/packages/flutter/test/material/persistent_bottom_sheet_test.dart +++ b/packages/flutter/test/material/persistent_bottom_sheet_test.dart @@ -185,6 +185,44 @@ void main() { expect(find.text('Two'), findsNothing); }); + testWidgets('Verify DraggableScrollableSheet.shouldCloseOnMinExtent == false prevents dismissal', (WidgetTester tester) async { + final GlobalKey scaffoldKey = GlobalKey(); + + await tester.pumpWidget(MaterialApp( + home: Scaffold( + key: scaffoldKey, + body: const Center(child: Text('body')), + ), + )); + + scaffoldKey.currentState!.showBottomSheet((BuildContext context) { + return DraggableScrollableSheet( + expand: false, + shouldCloseOnMinExtent: false, + builder: (_, ScrollController controller) { + return ListView( + controller: controller, + shrinkWrap: true, + children: const [ + SizedBox(height: 100.0, child: Text('One')), + SizedBox(height: 100.0, child: Text('Two')), + SizedBox(height: 100.0, child: Text('Three')), + ], + ); + }, + ); + }); + + await tester.pumpAndSettle(); + + expect(find.text('Two'), findsOneWidget); + + await tester.drag(find.text('Two'), const Offset(0.0, 400.0)); + await tester.pumpAndSettle(); + + expect(find.text('Two'), findsOneWidget); + }); + testWidgets('Verify that a BottomSheet animates non-linearly', (WidgetTester tester) async { final GlobalKey scaffoldKey = GlobalKey(); diff --git a/packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart b/packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart index 929893252b..db3fbb8d2d 100644 --- a/packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart +++ b/packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart @@ -20,7 +20,9 @@ void main() { Key? containerKey, Key? stackKey, NotificationListenerCallback? onScrollNotification, + NotificationListenerCallback? onDraggableScrollableNotification, bool ignoreController = false, + bool shouldCloseOnMinExtent = true, }) { return Directionality( textDirection: TextDirection.ltr, @@ -42,19 +44,23 @@ void main() { snap: snap, snapSizes: snapSizes, snapAnimationDuration: snapAnimationDuration, + shouldCloseOnMinExtent: shouldCloseOnMinExtent, builder: (BuildContext context, ScrollController scrollController) { return NotificationListener( onNotification: onScrollNotification, - child: ColoredBox( - key: containerKey, - color: const Color(0xFFABCDEF), - child: ListView.builder( - controller: ignoreController ? null : scrollController, - itemExtent: itemExtent, - itemCount: itemCount, - itemBuilder: (BuildContext context, int index) => Text('Item $index'), + child: NotificationListener( + onNotification: onDraggableScrollableNotification, + child:ColoredBox( + key: containerKey, + color: const Color(0xFFABCDEF), + child: ListView.builder( + controller: ignoreController ? null : scrollController, + itemExtent: itemExtent, + itemCount: itemCount, + itemBuilder: (BuildContext context, int index) => Text('Item $index'), + ), ), - ), + ) ); }, ), @@ -864,6 +870,22 @@ void main() { expect(notificationTypes, types); }); + testWidgets('Emits DraggableScrollableNotification with shouldCloseOnMinExtent set to non-default value', (WidgetTester tester) async { + DraggableScrollableNotification? receivedNotification; + await tester.pumpWidget(boilerplateWidget( + null, + shouldCloseOnMinExtent: false, + onDraggableScrollableNotification: (DraggableScrollableNotification notification) { + receivedNotification = notification; + return false; + }, + )); + + await tester.flingFrom(const Offset(0, 325), const Offset(0, -325), 200); + await tester.pumpAndSettle(); + expect(receivedNotification!.shouldCloseOnMinExtent, isFalse); + }); + testWidgets('Do not crash when remove the tree during animation.', (WidgetTester tester) async { // Regression test for https://github.com/flutter/flutter/issues/89214 await tester.pumpWidget(boilerplateWidget(