Skip to content

Commit 90d6303

Browse files
authored
Fix DraggableScrollableSheet crash when switching out scrollables (#105549)
1 parent 759f36b commit 90d6303

File tree

2 files changed

+73
-8
lines changed

2 files changed

+73
-8
lines changed

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
631631
@override
632632
void didUpdateWidget(covariant DraggableScrollableSheet oldWidget) {
633633
super.didUpdateWidget(oldWidget);
634-
_replaceExtent();
634+
_replaceExtent(oldWidget);
635635
}
636636

637637
@override
@@ -671,7 +671,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
671671
super.dispose();
672672
}
673673

674-
void _replaceExtent() {
674+
void _replaceExtent(covariant DraggableScrollableSheet oldWidget) {
675675
final _DraggableSheetExtent previousExtent = _extent;
676676
_extent.dispose();
677677
_extent = _extent.copyWith(
@@ -688,13 +688,21 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
688688
// If an external facing controller was provided, let it know that the
689689
// extent has been replaced.
690690
widget.controller?._onExtentReplaced(previousExtent);
691-
if (widget.snap) {
692-
// Trigger a snap in case snap or snapSizes has changed. We put this in a
693-
// post frame callback so that `build` can update `_extent.availablePixels`
694-
// before this runs-we can't use the previous extent's available pixels as
695-
// it may have changed when the widget was updated.
691+
if (widget.snap
692+
&& (widget.snap != oldWidget.snap || widget.snapSizes != oldWidget.snapSizes)
693+
&& _scrollController.hasClients
694+
) {
695+
// Trigger a snap in case snap or snapSizes has changed and there is a
696+
// scroll position currently attached. We put this in a post frame
697+
// callback so that `build` can update `_extent.availablePixels` before
698+
// this runs-we can't use the previous extent's available pixels as it may
699+
// have changed when the widget was updated.
696700
WidgetsBinding.instance.addPostFrameCallback((Duration timeStamp) {
697-
_scrollController.position.goBallistic(0);
701+
for (int index = 0; index < _scrollController.positions.length; index++) {
702+
final _DraggableScrollableSheetScrollPosition position =
703+
_scrollController.positions.elementAt(index) as _DraggableScrollableSheetScrollPosition;
704+
position.goBallistic(0);
705+
}
698706
});
699707
}
700708
}

packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,63 @@ void main() {
745745
await tester.pumpAndSettle();
746746
}, variant: TargetPlatformVariant.all());
747747

748+
testWidgets('Transitioning between scrollable children sharing a scroll controller will not throw', (WidgetTester tester) async {
749+
int s = 0;
750+
await tester.pumpWidget(MaterialApp(
751+
home: StatefulBuilder(
752+
builder: (BuildContext context, StateSetter setState) {
753+
return Scaffold(
754+
body: DraggableScrollableSheet(
755+
initialChildSize: 0.25,
756+
snap: true,
757+
snapSizes: const <double>[0.25, 0.5, 1.0],
758+
builder: (BuildContext context, ScrollController scrollController) {
759+
return PrimaryScrollController(
760+
controller: scrollController,
761+
child: AnimatedSwitcher(
762+
duration: const Duration(milliseconds: 500),
763+
child: (s.isEven)
764+
? ListView(
765+
children: <Widget>[
766+
ElevatedButton(
767+
onPressed: () => setState(() => ++s),
768+
child: const Text('Switch to 2'),
769+
),
770+
Container(
771+
height: 400,
772+
color: Colors.blue,
773+
),
774+
],
775+
)
776+
: SingleChildScrollView(
777+
child: Column(
778+
children: <Widget>[
779+
ElevatedButton(
780+
onPressed: () => setState(() => ++s),
781+
child: const Text('Switch to 1'),
782+
),
783+
Container(
784+
height: 400,
785+
color: Colors.blue,
786+
),
787+
],
788+
)
789+
),
790+
),
791+
);
792+
},
793+
),
794+
);
795+
},
796+
),
797+
));
798+
799+
// Trigger the AnimatedSwitcher between ListViews
800+
await tester.tap(find.text('Switch to 2'));
801+
await tester.pump();
802+
// Completes without throwing
803+
});
804+
748805
testWidgets('ScrollNotification correctly dispatched when flung without covering its container', (WidgetTester tester) async {
749806
final List<Type> notificationTypes = <Type>[];
750807
await tester.pumpWidget(boilerplateWidget(

0 commit comments

Comments
 (0)