Make DraggableScrollableSheet Reflect Parameter Updates (#90354)

packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart

@@ -8,6 +8,7 @@ import 'package:flutter/foundation.dart';
 import 'package:flutter/gestures.dart';
 
 import 'basic.dart';
+import 'binding.dart';
 import 'framework.dart';
 import 'inherited_notifier.dart';
 import 'layout_builder.dart';
@@ -132,6 +133,10 @@ class DraggableScrollableSheet extends StatefulWidget {
   /// The initial fractional value of the parent container's height to use when
   /// displaying the widget.
   ///
+  /// Rebuilding the sheet with a new [initialChildSize] will only move the
+  /// the sheet to the new value if the sheet has not yet been dragged since it
+  /// was first built or since the last call to [DraggableScrollableActuator.reset].
+  ///
   /// The default value is `0.5`.
   final double initialChildSize;
 
@@ -163,6 +168,11 @@ class DraggableScrollableSheet extends StatefulWidget {
   /// If the user's finger was still moving when they lifted it, the widget will
   /// snap to the next snap size (see [snapSizes]) in the direction of the drag.
   /// If their finger was still, the widget will snap to the nearest snap size.
+  ///
+  /// Rebuilding the sheet with snap newly enabled will immediately trigger a
+  /// snap unless the sheet has not yet been dragged away from
+  /// [initialChildSize] since first being built or since the last call to
+  /// [DraggableScrollableActuator.reset].
   final bool snap;
 
   /// A list of target sizes that the widget should snap to.
@@ -175,6 +185,13 @@ class DraggableScrollableSheet extends StatefulWidget {
   /// sizes and do not need to be specified here. For example, `snapSizes = [.5]`
   /// will result in a sheet that snaps between [minChildSize], `.5`, and
   /// [maxChildSize].
+  ///
+  /// Any modifications to the [snapSizes] list will not take effect until the
+  /// `build` function containing this widget is run again.
+  ///
+  /// Rebuilding with a modified or new list will trigger a snap unless the
+  /// sheet has not yet been dragged away from [initialChildSize] since first
+  /// being built or since the last call to [DraggableScrollableActuator.reset].
   final List<double>? snapSizes;
 
   /// The builder that creates a child to display in this widget, which will
@@ -274,16 +291,20 @@ class _DraggableSheetExtent {
     required this.snap,
     required this.snapSizes,
     required this.initialExtent,
-    required VoidCallback listener,
-  }) : assert(minExtent != null),
-       assert(maxExtent != null),
-       assert(initialExtent != null),
-       assert(minExtent >= 0),
-       assert(maxExtent <= 1),
-       assert(minExtent <= initialExtent),
-       assert(initialExtent <= maxExtent),
-       _currentExtent = ValueNotifier<double>(initialExtent)..addListener(listener),
-       availablePixels = double.infinity;
+    required this.onExtentChanged,
+    ValueNotifier<double>? currentExtent,
+    bool? hasChanged,
+  })  : assert(minExtent != null),
+        assert(maxExtent != null),
+        assert(initialExtent != null),
+        assert(minExtent >= 0),
+        assert(maxExtent <= 1),
+        assert(minExtent <= initialExtent),
+        assert(initialExtent <= maxExtent),
+        _currentExtent = (currentExtent ?? ValueNotifier<double>(initialExtent))
+          ..addListener(onExtentChanged),
+        availablePixels = double.infinity,
+        hasChanged = hasChanged ?? false;
 
   final double minExtent;
   final double maxExtent;
@@ -291,11 +312,12 @@ class _DraggableSheetExtent {
   final List<double> snapSizes;
   final double initialExtent;
   final ValueNotifier<double> _currentExtent;
+  final VoidCallback onExtentChanged;
   double availablePixels;
 
   // Used to disable snapping until the extent has changed. We do this because
   // we don't want to snap away from the initial extent.
-  bool hasChanged = false;
+  bool hasChanged;
 
   bool get isAtMin => minExtent >= _currentExtent.value;
   bool get isAtMax => maxExtent <= _currentExtent.value;
@@ -341,6 +363,33 @@ class _DraggableSheetExtent {
   double extentToPixels(double extent) {
     return extent / maxExtent * availablePixels;
   }
+
+  void dispose() {
+    _currentExtent.removeListener(onExtentChanged);
+  }
+
+  _DraggableSheetExtent copyWith({
+    required double minExtent,
+    required double maxExtent,
+    required bool snap,
+    required List<double> snapSizes,
+    required double initialExtent,
+    required VoidCallback onExtentChanged,
+  }) {
+    return _DraggableSheetExtent(
+      minExtent: minExtent,
+      maxExtent: maxExtent,
+      snap: snap,
+      snapSizes: snapSizes,
+      initialExtent: initialExtent,
+      onExtentChanged: onExtentChanged,
+      // Use the possibly updated initialExtent if the user hasn't dragged yet.
+      currentExtent: ValueNotifier<double>(hasChanged
+          ? _currentExtent.value.clamp(minExtent, maxExtent)
+          : initialExtent),
+      hasChanged: hasChanged,
+    );
+  }
 }
 
 class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
@@ -356,7 +405,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
       snap: widget.snap,
       snapSizes: _impliedSnapSizes(),
       initialExtent: widget.initialChildSize,
-      listener: _setExtent,
+      onExtentChanged: _setExtent,
     );
     _scrollController = _DraggableScrollableSheetScrollController(extent: _extent);
   }
@@ -385,6 +434,12 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
     ];
   }
 
+  @override
+  void didUpdateWidget(covariant DraggableScrollableSheet oldWidget) {
+    super.didUpdateWidget(oldWidget);
+    _replaceExtent();
+  }
+
   @override
   void didChangeDependencies() {
     super.didChangeDependencies();
@@ -408,7 +463,6 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
     setState(() {
       // _extent has been updated when this is called.
     });
-
   }
 
   @override
@@ -429,9 +483,34 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
   @override
   void dispose() {
     _scrollController.dispose();
+    _extent.dispose();
     super.dispose();
   }
 
+  void _replaceExtent() {
+    _extent.dispose();
+    _extent = _extent.copyWith(
+      minExtent: widget.minChildSize,
+      maxExtent: widget.maxChildSize,
+      snap: widget.snap,
+      snapSizes: _impliedSnapSizes(),
+      initialExtent: widget.initialChildSize,
+      onExtentChanged: _setExtent,
+    );
+    // Modify the existing scroll controller instead of replacing it so that
+    // developers listening to the controller do not have to rebuild their listeners.
+    _scrollController.extent = _extent;
+    if (widget.snap) {
+      // Trigger a snap in case snap or snapSizes has changed. We put this in a
+      // post frame callback so that `build` can update `_extent.availablePixels`
+      // before this runs-we can't use the previous extent's available pixels as
+      // it may have changed when the widget was updated.
+      WidgetsBinding.instance!.addPostFrameCallback((Duration timeStamp) {
+        _scrollController.position.goBallistic(0);
+      });
+    }
+  }
+
   String _snapSizeErrorMessage(int invalidIndex) {
     final List<String> snapSizesWithIndicator = widget.snapSizes!.asMap().keys.map(
           (int index) {
@@ -472,7 +551,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
          initialScrollOffset: initialScrollOffset,
        );
 
-  final _DraggableSheetExtent extent;
+  _DraggableSheetExtent extent;
 
   @override
   _DraggableScrollableSheetScrollPosition createScrollPosition(
@@ -484,7 +563,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
       physics: physics,
       context: context,
       oldPosition: oldPosition,
-      extent: extent,
+      getExtent: () => extent,
     );
   }
 
@@ -493,6 +572,10 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
     super.debugFillDescription(description);
     description.add('extent: $extent');
   }
+
+  @override
+  _DraggableScrollableSheetScrollPosition get position =>
+      super.position as _DraggableScrollableSheetScrollPosition;
 }
 
 /// A scroll position that manages scroll activities for
@@ -516,9 +599,8 @@ class _DraggableScrollableSheetScrollPosition
     bool keepScrollOffset = true,
     ScrollPosition? oldPosition,
     String? debugLabel,
