Skip to content

Commit

Permalink
Fixes issue with sheet reset on rebuild (#107876)
Browse files Browse the repository at this point in the history
  • Loading branch information
caseycrogers authored Jul 25, 2022
1 parent 2bebd9f commit 72163a8
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 8 deletions.
45 changes: 38 additions & 7 deletions packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ class DraggableScrollableController extends ChangeNotifier {
return _attachedController!.extent.pixelsToSize(pixels);
}

/// Animates the attached sheet from its current size to [size] to the
/// provided new `size`, a fractional value of the parent container's height.
/// Animates the attached sheet from its current size to the given [size], a
/// fractional value of the parent container's height.
///
/// Any active sheet animation is canceled. If the sheet's internal scrollable
/// is currently animating (e.g. responding to a user fling), that animation is
Expand All @@ -101,6 +101,9 @@ class DraggableScrollableController extends ChangeNotifier {
/// The duration must not be zero. To jump to a particular value without an
/// animation, use [jumpTo].
///
/// The sheet will not snap after calling [animateTo] even if [DraggableScrollableSheet.snap]
/// is true. Snapping only occurs after user drags.
///
/// When calling [animateTo] in widget tests, `await`ing the returned
/// [Future] may cause the test to hang and timeout. Instead, use
/// [WidgetTester.pumpAndSettle].
Expand All @@ -120,6 +123,7 @@ class DraggableScrollableController extends ChangeNotifier {
_attachedController!.position.goIdle();
// This disables any snapping until the next user interaction with the sheet.
_attachedController!.extent.hasDragged = false;
_attachedController!.extent.hasChanged = true;
_attachedController!.extent.startActivity(onCanceled: () {
// Don't stop the controller if it's already finished and may have been disposed.
if (animationController.isAnimating) {
Expand Down Expand Up @@ -149,13 +153,17 @@ class DraggableScrollableController extends ChangeNotifier {
/// Any active sheet animation is canceled. If the sheet's inner scrollable
/// is currently animating (e.g. responding to a user fling), that animation is
/// canceled as well.
///
/// The sheet will not snap after calling [jumpTo] even if [DraggableScrollableSheet.snap]
/// is true. Snapping only occurs after user drags.
void jumpTo(double size) {
_assertAttached();
assert(size >= 0 && size <= 1);
// Call start activity to interrupt any other playing activities.
_attachedController!.extent.startActivity(onCanceled: () {});
_attachedController!.position.goIdle();
_attachedController!.extent.hasDragged = false;
_attachedController!.extent.hasChanged = true;
_attachedController!.extent.updateSize(size, _attachedController!.position.context.notificationContext!);
}

Expand Down Expand Up @@ -231,6 +239,10 @@ class DraggableScrollableController extends ChangeNotifier {
/// [minChildSize] and [maxChildSize]. Use [snapSizes] to add more sizes for
/// the sheet to snap between.
///
/// The snapping effect is only applied on user drags. Programmatically
/// manipulating the sheet size via [DraggableScrollableController.animateTo] or
/// [DraggableScrollableController.jumpTo] will ignore [snap] and [snapSizes].
///
/// By default, the widget will expand its non-occupied area to fill available
/// space in the parent. If this is not desired, e.g. because the parent wants
/// to position sheet based on the space it is taking, the [expand] property
Expand Down Expand Up @@ -342,6 +354,9 @@ class DraggableScrollableSheet extends StatefulWidget {
/// 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.
///
/// Snapping is not applied when the sheet is programmatically moved by
/// calling [DraggableScrollableController.animateTo] or [DraggableScrollableController.jumpTo].
///
/// 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
Expand Down Expand Up @@ -477,6 +492,7 @@ class _DraggableSheetExtent {
this.snapAnimationDuration,
ValueNotifier<double>? currentSize,
bool? hasDragged,
bool? hasChanged,
}) : assert(minSize != null),
assert(maxSize != null),
assert(initialSize != null),
Expand All @@ -487,7 +503,8 @@ class _DraggableSheetExtent {
_currentSize = (currentSize ?? ValueNotifier<double>(initialSize))
..addListener(onSizeChanged),
availablePixels = double.infinity,
hasDragged = hasDragged ?? false;
hasDragged = hasDragged ?? false,
hasChanged = hasChanged ?? false;

VoidCallback? _cancelActivity;

Expand All @@ -501,10 +518,20 @@ class _DraggableSheetExtent {
final VoidCallback onSizeChanged;
double availablePixels;

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

// Used to determine if the sheet should move to a new initial size when it
// changes.
// We need both `hasChanged` and `hasDragged` to achieve the following
// behavior:
// 1. The sheet should only snap following user drags (as opposed to
// programmatic sheet changes). See docs for `animateTo` and `jumpTo`.
// 2. The sheet should move to a new initial child size on rebuild iff the
// sheet has not changed, either by drag or programmatic control. See
// docs for `initialChildSize`.
bool hasChanged;

bool get isAtMin => minSize >= _currentSize.value;
bool get isAtMax => maxSize <= _currentSize.value;

Expand Down Expand Up @@ -538,6 +565,7 @@ class _DraggableSheetExtent {
// The user has interacted with the sheet, set `hasDragged` to true so that
// we'll snap if applicable.
hasDragged = true;
hasChanged = true;
if (availablePixels == 0) {
return;
}
Expand Down Expand Up @@ -590,11 +618,13 @@ class _DraggableSheetExtent {
snapAnimationDuration: snapAnimationDuration,
initialSize: initialSize,
onSizeChanged: onSizeChanged,
// Use the possibly updated initialSize if the user hasn't dragged yet.
currentSize: ValueNotifier<double>(hasDragged
// Set the current size to the possibly updated initial size if the sheet
// hasn't changed yet.
currentSize: ValueNotifier<double>(hasChanged
? clampDouble(_currentSize.value, minSize, maxSize)
: initialSize),
hasDragged: hasDragged,
hasChanged: hasChanged,
);
}
}
Expand Down Expand Up @@ -785,6 +815,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
void reset() {
extent._cancelActivity?.call();
extent.hasDragged = false;
extent.hasChanged = false;
// jumpTo can result in trying to replace semantics during build.
// Just animate really fast.
// Avoid doing it at all if the offset is already 0.0.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ void main() {
expect(() => controller.animateTo(.5, duration: Duration.zero, curve: Curves.linear), throwsAssertionError);
});

testWidgets('DraggableScrollableController must be attached before using any of its paramters', (WidgetTester tester) async {
testWidgets('DraggableScrollableController must be attached before using any of its parameters', (WidgetTester tester) async {
final DraggableScrollableController controller = DraggableScrollableController();
expect(controller.isAttached, false);
expect(()=>controller.size, throwsAssertionError);
Expand Down Expand Up @@ -1389,4 +1389,70 @@ void main() {
expect(controller.isAttached, false);
expect(tester.takeException(), isNull);
});

testWidgets('DraggableScrollableSheet should not reset programmatic drag on rebuild', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/101114
const Key stackKey = ValueKey<String>('stack');
const Key containerKey = ValueKey<String>('container');
final DraggableScrollableController controller = DraggableScrollableController();
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
stackKey: stackKey,
containerKey: containerKey,
));
await tester.pumpAndSettle();
final double screenHeight = tester.getSize(find.byKey(stackKey)).height;

controller.jumpTo(.6);
await tester.pumpAndSettle();
expect(
tester.getSize(find.byKey(containerKey)).height / screenHeight,
closeTo(.6, precisionErrorTolerance),
);

// Force an arbitrary rebuild by pushing a new widget.
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
stackKey: stackKey,
containerKey: containerKey,
));
// Sheet remains at .6.
expect(
tester.getSize(find.byKey(containerKey)).height / screenHeight,
closeTo(.6, precisionErrorTolerance),
);

controller.reset();
await tester.pumpAndSettle();
expect(
tester.getSize(find.byKey(containerKey)).height / screenHeight,
closeTo(.5, precisionErrorTolerance),
);

controller.animateTo(
.6,
curve: Curves.linear,
duration: const Duration(milliseconds: 200),
);
await tester.pumpAndSettle();
expect(
tester.getSize(find.byKey(containerKey)).height / screenHeight,
closeTo(.6, precisionErrorTolerance),
);

// Force an arbitrary rebuild by pushing a new widget.
await tester.pumpWidget(boilerplateWidget(
null,
controller: controller,
stackKey: stackKey,
containerKey: containerKey,
));
// Sheet remains at .6.
expect(
tester.getSize(find.byKey(containerKey)).height / screenHeight,
closeTo(.6, precisionErrorTolerance),
);
});
}

0 comments on commit 72163a8

Please sign in to comment.