From 8ff1b6eb5e6377e0e724b32ccb6dd4997036eb8c Mon Sep 17 00:00:00 2001 From: Justin McCandless Date: Wed, 21 Dec 2022 13:58:23 -0800 Subject: [PATCH] Fix Scaffold bottomSheet null exceptions (#117008) * Prevent possibility of null exceptions on widget.bottomSheet * New approach that fixes bug in updating the bottomSheet * Real-world test for bottomSheet error * Allow bottomSheet to animate out after being killed, even if it was rebuilt * Go back to the simple solution of SizedBox.shrink --- .../flutter/lib/src/material/scaffold.dart | 4 +- .../flutter/test/material/scaffold_test.dart | 83 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/packages/flutter/lib/src/material/scaffold.dart b/packages/flutter/lib/src/material/scaffold.dart index 3e6c02b8ac7e..a9c3c46ebcd4 100644 --- a/packages/flutter/lib/src/material/scaffold.dart +++ b/packages/flutter/lib/src/material/scaffold.dart @@ -2230,7 +2230,9 @@ class ScaffoldState extends State with TickerProviderStateMixin, Resto child: DraggableScrollableActuator( child: StatefulBuilder( key: _currentBottomSheetKey, - builder: (BuildContext context, StateSetter setState) => widget.bottomSheet!, + builder: (BuildContext context, StateSetter setState) { + return widget.bottomSheet ?? const SizedBox.shrink(); + }, ), ), ); diff --git a/packages/flutter/test/material/scaffold_test.dart b/packages/flutter/test/material/scaffold_test.dart index 893009d15092..8e69edb43bf8 100644 --- a/packages/flutter/test/material/scaffold_test.dart +++ b/packages/flutter/test/material/scaffold_test.dart @@ -11,6 +11,9 @@ import 'package:flutter_test/flutter_test.dart'; import '../widgets/semantics_tester.dart'; +// From bottom_sheet.dart. +const Duration _bottomSheetExitDuration = Duration(milliseconds: 200); + void main() { // Regression test for https://github.com/flutter/flutter/issues/103741 testWidgets('extendBodyBehindAppBar change should not cause the body widget lose state', (WidgetTester tester) async { @@ -2622,6 +2625,86 @@ void main() { matchesSemantics(label: 'BottomSheet', hasDismissAction: false), ); }); + + // Regression test for https://github.com/flutter/flutter/issues/117004 + testWidgets('can rebuild and remove bottomSheet at the same time', (WidgetTester tester) async { + bool themeIsLight = true; + bool? defaultBottomSheet = true; + final GlobalKey bottomSheetKey1 = GlobalKey(); + final GlobalKey bottomSheetKey2 = GlobalKey(); + late StateSetter setState; + + await tester.pumpWidget( + StatefulBuilder( + builder: (BuildContext context, StateSetter stateSetter) { + setState = stateSetter; + return MaterialApp( + theme: themeIsLight ? ThemeData.light() : ThemeData.dark(), + home: Scaffold( + bottomSheet: defaultBottomSheet == null + ? null + : defaultBottomSheet! + ? Container( + key: bottomSheetKey1, + width: double.infinity, + height: 100, + color: Colors.blue, + child: const Text('BottomSheet'), + ) + : Container( + key: bottomSheetKey2, + width: double.infinity, + height: 100, + color: Colors.red, + child: const Text('BottomSheet'), + ), + body: const Placeholder(), + ), + ); + }, + ), + ); + + expect(find.byKey(bottomSheetKey1), findsOneWidget); + expect(find.byKey(bottomSheetKey2), findsNothing); + + // Change to the other bottomSheet. + setState(() { + defaultBottomSheet = false; + }); + expect(find.byKey(bottomSheetKey1), findsOneWidget); + expect(find.byKey(bottomSheetKey2), findsNothing); + await tester.pumpAndSettle(); + expect(find.byKey(bottomSheetKey1), findsNothing); + expect(find.byKey(bottomSheetKey2), findsOneWidget); + + // Set bottomSheet to null, which starts its exit animation. + setState(() { + defaultBottomSheet = null; + }); + expect(find.byKey(bottomSheetKey1), findsNothing); + expect(find.byKey(bottomSheetKey2), findsOneWidget); + + // While the bottomSheet is on the way out, change the theme to cause it to + // rebuild. + setState(() { + themeIsLight = false; + }); + expect(find.byKey(bottomSheetKey1), findsNothing); + expect(find.byKey(bottomSheetKey2), findsOneWidget); + + // The most recent bottomSheet remains on screen during the exit animation. + await tester.pump(_bottomSheetExitDuration); + expect(find.byKey(bottomSheetKey1), findsNothing); + expect(find.byKey(bottomSheetKey2), findsOneWidget); + + // After animating out, the bottomSheet is gone. + await tester.pumpAndSettle(); + expect(find.byKey(bottomSheetKey1), findsNothing); + expect(find.byKey(bottomSheetKey2), findsNothing); + + expect(tester.takeException(), isNull); + }); } class _GeometryListener extends StatefulWidget {