-    required this.extent,
-  })  : assert(extent != null),
-        super(
+    required this.getExtent,
+  })  : super(
           physics: physics,
           context: context,
           initialPixels: initialPixels,
@@ -529,9 +611,11 @@ class _DraggableScrollableSheetScrollPosition
 
   VoidCallback? _dragCancelCallback;
   VoidCallback? _ballisticCancelCallback;
-  final _DraggableSheetExtent extent;
+  final _DraggableSheetExtent Function() getExtent;
   bool get listShouldScroll => pixels > 0.0;
 
+  _DraggableSheetExtent get extent => getExtent();
+
   @override
   void beginActivity(ScrollActivity? newActivity) {
     // Cancel the running ballistic simulation, if there is one.

packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart

@@ -18,6 +18,7 @@ void main() {
     Key? containerKey,
     Key? stackKey,
     NotificationListenerCallback<ScrollNotification>? onScrollNotification,
+    bool ignoreController = false,
   }) {
     return Directionality(
       textDirection: TextDirection.ltr,
@@ -44,7 +45,7 @@ void main() {
                       key: containerKey,
                       color: const Color(0xFFABCDEF),
                       child: ListView.builder(
-                        controller: scrollController,
+                        controller: ignoreController ? null : scrollController,
                         itemExtent: itemExtent,
                         itemCount: itemCount,
                         itemBuilder: (BuildContext context, int index) => Text('Item $index'),
@@ -323,6 +324,8 @@ void main() {
       expect(find.text('Item 31'), findsNothing);
       expect(find.text('Item 70'), findsNothing);
     }, variant: TargetPlatformVariant.all());
+
+    debugDefaultTargetPlatformOverride = null;
   });
 
   testWidgets('Does not snap away from initial child on build', (WidgetTester tester) async {
@@ -429,32 +432,32 @@ void main() {
     testWidgets('Setting snapSizes to $snapSizes resolves to min and max', (WidgetTester tester) async {
       const Key stackKey = ValueKey<String>('stack');
       const Key containerKey = ValueKey<String>('container');
-        await tester.pumpWidget(_boilerplate(null,
-          snap: true,
-          stackKey: stackKey,
-          containerKey: containerKey,
-          snapSizes: snapSizes,
-        ));
-        await tester.pumpAndSettle();
-        final double screenHeight = tester.getSize(find.byKey(stackKey)).height;
-
-        await tester.drag(find.text('Item 1'), Offset(0, -.4 * screenHeight));
-        await tester.pumpAndSettle();
-        expect(
+      await tester.pumpWidget(_boilerplate(null,
+        snap: true,
+        stackKey: stackKey,
+        containerKey: containerKey,
+        snapSizes: snapSizes,
+      ));
+      await tester.pumpAndSettle();
+      final double screenHeight = tester.getSize(find.byKey(stackKey)).height;
+
+      await tester.drag(find.text('Item 1'), Offset(0, -.4 * screenHeight));
+      await tester.pumpAndSettle();
+      expect(
           tester.getSize(find.byKey(containerKey)).height / screenHeight,
           closeTo(1.0, precisionErrorTolerance,
-        ));
+          ));
 
-        await tester.drag(find.text('Item 1'), Offset(0, .7 * screenHeight));
-        await tester.pumpAndSettle();
-        expect(
-          tester.getSize(find.byKey(containerKey)).height / screenHeight,
-          closeTo(.25, precisionErrorTolerance),
-        );
+      await tester.drag(find.text('Item 1'), Offset(0, .7 * screenHeight));
+      await tester.pumpAndSettle();
+      expect(
+        tester.getSize(find.byKey(containerKey)).height / screenHeight,
+        closeTo(.25, precisionErrorTolerance),
+      );
     }, variant: TargetPlatformVariant.all());
   }
 
-  testWidgets('Min and max are implicitly added to snapSizes.', (WidgetTester tester) async {
+  testWidgets('Min and max are implicitly added to snapSizes', (WidgetTester tester) async {
     const Key stackKey = ValueKey<String>('stack');
     const Key containerKey = ValueKey<String>('container');
     await tester.pumpWidget(_boilerplate(null,
@@ -481,6 +484,114 @@ void main() {
     );
   }, variant: TargetPlatformVariant.all());
 
+  testWidgets('Changes to widget parameters are propagated', (WidgetTester tester) async {
+    const Key stackKey = ValueKey<String>('stack');
+    const Key containerKey = ValueKey<String>('container');
+    await tester.pumpWidget(_boilerplate(
+      null,
+      stackKey: stackKey,
+      containerKey: containerKey,
+    ));
+    await tester.pumpAndSettle();
+    final double screenHeight = tester.getSize(find.byKey(stackKey)).height;
+    expect(
+      tester.getSize(find.byKey(containerKey)).height / screenHeight,
+      closeTo(.5, precisionErrorTolerance),
+    );
+
+    // Pump the same widget but with a new initial child size.
+    await tester.pumpWidget(_boilerplate(
+      null,
+      stackKey: stackKey,
+      containerKey: containerKey,
+      initialChildSize: .6,
+    ));
+    await tester.pumpAndSettle();
+
+    // We jump to the new initial size because the sheet hasn't changed yet.
+    expect(
+      tester.getSize(find.byKey(containerKey)).height / screenHeight,
+      closeTo(.6, precisionErrorTolerance),
+    );
+
+    // Pump the same widget but with a new max child size.
+    await tester.pumpWidget(_boilerplate(
+      null,
+      stackKey: stackKey,
+      containerKey: containerKey,
+      initialChildSize: .6,
+      maxChildSize: .9
+    ));
+    await tester.pumpAndSettle();
+
+    expect(
+      tester.getSize(find.byKey(containerKey)).height / screenHeight,
+      closeTo(.6, precisionErrorTolerance),
+    );
+
+    await tester.drag(find.text('Item 1'), Offset(0, -.6 * screenHeight));
+    await tester.pumpAndSettle();
+    expect(
+      tester.getSize(find.byKey(containerKey)).height / screenHeight,
+      closeTo(.9, precisionErrorTolerance),
+    );
+
+    // Pump the same widget but with a new max child size and initial size.
+    await tester.pumpWidget(_boilerplate(
+      null,
+      stackKey: stackKey,
+      containerKey: containerKey,
+      maxChildSize: .8,
+      initialChildSize: .7,
+    ));
+    await tester.pumpAndSettle();
+
+    // The max child size has been reduced, we should be rebuilt at the new
+    // max of .8. We changed the initial size again, but the sheet has already
+    // been changed so the new initial is ignored.
+    expect(
+      tester.getSize(find.byKey(containerKey)).height / screenHeight,
+      closeTo(.8, precisionErrorTolerance),
+    );
+
+    await tester.drag(find.text('Item 1'), Offset(0, .2 * screenHeight));
+
+    // Pump the same widget but with snapping enabled.
+    await tester.pumpWidget(_boilerplate(
+      null,
+      snap: true,
+      stackKey: stackKey,
+      containerKey: containerKey,
+      maxChildSize: .8,
+      snapSizes: <double>[.5],
+    ));
+    await tester.pumpAndSettle();
+
+    // Sheet snaps immediately on a change to snap.
+    expect(
+      tester.getSize(find.byKey(containerKey)).height / screenHeight,
+      closeTo(.5, precisionErrorTolerance),
+    );
+
+    final List<double> snapSizes = <double>[.6];
+
+    // Change the snap sizes.
+    await tester.pumpWidget(_boilerplate(
+      null,
+      snap: true,
+      stackKey: stackKey,
+      containerKey: containerKey,
+      maxChildSize: .8,
+      snapSizes: snapSizes,
+    ));
+    await tester.pumpAndSettle();
+
+    expect(
+      tester.getSize(find.byKey(containerKey)).height / screenHeight,
+      closeTo(.6, precisionErrorTolerance),
+    );
+  }, variant: TargetPlatformVariant.all());
+
   testWidgets('Fling snaps in direction of momentum', (WidgetTester tester) async {
     const Key stackKey = ValueKey<String>('stack');
     const Key containerKey = ValueKey<String>('container');
@@ -509,6 +620,21 @@ void main() {
 
   }, variant: TargetPlatformVariant.all());
 
+  testWidgets("Changing parameters with an un-listened controller doesn't throw", (WidgetTester tester) async {
+    await tester.pumpWidget(_boilerplate(
+      null,
+      snap: true,
+      // Will prevent the sheet's child from listening to the controller.
+      ignoreController: true,
+    ));
+    await tester.pumpAndSettle();
+    await tester.pumpWidget(_boilerplate(
+      null,
+      snap: true,
+    ));
+    await tester.pumpAndSettle();
+  }, variant: TargetPlatformVariant.all());
+
   testWidgets('ScrollNotification correctly dispatched when flung without covering its container', (WidgetTester tester) async {
     final List<Type> notificationTypes = <Type>[];
     await tester.pumpWidget(_boilerplate(