diff --git a/package/lib/src/draggable/draggable_sheet.dart b/package/lib/src/draggable/draggable_sheet.dart index 76d2e61f..c1bf3240 100644 --- a/package/lib/src/draggable/draggable_sheet.dart +++ b/package/lib/src/draggable/draggable_sheet.dart @@ -73,30 +73,25 @@ class DraggableSheet extends StatelessWidget { this.keyboardDismissBehavior ?? theme?.keyboardDismissBehavior; final gestureTamper = TamperSheetGesture.maybeOf(context); - Widget result = ImplicitSheetControllerScope( + Widget result = SheetExtentScope( controller: controller, - builder: (context, controller) { - return SheetExtentScope( - controller: controller, - factory: const DraggableSheetExtentFactory(), - config: DraggableSheetExtentConfig( - initialExtent: initialExtent, - minExtent: minExtent, - maxExtent: maxExtent, - physics: physics, - gestureTamperer: gestureTamper, - debugLabel: 'DraggableSheet', + factory: const DraggableSheetExtentFactory(), + config: DraggableSheetExtentConfig( + initialExtent: initialExtent, + minExtent: minExtent, + maxExtent: maxExtent, + physics: physics, + gestureTamperer: gestureTamper, + debugLabel: 'DraggableSheet', + ), + child: SheetViewport( + child: SheetContentViewport( + child: SheetDraggable( + behavior: hitTestBehavior, + child: child, ), - child: SheetViewport( - child: SheetContentViewport( - child: SheetDraggable( - behavior: hitTestBehavior, - child: child, - ), - ), - ), - ); - }, + ), + ), ); if (keyboardDismissBehavior != null) { diff --git a/package/lib/src/foundation/sheet_controller.dart b/package/lib/src/foundation/sheet_controller.dart index d17992b7..c8460ca9 100644 --- a/package/lib/src/foundation/sheet_controller.dart +++ b/package/lib/src/foundation/sheet_controller.dart @@ -105,7 +105,21 @@ class SheetControllerScope extends InheritedWidget { } static SheetController of(BuildContext context) { - return maybeOf(context)!; + final controller = maybeOf(context); + + assert((() { + if (controller == null) { + throw FlutterError( + 'No $SheetControllerScope ancestor could be found starting ' + 'from the context that was passed to $SheetControllerScope.of(). ' + 'The context used was:\n' + '$context', + ); + } + return true; + })()); + + return controller!; } @override @@ -114,41 +128,6 @@ class SheetControllerScope extends InheritedWidget { } } -/// A widget that ensures that a [SheetController] is available in the subtree. -/// -/// The [builder] callback will be called with the [controller] if it is -/// explicitly provided and is not null, or a [SheetController] that is hosted -/// in the nearest ancestor [SheetControllerScope]. If neither is found, a newly -/// created [SheetController] hosted in a [DefaultSheetController] will be -/// used as a fallback. -@internal -// TODO: Remove this. -class ImplicitSheetControllerScope extends StatelessWidget { - const ImplicitSheetControllerScope({ - super.key, - this.controller, - required this.builder, - }); - - final SheetController? controller; - final Widget Function(BuildContext, SheetController) builder; - - @override - Widget build(BuildContext context) { - return switch (controller ?? DefaultSheetController.maybeOf(context)) { - final controller? => builder(context, controller), - null => DefaultSheetController( - child: Builder( - builder: (context) { - final controller = DefaultSheetController.of(context); - return builder(context, controller); - }, - ), - ), - }; - } -} - class DefaultSheetController extends StatefulWidget { const DefaultSheetController({ super.key, diff --git a/package/lib/src/foundation/sheet_extent.dart b/package/lib/src/foundation/sheet_extent.dart index 0e37a94b..c01916bd 100644 --- a/package/lib/src/foundation/sheet_extent.dart +++ b/package/lib/src/foundation/sheet_extent.dart @@ -855,15 +855,15 @@ class SheetExtentScope> /// Creates a widget that hosts a [SheetExtent]. const SheetExtentScope({ super.key, - required this.controller, + this.controller, required this.config, required this.factory, this.isPrimary = true, required this.child, }); - /// The [SheetController] that will be attached to the created [SheetExtent]. - final SheetController controller; + /// The [SheetController] attached to the [SheetExtent]. + final SheetController? controller; final C config; @@ -921,6 +921,7 @@ class _SheetExtentScopeState this; @@ -942,28 +943,31 @@ class _SheetExtentScopeState oldWidget) { super.didUpdateWidget(oldWidget); + _rewireControllerAndScope(); final oldExtent = _extent; if (widget.factory != oldWidget.factory) { _extent = _createExtent()..takeOver(oldExtent); _scopeKey?._notifySheetExtentCreation(); - _discard(oldExtent); + _disposeExtent(oldExtent); + _rewireControllerAndExtent(); } else if (widget.config != oldWidget.config) { _extent.applyNewConfig(widget.config); } - _invalidateExtentOwnership(); } E _createExtent() { @@ -973,22 +977,30 @@ class _SheetExtentScopeState widget.keyboardDismissBehavior ?? theme?.keyboardDismissBehavior; final gestureTamper = TamperSheetGesture.maybeOf(context); - Widget result = ImplicitSheetControllerScope( + Widget result = SheetExtentScope( + key: _scopeKey, controller: widget.controller, - builder: (context, controller) { - return SheetExtentScope( - key: _scopeKey, - controller: controller, - factory: this, - isPrimary: true, - config: SheetExtentConfig( - minExtent: const Extent.pixels(0), - maxExtent: const Extent.proportional(1), - // TODO: Use more appropriate physics. - physics: const ClampingSheetPhysics(), - gestureTamperer: gestureTamper, - debugLabel: kDebugMode ? 'NavigationSheet' : null, - ), - child: NavigationSheetViewport( - child: SheetContentViewport(child: widget.child), - ), - ); - }, + factory: this, + isPrimary: true, + config: SheetExtentConfig( + minExtent: const Extent.pixels(0), + maxExtent: const Extent.proportional(1), + // TODO: Use more appropriate physics. + physics: const ClampingSheetPhysics(), + gestureTamperer: gestureTamper, + debugLabel: kDebugMode ? 'NavigationSheet' : null, + ), + child: NavigationSheetViewport( + child: SheetContentViewport(child: widget.child), + ), ); if (keyboardDismissBehavior != null) { diff --git a/package/lib/src/scrollable/scrollable_sheet.dart b/package/lib/src/scrollable/scrollable_sheet.dart index eb6c3413..6299fc30 100644 --- a/package/lib/src/scrollable/scrollable_sheet.dart +++ b/package/lib/src/scrollable/scrollable_sheet.dart @@ -52,26 +52,21 @@ class ScrollableSheet extends StatelessWidget { this.keyboardDismissBehavior ?? theme?.keyboardDismissBehavior; final gestureTamper = TamperSheetGesture.maybeOf(context); - Widget result = ImplicitSheetControllerScope( + Widget result = SheetExtentScope( controller: controller, - builder: (context, controller) { - return SheetExtentScope( - controller: controller, - factory: const ScrollableSheetExtentFactory(), - config: ScrollableSheetExtentConfig.withFallbacks( - initialExtent: initialExtent, - minExtent: minExtent, - maxExtent: maxExtent, - physics: physics ?? theme?.physics, - gestureTamperer: gestureTamper, - debugLabel: 'ScrollableSheet', - ), - child: SheetViewport( - child: SheetContentViewport( - child: ScrollableSheetContent(child: child), - )), - ); - }, + factory: const ScrollableSheetExtentFactory(), + config: ScrollableSheetExtentConfig.withFallbacks( + initialExtent: initialExtent, + minExtent: minExtent, + maxExtent: maxExtent, + physics: physics ?? theme?.physics, + gestureTamperer: gestureTamper, + debugLabel: 'ScrollableSheet', + ), + child: SheetViewport( + child: SheetContentViewport( + child: ScrollableSheetContent(child: child), + )), ); if (keyboardDismissBehavior != null) { diff --git a/package/test/navigation/navigation_sheet_test.dart b/package/test/navigation/navigation_sheet_test.dart new file mode 100644 index 00000000..24173460 --- /dev/null +++ b/package/test/navigation/navigation_sheet_test.dart @@ -0,0 +1,168 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:smooth_sheets/smooth_sheets.dart'; + +class _TestWidget extends StatelessWidget { + const _TestWidget( + this.sheetTransitionObserver, { + required this.initialRoute, + required this.routes, + this.onTapBackgroundText, + this.sheetController, + }); + + final String initialRoute; + final Map>> routes; + final VoidCallback? onTapBackgroundText; + final SheetController? sheetController; + final NavigationSheetTransitionObserver sheetTransitionObserver; + + @override + Widget build(BuildContext context) { + final navigationSheet = NavigationSheet( + controller: sheetController, + transitionObserver: sheetTransitionObserver, + child: ColoredBox( + color: Colors.white, + child: Navigator( + observers: [sheetTransitionObserver], + initialRoute: initialRoute, + onGenerateRoute: (settings) => routes[settings.name]!(), + ), + ), + ); + + return Directionality( + textDirection: TextDirection.ltr, + child: MediaQuery( + data: const MediaQueryData(), + child: Stack( + children: [ + TextButton( + onPressed: onTapBackgroundText, + child: const Text('Background text'), + ), + navigationSheet, + ], + ), + ), + ); + } +} + +class _TestDraggablePageWidget extends StatelessWidget { + const _TestDraggablePageWidget({ + super.key, + required this.height, + required this.label, + this.onTapNext, + this.onTapBack, + }); + + final double height; + final String label; + final VoidCallback? onTapNext; + final VoidCallback? onTapBack; + + @override + Widget build(BuildContext context) { + return Container( + // Add an opaque background color, otherwise the container + // does not respond to drag gestures. + color: Colors.blue, + width: double.infinity, + height: height, + // Do not place the buttons in the center of the container, + // so that the drag gestures performed in `tester.darg` and + // starting from the center of the container are not stolen + // by the buttons. + alignment: Alignment.topLeft, + child: Column( + children: [ + Text(label), + TextButton( + onPressed: onTapNext, + child: const Text('Next'), + ), + TextButton( + onPressed: onTapBack, + child: const Text('Back'), + ), + ], + ), + ); + } + + static Route createRoute({ + Key? key, + required String label, + required double height, + String? nextRoute, + Extent minExtent = const Extent.proportional(1), + SheetPhysics? physics, + }) { + return DraggableNavigationSheetRoute( + physics: physics, + minExtent: minExtent, + builder: (context) => _TestDraggablePageWidget( + key: key, + height: height, + label: label, + onTapBack: () => Navigator.pop(context), + onTapNext: nextRoute != null + ? () => Navigator.pushNamed(context, nextRoute) + : null, + ), + ); + } +} + +void main() { + late NavigationSheetTransitionObserver transitionObserver; + + setUp(() { + transitionObserver = NavigationSheetTransitionObserver(); + }); + + testWidgets( + 'Attached controller emits correct pixel values when dragging', + (tester) async { + final pixelTracking = []; + final controller = SheetController(); + controller.addListener(() { + pixelTracking.add(controller.value.maybePixels); + }); + + await tester.pumpWidget( + _TestWidget( + transitionObserver, + sheetController: controller, + initialRoute: 'First', + routes: { + 'First': () => _TestDraggablePageWidget.createRoute( + key: const Key('First'), + label: 'First', + nextRoute: 'Second', + height: 300, + minExtent: const Extent.pixels(0), + // Disable the snapping effect. + physics: const ClampingSheetPhysics(), + ), + }, + ), + ); + // Initial pixel value is emitted after the first build. + expect(pixelTracking, equals([300])); + + // Drag the sheet down by 50 pixels. + await tester.drag( + find.byKey(const Key('First')), + const Offset(0, 50), + // The drag will be broken into two separate calls. + touchSlopY: 20, + ); + await tester.pumpAndSettle(); + expect(pixelTracking, equals([300, 280, 250])); + }, + ); +}