From 90d6303ff5e03dfdf56490df229ec8b03efa6641 Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Thu, 30 Jun 2022 15:26:10 -0500 Subject: [PATCH] Fix DraggableScrollableSheet crash when switching out scrollables (#105549) --- .../widgets/draggable_scrollable_sheet.dart | 24 +++++--- .../draggable_scrollable_sheet_test.dart | 57 +++++++++++++++++++ 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart b/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart index 9550969fe5..a721548dc6 100644 --- a/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart +++ b/packages/flutter/lib/src/widgets/draggable_scrollable_sheet.dart @@ -631,7 +631,7 @@ class _DraggableScrollableSheetState extends State { @override void didUpdateWidget(covariant DraggableScrollableSheet oldWidget) { super.didUpdateWidget(oldWidget); - _replaceExtent(); + _replaceExtent(oldWidget); } @override @@ -671,7 +671,7 @@ class _DraggableScrollableSheetState extends State { super.dispose(); } - void _replaceExtent() { + void _replaceExtent(covariant DraggableScrollableSheet oldWidget) { final _DraggableSheetExtent previousExtent = _extent; _extent.dispose(); _extent = _extent.copyWith( @@ -688,13 +688,21 @@ class _DraggableScrollableSheetState extends State { // If an external facing controller was provided, let it know that the // extent has been replaced. widget.controller?._onExtentReplaced(previousExtent); - if (widget.snap) { - // Trigger a snap in case snap or snapSizes has changed. 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. + if (widget.snap + && (widget.snap != oldWidget.snap || widget.snapSizes != oldWidget.snapSizes) + && _scrollController.hasClients + ) { + // 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) { - _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); + } }); } } diff --git a/packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart b/packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart index 1ac573e810..8781f1f99b 100644 --- a/packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart +++ b/packages/flutter/test/widgets/draggable_scrollable_sheet_test.dart @@ -745,6 +745,63 @@ void main() { await tester.pumpAndSettle(); }, 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 [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: [ + ElevatedButton( + onPressed: () => setState(() => ++s), + child: const Text('Switch to 2'), + ), + Container( + height: 400, + color: Colors.blue, + ), + ], + ) + : SingleChildScrollView( + child: Column( + children: [ + 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 { final List notificationTypes = []; await tester.pumpWidget(boilerplateWidget(