Skip to content

Commit 72163a8

Browse files
authored
Fixes issue with sheet reset on rebuild (#107876)
1 parent 2bebd9f commit 72163a8

File tree

2 files changed

+105
-8
lines changed

2 files changed

+105
-8
lines changed

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

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ class DraggableScrollableController extends ChangeNotifier {
8686
return _attachedController!.extent.pixelsToSize(pixels);
8787
}
8888

89-
/// Animates the attached sheet from its current size to [size] to the
90-
/// provided new `size`, a fractional value of the parent container's height.
89+
/// Animates the attached sheet from its current size to the given [size], a
90+
/// fractional value of the parent container's height.
9191
///
9292
/// Any active sheet animation is canceled. If the sheet's internal scrollable
9393
/// is currently animating (e.g. responding to a user fling), that animation is
@@ -101,6 +101,9 @@ class DraggableScrollableController extends ChangeNotifier {
101101
/// The duration must not be zero. To jump to a particular value without an
102102
/// animation, use [jumpTo].
103103
///
104+
/// The sheet will not snap after calling [animateTo] even if [DraggableScrollableSheet.snap]
105+
/// is true. Snapping only occurs after user drags.
106+
///
104107
/// When calling [animateTo] in widget tests, `await`ing the returned
105108
/// [Future] may cause the test to hang and timeout. Instead, use
106109
/// [WidgetTester.pumpAndSettle].
@@ -120,6 +123,7 @@ class DraggableScrollableController extends ChangeNotifier {
120123
_attachedController!.position.goIdle();
121124
// This disables any snapping until the next user interaction with the sheet.
122125
_attachedController!.extent.hasDragged = false;
126+
_attachedController!.extent.hasChanged = true;
123127
_attachedController!.extent.startActivity(onCanceled: () {
124128
// Don't stop the controller if it's already finished and may have been disposed.
125129
if (animationController.isAnimating) {
@@ -149,13 +153,17 @@ class DraggableScrollableController extends ChangeNotifier {
149153
/// Any active sheet animation is canceled. If the sheet's inner scrollable
150154
/// is currently animating (e.g. responding to a user fling), that animation is
151155
/// canceled as well.
156+
///
157+
/// The sheet will not snap after calling [jumpTo] even if [DraggableScrollableSheet.snap]
158+
/// is true. Snapping only occurs after user drags.
152159
void jumpTo(double size) {
153160
_assertAttached();
154161
assert(size >= 0 && size <= 1);
155162
// Call start activity to interrupt any other playing activities.
156163
_attachedController!.extent.startActivity(onCanceled: () {});
157164
_attachedController!.position.goIdle();
158165
_attachedController!.extent.hasDragged = false;
166+
_attachedController!.extent.hasChanged = true;
159167
_attachedController!.extent.updateSize(size, _attachedController!.position.context.notificationContext!);
160168
}
161169

@@ -231,6 +239,10 @@ class DraggableScrollableController extends ChangeNotifier {
231239
/// [minChildSize] and [maxChildSize]. Use [snapSizes] to add more sizes for
232240
/// the sheet to snap between.
233241
///
242+
/// The snapping effect is only applied on user drags. Programmatically
243+
/// manipulating the sheet size via [DraggableScrollableController.animateTo] or
244+
/// [DraggableScrollableController.jumpTo] will ignore [snap] and [snapSizes].
245+
///
234246
/// By default, the widget will expand its non-occupied area to fill available
235247
/// space in the parent. If this is not desired, e.g. because the parent wants
236248
/// to position sheet based on the space it is taking, the [expand] property
@@ -342,6 +354,9 @@ class DraggableScrollableSheet extends StatefulWidget {
342354
/// snap to the next snap size (see [snapSizes]) in the direction of the drag.
343355
/// If their finger was still, the widget will snap to the nearest snap size.
344356
///
357+
/// Snapping is not applied when the sheet is programmatically moved by
358+
/// calling [DraggableScrollableController.animateTo] or [DraggableScrollableController.jumpTo].
359+
///
345360
/// Rebuilding the sheet with snap newly enabled will immediately trigger a
346361
/// snap unless the sheet has not yet been dragged away from
347362
/// [initialChildSize] since first being built or since the last call to
@@ -477,6 +492,7 @@ class _DraggableSheetExtent {
477492
this.snapAnimationDuration,
478493
ValueNotifier<double>? currentSize,
479494
bool? hasDragged,
495+
bool? hasChanged,
480496
}) : assert(minSize != null),
481497
assert(maxSize != null),
482498
assert(initialSize != null),
@@ -487,7 +503,8 @@ class _DraggableSheetExtent {
487503
_currentSize = (currentSize ?? ValueNotifier<double>(initialSize))
488504
..addListener(onSizeChanged),
489505
availablePixels = double.infinity,
490-
hasDragged = hasDragged ?? false;
506+
hasDragged = hasDragged ?? false,
507+
hasChanged = hasChanged ?? false;
491508

492509
VoidCallback? _cancelActivity;
493510

@@ -501,10 +518,20 @@ class _DraggableSheetExtent {
501518
final VoidCallback onSizeChanged;
502519
double availablePixels;
503520

504-
// Used to disable snapping until the user has dragged on the sheet. We do
505-
// this because we don't want to snap away from an initial or programmatically set size.
521+
// Used to disable snapping until the user has dragged on the sheet.
506522
bool hasDragged;
507523

524+
// Used to determine if the sheet should move to a new initial size when it
525+
// changes.
526+
// We need both `hasChanged` and `hasDragged` to achieve the following
527+
// behavior:
528+
// 1. The sheet should only snap following user drags (as opposed to
529+
// programmatic sheet changes). See docs for `animateTo` and `jumpTo`.
530+
// 2. The sheet should move to a new initial child size on rebuild iff the
531+
// sheet has not changed, either by drag or programmatic control. See
532+
// docs for `initialChildSize`.
533+
bool hasChanged;
534+
508535
bool get isAtMin => minSize >= _currentSize.value;
509536
bool get isAtMax => maxSize <= _currentSize.value;
510537

@@ -538,6 +565,7 @@ class _DraggableSheetExtent {
538565
// The user has interacted with the sheet, set `hasDragged` to true so that
539566
// we'll snap if applicable.
540567
hasDragged = true;
568+
hasChanged = true;
541569
if (availablePixels == 0) {
542570
return;
543571
}
@@ -590,11 +618,13 @@ class _DraggableSheetExtent {
590618
snapAnimationDuration: snapAnimationDuration,
591619
initialSize: initialSize,
592620
onSizeChanged: onSizeChanged,
593-
// Use the possibly updated initialSize if the user hasn't dragged yet.
594-
currentSize: ValueNotifier<double>(hasDragged
621+
// Set the current size to the possibly updated initial size if the sheet
622+
// hasn't changed yet.
623+
currentSize: ValueNotifier<double>(hasChanged
595624
? clampDouble(_currentSize.value, minSize, maxSize)
596625
: initialSize),
597626
hasDragged: hasDragged,
627+
hasChanged: hasChanged,
598628
);
599629
}
600630
}
@@ -785,6 +815,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
785815
void reset() {
786816
extent._cancelActivity?.call();
787817
extent.hasDragged = false;
818+
extent.hasChanged = false;
788819
// jumpTo can result in trying to replace semantics during build.
789820
// Just animate really fast.
790821
// Avoid doing it at all if the offset is already 0.0.

packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1361,7 +1361,7 @@ void main() {
13611361
expect(() => controller.animateTo(.5, duration: Duration.zero, curve: Curves.linear), throwsAssertionError);
13621362
});
13631363

1364-
testWidgets('DraggableScrollableController must be attached before using any of its paramters', (WidgetTester tester) async {
1364+
testWidgets('DraggableScrollableController must be attached before using any of its parameters', (WidgetTester tester) async {
13651365
final DraggableScrollableController controller = DraggableScrollableController();
13661366
expect(controller.isAttached, false);
13671367
expect(()=>controller.size, throwsAssertionError);
@@ -1389,4 +1389,70 @@ void main() {
13891389
expect(controller.isAttached, false);
13901390
expect(tester.takeException(), isNull);
13911391
});
1392+
1393+
testWidgets('DraggableScrollableSheet should not reset programmatic drag on rebuild', (WidgetTester tester) async {
1394+
// Regression test for https://github.com/flutter/flutter/issues/101114
1395+
const Key stackKey = ValueKey<String>('stack');
1396+
const Key containerKey = ValueKey<String>('container');
1397+
final DraggableScrollableController controller = DraggableScrollableController();
1398+
await tester.pumpWidget(boilerplateWidget(
1399+
null,
1400+
controller: controller,
1401+
stackKey: stackKey,
1402+
containerKey: containerKey,
1403+
));
1404+
await tester.pumpAndSettle();
1405+
final double screenHeight = tester.getSize(find.byKey(stackKey)).height;
1406+
1407+
controller.jumpTo(.6);
1408+
await tester.pumpAndSettle();
1409+
expect(
1410+
tester.getSize(find.byKey(containerKey)).height / screenHeight,
1411+
closeTo(.6, precisionErrorTolerance),
1412+
);
1413+
1414+
// Force an arbitrary rebuild by pushing a new widget.
1415+
await tester.pumpWidget(boilerplateWidget(
1416+
null,
1417+
controller: controller,
1418+
stackKey: stackKey,
1419+
containerKey: containerKey,
1420+
));
1421+
// Sheet remains at .6.
1422+
expect(
1423+
tester.getSize(find.byKey(containerKey)).height / screenHeight,
1424+
closeTo(.6, precisionErrorTolerance),
1425+
);
1426+
1427+
controller.reset();
1428+
await tester.pumpAndSettle();
1429+
expect(
1430+
tester.getSize(find.byKey(containerKey)).height / screenHeight,
1431+
closeTo(.5, precisionErrorTolerance),
1432+
);
1433+
1434+
controller.animateTo(
1435+
.6,
1436+
curve: Curves.linear,
1437+
duration: const Duration(milliseconds: 200),
1438+
);
1439+
await tester.pumpAndSettle();
1440+
expect(
1441+
tester.getSize(find.byKey(containerKey)).height / screenHeight,
1442+
closeTo(.6, precisionErrorTolerance),
1443+
);
1444+
1445+
// Force an arbitrary rebuild by pushing a new widget.
1446+
await tester.pumpWidget(boilerplateWidget(
1447+
null,
1448+
controller: controller,
1449+
stackKey: stackKey,
1450+
containerKey: containerKey,
1451+
));
1452+
// Sheet remains at .6.
1453+
expect(
1454+
tester.getSize(find.byKey(containerKey)).height / screenHeight,
1455+
closeTo(.6, precisionErrorTolerance),
1456+
);
1457+
});
13921458
}

0 commit comments

Comments
 (0)