Unverified Commit 1b310472 authored by chunhtai's avatar chunhtai Committed by GitHub

Fix canpop logic to be more robust (#73993)

parent b73722b1
...@@ -709,8 +709,6 @@ class _AppBarState extends State<AppBar> { ...@@ -709,8 +709,6 @@ class _AppBarState extends State<AppBar> {
Scaffold.of(context).openEndDrawer(); Scaffold.of(context).openEndDrawer();
} }
bool? hadBackButtonWhenRouteWasActive;
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
assert(!widget.primary || debugCheckHasMediaQuery(context)); assert(!widget.primary || debugCheckHasMediaQuery(context));
...@@ -723,11 +721,7 @@ class _AppBarState extends State<AppBar> { ...@@ -723,11 +721,7 @@ class _AppBarState extends State<AppBar> {
final bool hasDrawer = scaffold?.hasDrawer ?? false; final bool hasDrawer = scaffold?.hasDrawer ?? false;
final bool hasEndDrawer = scaffold?.hasEndDrawer ?? false; final bool hasEndDrawer = scaffold?.hasEndDrawer ?? false;
hadBackButtonWhenRouteWasActive ??= false; final bool canPop = parentRoute?.canPop ?? false;
if (parentRoute?.isActive == true) {
hadBackButtonWhenRouteWasActive = parentRoute!.canPop;
}
assert(hadBackButtonWhenRouteWasActive != null);
final bool useCloseButton = parentRoute is PageRoute<dynamic> && parentRoute.fullscreenDialog; final bool useCloseButton = parentRoute is PageRoute<dynamic> && parentRoute.fullscreenDialog;
final double toolbarHeight = widget.toolbarHeight ?? kToolbarHeight; final double toolbarHeight = widget.toolbarHeight ?? kToolbarHeight;
...@@ -796,7 +790,7 @@ class _AppBarState extends State<AppBar> { ...@@ -796,7 +790,7 @@ class _AppBarState extends State<AppBar> {
tooltip: MaterialLocalizations.of(context).openAppDrawerTooltip, tooltip: MaterialLocalizations.of(context).openAppDrawerTooltip,
); );
} else { } else {
if (!hasEndDrawer && hadBackButtonWhenRouteWasActive!) if (!hasEndDrawer && canPop)
leading = useCloseButton ? const CloseButton() : const BackButton(); leading = useCloseButton ? const CloseButton() : const BackButton();
} }
} }
......
...@@ -464,10 +464,7 @@ abstract class Route<T> { ...@@ -464,10 +464,7 @@ abstract class Route<T> {
return currentRouteEntry.route == this; return currentRouteEntry.route == this;
} }
/// Whether this route is the bottom-most route on the navigator. /// Whether this route is the bottom-most active route on the navigator.
///
/// If this is true, then [Navigator.canPop] will return false if this route's
/// [willHandlePopInternally] returns false.
/// ///
/// If [isFirst] and [isCurrent] are both true then this is the only route on /// If [isFirst] and [isCurrent] are both true then this is the only route on
/// the navigator (and [isActive] will also be true). /// the navigator (and [isActive] will also be true).
...@@ -483,6 +480,20 @@ abstract class Route<T> { ...@@ -483,6 +480,20 @@ abstract class Route<T> {
return currentRouteEntry.route == this; return currentRouteEntry.route == this;
} }
/// Whether there is at least one active route underneath this route.
@protected
bool get hasActiveRouteBelow {
if (_navigator == null)
return false;
for (final _RouteEntry entry in _navigator!._history) {
if (entry.route == this)
return false;
if (_RouteEntry.isPresentPredicate(entry))
return true;
}
return false;
}
/// Whether this route is on the navigator. /// Whether this route is on the navigator.
/// ///
/// If the route is not only active, but also the current route (the top-most /// If the route is not only active, but also the current route (the top-most
......
...@@ -1483,10 +1483,13 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -1483,10 +1483,13 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// Whether this route can be popped. /// Whether this route can be popped.
/// ///
/// A route can be popped if there is at least one active route below it, or
/// if [willHandlePopInternally] returns true.
///
/// When this changes, if the route is visible, the route will /// When this changes, if the route is visible, the route will
/// rebuild, and any widgets that used [ModalRoute.of] will be /// rebuild, and any widgets that used [ModalRoute.of] will be
/// notified. /// notified.
bool get canPop => isActive && (!isFirst || willHandlePopInternally); bool get canPop => hasActiveRouteBelow || willHandlePopInternally;
// Internals // Internals
......
...@@ -93,6 +93,36 @@ void main() { ...@@ -93,6 +93,36 @@ void main() {
expect(find.byType(BackdropFilter), findsOneWidget); expect(find.byType(BackdropFilter), findsOneWidget);
}); });
testWidgets('Nav bar displays correctly', (WidgetTester tester) async {
final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
await tester.pumpWidget(
CupertinoApp(
navigatorKey: navigator,
home: const CupertinoNavigationBar(
middle: Text('Page 1'),
),
),
);
navigator.currentState!.push<void>(CupertinoPageRoute<void>(
builder: (BuildContext context) {
return const CupertinoNavigationBar(
middle: Text('Page 2'),
);
}
));
await tester.pumpAndSettle();
expect(find.byType(CupertinoNavigationBarBackButton), findsOneWidget);
// Pops the page 2
navigator.currentState!.pop();
await tester.pump();
// Needs another pump to trigger the rebuild;
await tester.pump();
// The back button should still persist;
expect(find.byType(CupertinoNavigationBarBackButton), findsOneWidget);
// The app does not crash
expect(tester.takeException(), isNull);
});
testWidgets('Can specify custom padding', (WidgetTester tester) async { testWidgets('Can specify custom padding', (WidgetTester tester) async {
final Key middleBox = GlobalKey(); final Key middleBox = GlobalKey();
await tester.pumpWidget( await tester.pumpWidget(
......
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