From 62828f43f7333a14ef29a2e8f180feaa764ae1a0 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 8 Aug 2024 13:35:11 -0700 Subject: [PATCH 1/4] [go_router] Fixes an issue where android back button pops wrong page. --- packages/go_router/CHANGELOG.md | 4 ++ packages/go_router/lib/src/delegate.dart | 39 +++++++------ packages/go_router/lib/src/router.dart | 2 +- packages/go_router/pubspec.yaml | 2 +- packages/go_router/test/go_router_test.dart | 61 +++++++++++++++++++++ 5 files changed, 88 insertions(+), 20 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 4f9be82467f..c0429243016 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 14.2.5 + +- Fixes an issue where android back button pops wrong page. + ## 14.2.4 - Updates minimum supported SDK version to Flutter 3.19/Dart 3.3. diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index eba3ef0c801..d2bfc55e0a6 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -52,22 +52,9 @@ class GoRouterDelegate extends RouterDelegate @override Future popRoute() async { - NavigatorState? state = navigatorKey.currentState; - if (state == null) { - return false; - } - if (!state.canPop()) { - state = null; - } - RouteMatchBase walker = currentConfiguration.matches.last; - while (walker is ShellRouteMatch) { - if (walker.navigatorKey.currentState?.canPop() ?? false) { - state = walker.navigatorKey.currentState; - } - walker = walker.matches.last; - } - assert(walker is RouteMatch); + final NavigatorState? state = _findCurrentNavigator(); if (state != null) { + // now we have to figure out whether we are the last return state.maybePop(); } // This should be the only place where the last GoRoute exit the screen. @@ -75,7 +62,8 @@ class GoRouterDelegate extends RouterDelegate if (lastRoute.onExit != null && navigatorKey.currentContext != null) { return !(await lastRoute.onExit!( navigatorKey.currentContext!, - walker.buildState(_configuration, currentConfiguration), + currentConfiguration.last + .buildState(_configuration, currentConfiguration), )); } return false; @@ -98,6 +86,14 @@ class GoRouterDelegate extends RouterDelegate /// Pops the top-most route. void pop([T? result]) { + final NavigatorState? state = _findCurrentNavigator(); + if (state == null) { + throw GoError('There is nothing to pop'); + } + state.pop(result); + } + + NavigatorState? _findCurrentNavigator() { NavigatorState? state; if (navigatorKey.currentState?.canPop() ?? false) { state = navigatorKey.currentState; @@ -110,9 +106,16 @@ class GoRouterDelegate extends RouterDelegate walker = walker.matches.last; } if (state == null) { - throw GoError('There is nothing to pop'); + return null; } - state.pop(result); + NavigatorState currentState = state; + bool isNavigatorCurrent = ModalRoute.isCurrentOf(state.context) ?? true; + while (!isNavigatorCurrent) { + currentState = + currentState.context.findAncestorStateOfType()!; + isNavigatorCurrent = ModalRoute.isCurrentOf(currentState.context) ?? true; + } + return currentState; } void _debugAssertMatchListNotEmpty() { diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart index dc6d88057eb..2bc2057a3bb 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -199,7 +199,7 @@ class GoRouter implements RouterConfig { setLogging(enabled: debugLogDiagnostics); WidgetsFlutterBinding.ensureInitialized(); - navigatorKey ??= GlobalKey(); + navigatorKey ??= GlobalKey(debugLabel: 'root'); _routingConfig.addListener(_handleRoutingConfigChanged); configuration = RouteConfiguration( diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index e5171092513..2ad6bd28e90 100644 --- a/packages/go_router/pubspec.yaml +++ b/packages/go_router/pubspec.yaml @@ -1,7 +1,7 @@ name: go_router description: A declarative router for Flutter based on Navigation 2 supporting deep linking, data-driven routes and more -version: 14.2.4 +version: 14.2.5 repository: https://github.com/flutter/packages/tree/main/packages/go_router issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22 diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index f395faf906f..1a0effb663a 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -398,6 +398,67 @@ void main() { expect(find.byKey(settings), findsOneWidget); }); + testWidgets('android back button pop in correct order', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/141906. + final List routes = [ + GoRoute( + path: '/', + builder: (_, __) => const Text('home'), + routes: [ + ShellRoute( + builder: ( + BuildContext context, + GoRouterState state, + Widget child, + ) { + return Column( + children: [ + const Text('shell'), + child, + ], + ); + }, + routes: [ + GoRoute( + path: 'page', + builder: (BuildContext context, __) { + return TextButton( + onPressed: () { + Navigator.of(context, rootNavigator: true).push( + MaterialPageRoute( + builder: (BuildContext context) { + return const Text('pageless'); + }), + ); + }, + child: const Text('page'), + ); + }, + ), + ], + ), + ]), + ]; + final GoRouter router = + await createRouter(routes, tester, initialLocation: '/page'); + expect(find.text('shell'), findsOneWidget); + expect(find.text('page'), findsOneWidget); + + await tester.tap(find.text('page')); + await tester.pumpAndSettle(); + expect(find.text('shell'), findsNothing); + expect(find.text('page'), findsNothing); + expect(find.text('pageless'), findsOneWidget); + + final bool result = await router.routerDelegate.popRoute(); + expect(result, isTrue); + await tester.pumpAndSettle(); + expect(find.text('shell'), findsOneWidget); + expect(find.text('page'), findsOneWidget); + expect(find.text('pageless'), findsNothing); + }); + testWidgets('can correctly pop stacks of repeated pages', (WidgetTester tester) async { // Regression test for https://github.com/flutter/flutter/issues/#132229. From fd4c3b9789ea667076c525c6e49556e51e01db40 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 8 Aug 2024 13:54:36 -0700 Subject: [PATCH 2/4] update --- packages/go_router/lib/src/delegate.dart | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index d2bfc55e0a6..15652aec597 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -54,7 +54,6 @@ class GoRouterDelegate extends RouterDelegate Future popRoute() async { final NavigatorState? state = _findCurrentNavigator(); if (state != null) { - // now we have to figure out whether we are the last return state.maybePop(); } // This should be the only place where the last GoRoute exit the screen. @@ -100,22 +99,18 @@ class GoRouterDelegate extends RouterDelegate } RouteMatchBase walker = currentConfiguration.matches.last; while (walker is ShellRouteMatch) { - if (walker.navigatorKey.currentState?.canPop() ?? false) { + final NavigatorState potentialCandidate = walker.navigatorKey.currentState!; + if (!ModalRoute.isCurrentOf(potentialCandidate.context)!) { + // There is a pageless route on top of the shell route. it needs to be + // popped first. + break; + } + if (potentialCandidate.canPop()) { state = walker.navigatorKey.currentState; } walker = walker.matches.last; } - if (state == null) { - return null; - } - NavigatorState currentState = state; - bool isNavigatorCurrent = ModalRoute.isCurrentOf(state.context) ?? true; - while (!isNavigatorCurrent) { - currentState = - currentState.context.findAncestorStateOfType()!; - isNavigatorCurrent = ModalRoute.isCurrentOf(currentState.context) ?? true; - } - return currentState; + return state; } void _debugAssertMatchListNotEmpty() { From f03bfdb65775bfb0649cd7d0cfe2b0a2cfe263e3 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 8 Aug 2024 13:55:40 -0700 Subject: [PATCH 3/4] update --- packages/go_router/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index c0429243016..9374421558e 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,6 +1,6 @@ ## 14.2.5 -- Fixes an issue where android back button pops wrong page. +- Fixes an issue where android back button pops pages in the wrong order. ## 14.2.4 From ccd285c9debc47740b0ecb416c0b96d0a498eefd Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 8 Aug 2024 14:02:11 -0700 Subject: [PATCH 4/4] update --- packages/go_router/lib/src/delegate.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index 15652aec597..06e4f08184f 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -99,8 +99,9 @@ class GoRouterDelegate extends RouterDelegate } RouteMatchBase walker = currentConfiguration.matches.last; while (walker is ShellRouteMatch) { - final NavigatorState potentialCandidate = walker.navigatorKey.currentState!; - if (!ModalRoute.isCurrentOf(potentialCandidate.context)!) { + final NavigatorState potentialCandidate = + walker.navigatorKey.currentState!; + if (!ModalRoute.of(potentialCandidate.context)!.isCurrent) { // There is a pageless route on top of the shell route. it needs to be // popped first. break;