From 1b310472c044a461dcfac823dca74f9b6359e4a7 Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Fri, 15 Jan 2021 12:29:04 -0800 Subject: [PATCH] Fix canpop logic to be more robust (#73993) --- .../flutter/lib/src/material/app_bar.dart | 10 ++----- .../flutter/lib/src/widgets/navigator.dart | 19 +++++++++--- packages/flutter/lib/src/widgets/routes.dart | 5 +++- .../flutter/test/cupertino/nav_bar_test.dart | 30 +++++++++++++++++++ 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/packages/flutter/lib/src/material/app_bar.dart b/packages/flutter/lib/src/material/app_bar.dart index e2dedcd87c9..b57b7a49604 100644 --- a/packages/flutter/lib/src/material/app_bar.dart +++ b/packages/flutter/lib/src/material/app_bar.dart @@ -709,8 +709,6 @@ class _AppBarState extends State { Scaffold.of(context).openEndDrawer(); } - bool? hadBackButtonWhenRouteWasActive; - @override Widget build(BuildContext context) { assert(!widget.primary || debugCheckHasMediaQuery(context)); @@ -723,11 +721,7 @@ class _AppBarState extends State { final bool hasDrawer = scaffold?.hasDrawer ?? false; final bool hasEndDrawer = scaffold?.hasEndDrawer ?? false; - hadBackButtonWhenRouteWasActive ??= false; - if (parentRoute?.isActive == true) { - hadBackButtonWhenRouteWasActive = parentRoute!.canPop; - } - assert(hadBackButtonWhenRouteWasActive != null); + final bool canPop = parentRoute?.canPop ?? false; final bool useCloseButton = parentRoute is PageRoute && parentRoute.fullscreenDialog; final double toolbarHeight = widget.toolbarHeight ?? kToolbarHeight; @@ -796,7 +790,7 @@ class _AppBarState extends State { tooltip: MaterialLocalizations.of(context).openAppDrawerTooltip, ); } else { - if (!hasEndDrawer && hadBackButtonWhenRouteWasActive!) + if (!hasEndDrawer && canPop) leading = useCloseButton ? const CloseButton() : const BackButton(); } } diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index deef670448b..89389b3285d 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -464,10 +464,7 @@ abstract class Route { return currentRouteEntry.route == this; } - /// Whether this route is the bottom-most route on the navigator. - /// - /// If this is true, then [Navigator.canPop] will return false if this route's - /// [willHandlePopInternally] returns false. + /// Whether this route is the bottom-most active route on the navigator. /// /// If [isFirst] and [isCurrent] are both true then this is the only route on /// the navigator (and [isActive] will also be true). @@ -483,6 +480,20 @@ abstract class Route { 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. /// /// If the route is not only active, but also the current route (the top-most diff --git a/packages/flutter/lib/src/widgets/routes.dart b/packages/flutter/lib/src/widgets/routes.dart index dc41b755ff3..cc77214a393 100644 --- a/packages/flutter/lib/src/widgets/routes.dart +++ b/packages/flutter/lib/src/widgets/routes.dart @@ -1483,10 +1483,13 @@ abstract class ModalRoute extends TransitionRoute with LocalHistoryRoute isActive && (!isFirst || willHandlePopInternally); + bool get canPop => hasActiveRouteBelow || willHandlePopInternally; // Internals diff --git a/packages/flutter/test/cupertino/nav_bar_test.dart b/packages/flutter/test/cupertino/nav_bar_test.dart index 0b69494e710..8c77a8ef9e9 100644 --- a/packages/flutter/test/cupertino/nav_bar_test.dart +++ b/packages/flutter/test/cupertino/nav_bar_test.dart @@ -93,6 +93,36 @@ void main() { expect(find.byType(BackdropFilter), findsOneWidget); }); + testWidgets('Nav bar displays correctly', (WidgetTester tester) async { + final GlobalKey navigator = GlobalKey(); + await tester.pumpWidget( + CupertinoApp( + navigatorKey: navigator, + home: const CupertinoNavigationBar( + middle: Text('Page 1'), + ), + ), + ); + navigator.currentState!.push(CupertinoPageRoute( + 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 { final Key middleBox = GlobalKey(); await tester.pumpWidget(