Unverified Commit 72163a89 authored by Casey Rogers's avatar Casey Rogers Committed by GitHub

Fixes issue with sheet reset on rebuild (#107876)

parent 2bebd9f5
...@@ -86,8 +86,8 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -86,8 +86,8 @@ class DraggableScrollableController extends ChangeNotifier {
return _attachedController!.extent.pixelsToSize(pixels); return _attachedController!.extent.pixelsToSize(pixels);
} }
/// Animates the attached sheet from its current size to [size] to the /// Animates the attached sheet from its current size to the given [size], a
/// provided new `size`, a fractional value of the parent container's height. /// fractional value of the parent container's height.
/// ///
/// Any active sheet animation is canceled. If the sheet's internal scrollable /// 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 /// is currently animating (e.g. responding to a user fling), that animation is
...@@ -101,6 +101,9 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -101,6 +101,9 @@ class DraggableScrollableController extends ChangeNotifier {
/// The duration must not be zero. To jump to a particular value without an /// The duration must not be zero. To jump to a particular value without an
/// animation, use [jumpTo]. /// 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 /// When calling [animateTo] in widget tests, `await`ing the returned
/// [Future] may cause the test to hang and timeout. Instead, use /// [Future] may cause the test to hang and timeout. Instead, use
/// [WidgetTester.pumpAndSettle]. /// [WidgetTester.pumpAndSettle].
...@@ -120,6 +123,7 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -120,6 +123,7 @@ class DraggableScrollableController extends ChangeNotifier {
_attachedController!.position.goIdle(); _attachedController!.position.goIdle();
// This disables any snapping until the next user interaction with the sheet. // This disables any snapping until the next user interaction with the sheet.
_attachedController!.extent.hasDragged = false; _attachedController!.extent.hasDragged = false;
_attachedController!.extent.hasChanged = true;
_attachedController!.extent.startActivity(onCanceled: () { _attachedController!.extent.startActivity(onCanceled: () {
// Don't stop the controller if it's already finished and may have been disposed. // Don't stop the controller if it's already finished and may have been disposed.
if (animationController.isAnimating) { if (animationController.isAnimating) {
...@@ -149,6 +153,9 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -149,6 +153,9 @@ class DraggableScrollableController extends ChangeNotifier {
/// Any active sheet animation is canceled. If the sheet's inner scrollable /// 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 /// is currently animating (e.g. responding to a user fling), that animation is
/// canceled as well. /// 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) { void jumpTo(double size) {
_assertAttached(); _assertAttached();
assert(size >= 0 && size <= 1); assert(size >= 0 && size <= 1);
...@@ -156,6 +163,7 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -156,6 +163,7 @@ class DraggableScrollableController extends ChangeNotifier {
_attachedController!.extent.startActivity(onCanceled: () {}); _attachedController!.extent.startActivity(onCanceled: () {});
_attachedController!.position.goIdle(); _attachedController!.position.goIdle();
_attachedController!.extent.hasDragged = false; _attachedController!.extent.hasDragged = false;
_attachedController!.extent.hasChanged = true;
_attachedController!.extent.updateSize(size, _attachedController!.position.context.notificationContext!); _attachedController!.extent.updateSize(size, _attachedController!.position.context.notificationContext!);
} }
...@@ -231,6 +239,10 @@ class DraggableScrollableController extends ChangeNotifier { ...@@ -231,6 +239,10 @@ class DraggableScrollableController extends ChangeNotifier {
/// [minChildSize] and [maxChildSize]. Use [snapSizes] to add more sizes for /// [minChildSize] and [maxChildSize]. Use [snapSizes] to add more sizes for
/// the sheet to snap between. /// 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 /// 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 /// 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 /// to position sheet based on the space it is taking, the [expand] property
...@@ -342,6 +354,9 @@ class DraggableScrollableSheet extends StatefulWidget { ...@@ -342,6 +354,9 @@ class DraggableScrollableSheet extends StatefulWidget {
/// snap to the next snap size (see [snapSizes]) in the direction of the drag. /// 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. /// 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 /// Rebuilding the sheet with snap newly enabled will immediately trigger a
/// snap unless the sheet has not yet been dragged away from /// snap unless the sheet has not yet been dragged away from
/// [initialChildSize] since first being built or since the last call to /// [initialChildSize] since first being built or since the last call to
...@@ -477,6 +492,7 @@ class _DraggableSheetExtent { ...@@ -477,6 +492,7 @@ class _DraggableSheetExtent {
this.snapAnimationDuration, this.snapAnimationDuration,
ValueNotifier<double>? currentSize, ValueNotifier<double>? currentSize,
bool? hasDragged, bool? hasDragged,
bool? hasChanged,
}) : assert(minSize != null), }) : assert(minSize != null),
assert(maxSize != null), assert(maxSize != null),
assert(initialSize != null), assert(initialSize != null),
...@@ -487,7 +503,8 @@ class _DraggableSheetExtent { ...@@ -487,7 +503,8 @@ class _DraggableSheetExtent {
_currentSize = (currentSize ?? ValueNotifier<double>(initialSize)) _currentSize = (currentSize ?? ValueNotifier<double>(initialSize))
..addListener(onSizeChanged), ..addListener(onSizeChanged),
availablePixels = double.infinity, availablePixels = double.infinity,
hasDragged = hasDragged ?? false; hasDragged = hasDragged ?? false,
hasChanged = hasChanged ?? false;
VoidCallback? _cancelActivity; VoidCallback? _cancelActivity;
...@@ -501,10 +518,20 @@ class _DraggableSheetExtent { ...@@ -501,10 +518,20 @@ class _DraggableSheetExtent {
final VoidCallback onSizeChanged; final VoidCallback onSizeChanged;
double availablePixels; double availablePixels;
// Used to disable snapping until the user has dragged on the sheet. We do // Used to disable snapping until the user has dragged on the sheet.
// this because we don't want to snap away from an initial or programmatically set size.
bool hasDragged; 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 isAtMin => minSize >= _currentSize.value;
bool get isAtMax => maxSize <= _currentSize.value; bool get isAtMax => maxSize <= _currentSize.value;
...@@ -538,6 +565,7 @@ class _DraggableSheetExtent { ...@@ -538,6 +565,7 @@ class _DraggableSheetExtent {
// The user has interacted with the sheet, set `hasDragged` to true so that // The user has interacted with the sheet, set `hasDragged` to true so that
// we'll snap if applicable. // we'll snap if applicable.
hasDragged = true; hasDragged = true;
hasChanged = true;
if (availablePixels == 0) { if (availablePixels == 0) {
return; return;
} }
...@@ -590,11 +618,13 @@ class _DraggableSheetExtent { ...@@ -590,11 +618,13 @@ class _DraggableSheetExtent {
snapAnimationDuration: snapAnimationDuration, snapAnimationDuration: snapAnimationDuration,
initialSize: initialSize, initialSize: initialSize,
onSizeChanged: onSizeChanged, onSizeChanged: onSizeChanged,
// Use the possibly updated initialSize if the user hasn't dragged yet. // Set the current size to the possibly updated initial size if the sheet
currentSize: ValueNotifier<double>(hasDragged // hasn't changed yet.
currentSize: ValueNotifier<double>(hasChanged
? clampDouble(_currentSize.value, minSize, maxSize) ? clampDouble(_currentSize.value, minSize, maxSize)
: initialSize), : initialSize),
hasDragged: hasDragged, hasDragged: hasDragged,
hasChanged: hasChanged,
); );
} }
} }
...@@ -785,6 +815,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController { ...@@ -785,6 +815,7 @@ class _DraggableScrollableSheetScrollController extends ScrollController {
void reset() { void reset() {
extent._cancelActivity?.call(); extent._cancelActivity?.call();
extent.hasDragged = false; extent.hasDragged = false;
extent.hasChanged = false;
// jumpTo can result in trying to replace semantics during build. // jumpTo can result in trying to replace semantics during build.
// Just animate really fast. // Just animate really fast.
// Avoid doing it at all if the offset is already 0.0. // Avoid doing it at all if the offset is already 0.0.
......
...@@ -1361,7 +1361,7 @@ void main() { ...@@ -1361,7 +1361,7 @@ void main() {
expect(() => controller.animateTo(.5, duration: Duration.zero, curve: Curves.linear), throwsAssertionError); 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(); final DraggableScrollableController controller = DraggableScrollableController();
expect(controller.isAttached, false); expect(controller.isAttached, false);
expect(()=>controller.size, throwsAssertionError); expect(()=>controller.size, throwsAssertionError);
...@@ -1389,4 +1389,70 @@ void main() { ...@@ -1389,4 +1389,70 @@ void main() {
expect(controller.isAttached, false); expect(controller.isAttached, false);
expect(tester.takeException(), isNull); 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),
);
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment