From 2df917a47309a97bb5261089229a3648e8112faa Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Mon, 12 Jun 2023 11:21:19 -0700 Subject: [PATCH 1/5] [go_router] Adds parent navigator key to ShellRoute and StatefulShellRoute. --- packages/go_router/CHANGELOG.md | 4 + packages/go_router/lib/src/builder.dart | 130 ++++++++++--------- packages/go_router/lib/src/route.dart | 29 +++-- packages/go_router/pubspec.yaml | 2 +- packages/go_router/test/go_route_test.dart | 138 +++++++++++++++++++++ 5 files changed, 224 insertions(+), 79 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index b0df45c5fe6..b530a9a0a69 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 8.1.0 + +- Adds parent navigator key to ShellRoute and StatefulShellRoute. + ## 8.0.5 - Fixes a bug that GoRouterState in top level redirect doesn't contain complete data. diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart index c98f2f8bd12..26d70ae9212 100644 --- a/packages/go_router/lib/src/builder.dart +++ b/packages/go_router/lib/src/builder.dart @@ -204,77 +204,73 @@ class RouteBuilder { keyToPages.putIfAbsent(navigatorKey, () => >[]).add(page); _buildRecursive(context, matchList, startIndex + 1, pagePopContext, routerNeglect, keyToPages, navigatorKey, registry); - } else if (route is GoRoute) { - page = _buildPageForGoRoute(context, state, match, route, pagePopContext); - // If this GoRoute is for a different Navigator, add it to the + } else { + // If this RouteBase is for a different Navigator, add it to the // list of out of scope pages - final GlobalKey goRouteNavKey = + final GlobalKey routeNavKey = route.parentNavigatorKey ?? navigatorKey; - - keyToPages.putIfAbsent(goRouteNavKey, () => >[]).add(page); - - _buildRecursive(context, matchList, startIndex + 1, pagePopContext, - routerNeglect, keyToPages, navigatorKey, registry); - } else if (route is ShellRouteBase) { - assert(startIndex + 1 < matchList.matches.length, - 'Shell routes must always have child routes'); - // The key for the Navigator that will display this ShellRoute's page. - final GlobalKey parentNavigatorKey = navigatorKey; - - // Add an entry for the parent navigator if none exists. - keyToPages.putIfAbsent(parentNavigatorKey, () => >[]); - - // Calling _buildRecursive can result in adding pages to the - // parentNavigatorKey entry's list. Store the current length so - // that the page for this ShellRoute is placed at the right index. - final int shellPageIdx = keyToPages[parentNavigatorKey]!.length; - - // Get the current sub-route of this shell route from the match list. - final RouteBase subRoute = matchList.matches[startIndex + 1].route; - - // The key to provide to the shell route's Navigator. - final GlobalKey shellNavigatorKey = - route.navigatorKeyForSubRoute(subRoute); - - // Add an entry for the shell route's navigator - keyToPages.putIfAbsent(shellNavigatorKey, () => >[]); - - // Build the remaining pages - _buildRecursive(context, matchList, startIndex + 1, pagePopContext, - routerNeglect, keyToPages, shellNavigatorKey, registry); - - final HeroController heroController = _goHeroCache.putIfAbsent( - shellNavigatorKey, () => _getHeroController(context)); - - // Build the Navigator for this shell route - Widget buildShellNavigator( - List? observers, String? restorationScopeId) { - return _buildNavigator( - pagePopContext.onPopPage, - keyToPages[shellNavigatorKey]!, - shellNavigatorKey, - observers: observers ?? const [], - restorationScopeId: restorationScopeId, - heroController: heroController, + if (route is GoRoute) { + page = + _buildPageForGoRoute(context, state, match, route, pagePopContext); + + keyToPages.putIfAbsent(routeNavKey, () => >[]).add(page); + + _buildRecursive(context, matchList, startIndex + 1, pagePopContext, + routerNeglect, keyToPages, navigatorKey, registry); + } else if (route is ShellRouteBase) { + assert(startIndex + 1 < matchList.matches.length, + 'Shell routes must always have child routes'); + + // Add an entry for the parent navigator if none exists. + // + // Calling _buildRecursive can result in adding pages to the + // parentNavigatorKey entry's list. Store the current length so + // that the page for this ShellRoute is placed at the right index. + final int shellPageIdx = + keyToPages.putIfAbsent(routeNavKey, () => >[]).length; + + // Find the the navigator key for the sub-route of this shell route. + final RouteBase subRoute = matchList.matches[startIndex + 1].route; + final GlobalKey shellNavigatorKey = + route.navigatorKeyForSubRoute(subRoute); + + keyToPages.putIfAbsent(shellNavigatorKey, () => >[]); + + // Build the remaining pages + _buildRecursive(context, matchList, startIndex + 1, pagePopContext, + routerNeglect, keyToPages, shellNavigatorKey, registry); + + final HeroController heroController = _goHeroCache.putIfAbsent( + shellNavigatorKey, () => _getHeroController(context)); + + // Build the Navigator for this shell route + Widget buildShellNavigator( + List? observers, String? restorationScopeId) { + return _buildNavigator( + pagePopContext.onPopPage, + keyToPages[shellNavigatorKey]!, + shellNavigatorKey, + observers: observers ?? const [], + restorationScopeId: restorationScopeId, + heroController: heroController, + ); + } + + // Call the ShellRouteBase to create/update the shell route state + final ShellRouteContext shellRouteContext = ShellRouteContext( + route: route, + routerState: state, + navigatorKey: shellNavigatorKey, + routeMatchList: matchList, + navigatorBuilder: buildShellNavigator, ); - } - - // Call the ShellRouteBase to create/update the shell route state - final ShellRouteContext shellRouteContext = ShellRouteContext( - route: route, - routerState: state, - navigatorKey: shellNavigatorKey, - routeMatchList: matchList, - navigatorBuilder: buildShellNavigator, - ); - // Build the Page for this route - page = _buildPageForShellRoute( - context, state, match, route, pagePopContext, shellRouteContext); - // Place the ShellRoute's Page onto the list for the parent navigator. - keyToPages - .putIfAbsent(parentNavigatorKey, () => >[]) - .insert(shellPageIdx, page); + // Build the Page for this route + page = _buildPageForShellRoute( + context, state, match, route, pagePopContext, shellRouteContext); + // Place the ShellRoute's Page onto the list for the parent navigator. + keyToPages[routeNavKey]!.insert(shellPageIdx, page); + } } if (page != null) { registry[page] = state; diff --git a/packages/go_router/lib/src/route.dart b/packages/go_router/lib/src/route.dart index ba656326e32..9d33e886621 100644 --- a/packages/go_router/lib/src/route.dart +++ b/packages/go_router/lib/src/route.dart @@ -98,12 +98,20 @@ import 'typedefs.dart'; @immutable abstract class RouteBase { const RouteBase._({ - this.routes = const [], + required this.routes, + required this.parentNavigatorKey, }); /// The list of child routes associated with this route. final List routes; + /// An optional key specifying which Navigator to display this route's screen + /// onto. + /// + /// Specifying the root Navigator will stack this route onto that + /// Navigator instead of the nearest ShellRoute ancestor. + final GlobalKey? parentNavigatorKey; + /// Builds a lists containing the provided routes along with all their /// descendant [routes]. static Iterable routesRecursively(Iterable routes) { @@ -137,7 +145,7 @@ class GoRoute extends RouteBase { this.name, this.builder, this.pageBuilder, - this.parentNavigatorKey, + super.parentNavigatorKey, this.redirect, super.routes = const [], }) : assert(path.isNotEmpty, 'GoRoute path cannot be empty'), @@ -301,13 +309,6 @@ class GoRoute extends RouteBase { /// re-evaluation will be triggered if the [InheritedWidget] changes. final GoRouterRedirect? redirect; - /// An optional key specifying which Navigator to display this route's screen - /// onto. - /// - /// Specifying the root Navigator will stack this route onto that - /// Navigator instead of the nearest ShellRoute ancestor. - final GlobalKey? parentNavigatorKey; - // TODO(chunhtai): move all regex related help methods to path_utils.dart. /// Match this route against a location. RegExpMatch? matchPatternAsPrefix(String loc) => @@ -333,7 +334,9 @@ class GoRoute extends RouteBase { /// as [ShellRoute] and [StatefulShellRoute]. abstract class ShellRouteBase extends RouteBase { /// Constructs a [ShellRouteBase]. - const ShellRouteBase._({super.routes}) : super._(); + const ShellRouteBase._( + {required super.routes, required super.parentNavigatorKey}) + : super._(); /// Attempts to build the Widget representing this shell route. /// @@ -496,7 +499,8 @@ class ShellRoute extends ShellRouteBase { this.builder, this.pageBuilder, this.observers, - super.routes, + required super.routes, + super.parentNavigatorKey, GlobalKey? navigatorKey, this.restorationScopeId, }) : assert(routes.isNotEmpty), @@ -653,6 +657,7 @@ class StatefulShellRoute extends ShellRouteBase { this.builder, this.pageBuilder, required this.navigatorContainerBuilder, + super.parentNavigatorKey, this.restorationScopeId, }) : assert(branches.isNotEmpty), assert((pageBuilder != null) ^ (builder != null), @@ -676,12 +681,14 @@ class StatefulShellRoute extends ShellRouteBase { StatefulShellRoute.indexedStack({ required List branches, StatefulShellRouteBuilder? builder, + GlobalKey? parentNavigatorKey, StatefulShellRoutePageBuilder? pageBuilder, String? restorationScopeId, }) : this( branches: branches, builder: builder, pageBuilder: pageBuilder, + parentNavigatorKey: parentNavigatorKey, restorationScopeId: restorationScopeId, navigatorContainerBuilder: _indexedStackContainerBuilder, ); diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index db489edff28..ded370a7c29 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: 8.0.5 +version: 8.1.0 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_route_test.dart b/packages/go_router/test/go_route_test.dart index 31361aa6538..f7ffe7001bc 100644 --- a/packages/go_router/test/go_route_test.dart +++ b/packages/go_router/test/go_route_test.dart @@ -2,9 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; +import 'test_helpers.dart'; + void main() { test('throws when a builder is not set', () { expect(() => GoRoute(path: '/'), throwsA(isAssertionError)); @@ -17,4 +20,139 @@ void main() { test('does not throw when only redirect is provided', () { GoRoute(path: '/', redirect: (_, __) => '/a'); }); + + testWidgets('ShellRoute can use parent navigator key', + (WidgetTester tester) async { + final GlobalKey rootNavigatorKey = + GlobalKey(); + final GlobalKey shellNavigatorKey = + GlobalKey(); + + final List routes = [ + ShellRoute( + navigatorKey: shellNavigatorKey, + builder: (BuildContext context, GoRouterState state, Widget child) { + return Scaffold( + body: Column( + children: [ + const Text('Screen A'), + Expanded(child: child), + ], + ), + ); + }, + routes: [ + GoRoute( + path: '/b', + builder: (BuildContext context, GoRouterState state) { + return const Scaffold( + body: Text('Screen B'), + ); + }, + routes: [ + ShellRoute( + parentNavigatorKey: rootNavigatorKey, + builder: + (BuildContext context, GoRouterState state, Widget child) { + return Scaffold( + body: Column( + children: [ + const Text('Screen D'), + Expanded(child: child), + ], + ), + ); + }, + routes: [ + GoRoute( + path: 'c', + builder: (BuildContext context, GoRouterState state) { + return const Scaffold( + body: Text('Screen C'), + ); + }, + ), + ], + ), + ], + ), + ], + ), + ]; + + await createRouter(routes, tester, + initialLocation: '/b/c', navigatorKey: rootNavigatorKey); + expect(find.text('Screen A'), findsNothing); + expect(find.text('Screen B'), findsNothing); + expect(find.text('Screen D'), findsOneWidget); + expect(find.text('Screen C'), findsOneWidget); + }); + + testWidgets('StatefulShellRoute can use parent navigator key', + (WidgetTester tester) async { + final GlobalKey rootNavigatorKey = + GlobalKey(); + final GlobalKey shellNavigatorKey = + GlobalKey(); + + final List routes = [ + ShellRoute( + navigatorKey: shellNavigatorKey, + builder: (BuildContext context, GoRouterState state, Widget child) { + return Scaffold( + body: Column( + children: [ + const Text('Screen A'), + Expanded(child: child), + ], + ), + ); + }, + routes: [ + GoRoute( + path: '/b', + builder: (BuildContext context, GoRouterState state) { + return const Scaffold( + body: Text('Screen B'), + ); + }, + routes: [ + StatefulShellRoute.indexedStack( + parentNavigatorKey: rootNavigatorKey, + builder: (_, __, StatefulNavigationShell navigationShell) { + return Column( + children: [ + const Text('Screen D'), + Expanded(child: navigationShell), + ], + ); + }, + branches: [ + StatefulShellBranch( + routes: [ + GoRoute( + path: 'c', + builder: (BuildContext context, GoRouterState state) { + return const Scaffold( + body: Text('Screen C'), + ); + }, + ), + ], + ), + ], + ), + ], + ), + ], + ), + ]; + + await createRouter(routes, tester, + initialLocation: '/b/c', navigatorKey: rootNavigatorKey); + expect(find.text('Screen A'), findsNothing); + expect(find.text('Screen B'), findsNothing); + expect(find.text('Screen D'), findsOneWidget); + expect(find.text('Screen C'), findsOneWidget); + }); } From f7f0b45f98d40b8a8cec0d63836b4cd84c6889de Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 16 Jun 2023 10:07:54 -0700 Subject: [PATCH 2/5] update --- packages/go_router/lib/src/delegate.dart | 85 ++++++++++------------ packages/go_router/test/delegate_test.dart | 38 ++++++++++ 2 files changed, 76 insertions(+), 47 deletions(-) diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index 2ba3729ef95..3651185aa11 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -148,73 +148,64 @@ class GoRouterDelegate extends RouterDelegate /// pageless route, such as a dialog or bottom sheet. class _NavigatorStateIterator extends Iterator { _NavigatorStateIterator(this.matchList, this.root) - : index = matchList.matches.length; + : index = matchList.matches.length - 1; final RouteMatchList matchList; - int index = 0; + int index; + final NavigatorState root; @override late NavigatorState current; + RouteBase _getRouteAtIndex(int index) => matchList.matches[index].route; + + /// Update [index] to point to the ShellRouteBase that uses the navigator key. + /// + /// The [index] will be -1 if navigator key points to root. + void _findsNextIndex() { + final GlobalKey? parentNavigatorKey = + _getRouteAtIndex(index).parentNavigatorKey; + if (parentNavigatorKey == null) { + index -= 1; + return; + } + + for (index -= 1; index >= 0; index -= 1) { + final RouteBase route = _getRouteAtIndex(index); + if (route is ShellRouteBase) { + if (route.navigatorKeyForSubRoute(_getRouteAtIndex(index + 1)) == + parentNavigatorKey) { + return; + } + } + } + assert(root == parentNavigatorKey.currentState); + } + @override bool moveNext() { if (index < 0) { return false; } - late RouteBase subRoute; - for (index -= 1; index >= 0; index -= 1) { - final RouteMatch match = matchList.matches[index]; - final RouteBase route = match.route; - if (route is GoRoute && route.parentNavigatorKey != null) { - final GlobalKey parentNavigatorKey = - route.parentNavigatorKey!; - final ModalRoute? parentModalRoute = - ModalRoute.of(parentNavigatorKey.currentContext!); - // The ModalRoute can be null if the parentNavigatorKey references the - // root navigator. - if (parentModalRoute == null) { - index = -1; - assert(root == parentNavigatorKey.currentState); - current = root; - return true; - } - // It must be a ShellRoute that holds this parentNavigatorKey; - // otherwise, parentModalRoute would have been null. Updates the index - // to the ShellRoute - for (index -= 1; index >= 0; index -= 1) { - final RouteBase route = matchList.matches[index].route; - if (route is ShellRoute) { - if (route.navigatorKey == parentNavigatorKey) { - break; - } - } - } - // There may be a pageless route on top of ModalRoute that the - // NavigatorState of parentNavigatorKey is in. For example, an open - // dialog. In that case we want to find the navigator that host the - // pageless route. - if (parentModalRoute.isCurrent == false) { - continue; - } + _findsNextIndex(); - current = parentNavigatorKey.currentState!; - return true; - } else if (route is ShellRouteBase) { + while (index >= 0) { + final RouteBase route = _getRouteAtIndex(index); + if (route is ShellRouteBase) { + final GlobalKey navigatorKey = + route.navigatorKeyForSubRoute(_getRouteAtIndex(index + 1)); // Must have a ModalRoute parent because the navigator ShellRoute // created must not be the root navigator. - final GlobalKey navigatorKey = - route.navigatorKeyForSubRoute(subRoute); final ModalRoute parentModalRoute = ModalRoute.of(navigatorKey.currentContext!)!; // There may be pageless route on top of ModalRoute that the // parentNavigatorKey is in. For example an open dialog. - if (parentModalRoute.isCurrent == false) { - continue; + if (parentModalRoute.isCurrent) { + current = navigatorKey.currentState!; + return true; } - current = navigatorKey.currentState!; - return true; } - subRoute = route; + _findsNextIndex(); } assert(index == -1); current = root; diff --git a/packages/go_router/test/delegate_test.dart b/packages/go_router/test/delegate_test.dart index 859a4f2d70d..be335bab56c 100644 --- a/packages/go_router/test/delegate_test.dart +++ b/packages/go_router/test/delegate_test.dart @@ -7,6 +7,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; import 'package:go_router/src/match.dart'; import 'package:go_router/src/misc/error_screen.dart'; +import 'package:go_router/src/misc/errors.dart'; import 'test_helpers.dart'; @@ -96,6 +97,43 @@ void main() { await goRouter.routerDelegate.popRoute(); expect(await goRouter.routerDelegate.popRoute(), isFalse); }); + + testWidgets('throw if nothing to pop', (WidgetTester tester) async { + final GlobalKey rootKey = GlobalKey(); + final GlobalKey navKey = GlobalKey(); + final GoRouter goRouter = await createRouter( + [ + ShellRoute( + navigatorKey: rootKey, + builder: (_, __, Widget child) => child, + routes: [ + ShellRoute( + parentNavigatorKey: rootKey, + navigatorKey: navKey, + builder: (_, __, Widget child) => child, + routes: [ + GoRoute( + path: '/', + parentNavigatorKey: navKey, + builder: (_, __) => const Text('Home'), + ), + ], + ), + ], + ), + ], + tester, + ); + await tester.pumpAndSettle(); + expect(find.text('Home'), findsOneWidget); + String? message; + try { + goRouter.pop(); + } on GoError catch (e) { + message = e.message; + } + expect(message, 'There is nothing to pop'); + }); }); group('push', () { From cad596e8e19d330b37d2fb204f0541d4db465bcf Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 16 Jun 2023 10:12:30 -0700 Subject: [PATCH 3/5] update --- packages/go_router/lib/src/delegate.dart | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index 3651185aa11..37986714a3c 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -159,9 +159,6 @@ class _NavigatorStateIterator extends Iterator { RouteBase _getRouteAtIndex(int index) => matchList.matches[index].route; - /// Update [index] to point to the ShellRouteBase that uses the navigator key. - /// - /// The [index] will be -1 if navigator key points to root. void _findsNextIndex() { final GlobalKey? parentNavigatorKey = _getRouteAtIndex(index).parentNavigatorKey; From 5db2ec46f657e981e0c672003a702f25dc2f382e Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 16 Jun 2023 10:42:16 -0700 Subject: [PATCH 4/5] update --- packages/go_router/test/delegate_test.dart | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/go_router/test/delegate_test.dart b/packages/go_router/test/delegate_test.dart index be335bab56c..87c45e1b1b0 100644 --- a/packages/go_router/test/delegate_test.dart +++ b/packages/go_router/test/delegate_test.dart @@ -134,6 +134,35 @@ void main() { } expect(message, 'There is nothing to pop'); }); + + testWidgets('poproute return false if nothing to pop', (WidgetTester tester) async { + final GlobalKey rootKey = GlobalKey(); + final GlobalKey navKey = GlobalKey(); + final GoRouter goRouter = await createRouter( + [ + ShellRoute( + navigatorKey: rootKey, + builder: (_, __, Widget child) => child, + routes: [ + ShellRoute( + parentNavigatorKey: rootKey, + navigatorKey: navKey, + builder: (_, __, Widget child) => child, + routes: [ + GoRoute( + path: '/', + parentNavigatorKey: navKey, + builder: (_, __) => const Text('Home'), + ), + ], + ), + ], + ), + ], + tester, + ); + expect(await goRouter.routerDelegate.popRoute(), isFalse); + }); }); group('push', () { From 3bc625f758ffed9315439396e12a3ee4abdaca0c Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 16 Jun 2023 11:19:20 -0700 Subject: [PATCH 5/5] update --- packages/go_router/test/delegate_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/go_router/test/delegate_test.dart b/packages/go_router/test/delegate_test.dart index 87c45e1b1b0..1b1d5e575ff 100644 --- a/packages/go_router/test/delegate_test.dart +++ b/packages/go_router/test/delegate_test.dart @@ -135,7 +135,8 @@ void main() { expect(message, 'There is nothing to pop'); }); - testWidgets('poproute return false if nothing to pop', (WidgetTester tester) async { + testWidgets('poproute return false if nothing to pop', + (WidgetTester tester) async { final GlobalKey rootKey = GlobalKey(); final GlobalKey navKey = GlobalKey(); final GoRouter goRouter = await createRouter(