Skip to content

Commit

Permalink
Fix Scaffold bottomSheet null exceptions (#117008)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
justinmc authored Dec 21, 2022
1 parent ddb7e43 commit 8ff1b6e
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 1 deletion.
4 changes: 3 additions & 1 deletion packages/flutter/lib/src/material/scaffold.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2230,7 +2230,9 @@ class ScaffoldState extends State<Scaffold> 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();
},
),
),
);
Expand Down
83 changes: 83 additions & 0 deletions packages/flutter/test/material/scaffold_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 8ff1b6e

Please sign in to comment.