Unverified Commit 90d6303f authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Fix DraggableScrollableSheet crash when switching out scrollables (#105549)

parent 759f36b4
...@@ -631,7 +631,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> { ...@@ -631,7 +631,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
@override @override
void didUpdateWidget(covariant DraggableScrollableSheet oldWidget) { void didUpdateWidget(covariant DraggableScrollableSheet oldWidget) {
super.didUpdateWidget(oldWidget); super.didUpdateWidget(oldWidget);
_replaceExtent(); _replaceExtent(oldWidget);
} }
@override @override
...@@ -671,7 +671,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> { ...@@ -671,7 +671,7 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
super.dispose(); super.dispose();
} }
void _replaceExtent() { void _replaceExtent(covariant DraggableScrollableSheet oldWidget) {
final _DraggableSheetExtent previousExtent = _extent; final _DraggableSheetExtent previousExtent = _extent;
_extent.dispose(); _extent.dispose();
_extent = _extent.copyWith( _extent = _extent.copyWith(
...@@ -688,13 +688,21 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> { ...@@ -688,13 +688,21 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
// If an external facing controller was provided, let it know that the // If an external facing controller was provided, let it know that the
// extent has been replaced. // extent has been replaced.
widget.controller?._onExtentReplaced(previousExtent); widget.controller?._onExtentReplaced(previousExtent);
if (widget.snap) { if (widget.snap
// Trigger a snap in case snap or snapSizes has changed. We put this in a && (widget.snap != oldWidget.snap || widget.snapSizes != oldWidget.snapSizes)
// post frame callback so that `build` can update `_extent.availablePixels` && _scrollController.hasClients
// before this runs-we can't use the previous extent's available pixels as ) {
// it may have changed when the widget was updated. // Trigger a snap in case snap or snapSizes has changed and there is a
// scroll position currently attached. 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) { WidgetsBinding.instance.addPostFrameCallback((Duration timeStamp) {
_scrollController.position.goBallistic(0); for (int index = 0; index < _scrollController.positions.length; index++) {
final _DraggableScrollableSheetScrollPosition position =
_scrollController.positions.elementAt(index) as _DraggableScrollableSheetScrollPosition;
position.goBallistic(0);
}
}); });
} }
} }
......
...@@ -745,6 +745,63 @@ void main() { ...@@ -745,6 +745,63 @@ void main() {
await tester.pumpAndSettle(); await tester.pumpAndSettle();
}, variant: TargetPlatformVariant.all()); }, variant: TargetPlatformVariant.all());
testWidgets('Transitioning between scrollable children sharing a scroll controller will not throw', (WidgetTester tester) async {
int s = 0;
await tester.pumpWidget(MaterialApp(
home: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
return Scaffold(
body: DraggableScrollableSheet(
initialChildSize: 0.25,
snap: true,
snapSizes: const <double>[0.25, 0.5, 1.0],
builder: (BuildContext context, ScrollController scrollController) {
return PrimaryScrollController(
controller: scrollController,
child: AnimatedSwitcher(
duration: const Duration(milliseconds: 500),
child: (s.isEven)
? ListView(
children: <Widget>[
ElevatedButton(
onPressed: () => setState(() => ++s),
child: const Text('Switch to 2'),
),
Container(
height: 400,
color: Colors.blue,
),
],
)
: SingleChildScrollView(
child: Column(
children: <Widget>[
ElevatedButton(
onPressed: () => setState(() => ++s),
child: const Text('Switch to 1'),
),
Container(
height: 400,
color: Colors.blue,
),
],
)
),
),
);
},
),
);
},
),
));
// Trigger the AnimatedSwitcher between ListViews
await tester.tap(find.text('Switch to 2'));
await tester.pump();
// Completes without throwing
});
testWidgets('ScrollNotification correctly dispatched when flung without covering its container', (WidgetTester tester) async { testWidgets('ScrollNotification correctly dispatched when flung without covering its container', (WidgetTester tester) async {
final List<Type> notificationTypes = <Type>[]; final List<Type> notificationTypes = <Type>[];
await tester.pumpWidget(boilerplateWidget( await tester.pumpWidget(boilerplateWidget(
......
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