From b321f2c0ac660e8f9632714ec67e8e7e0ea82e30 Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Wed, 21 Jun 2023 15:10:00 -0700 Subject: [PATCH] =?UTF-8?q?[go=5Frouter]=20Adds=20parent=20navigator=20key?= =?UTF-8?q?=20to=20ShellRoute=20and=20StatefulShell=E2=80=A6=20(#4201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …Route. fixes https://github.com/flutter/flutter/issues/111678 fixes https://github.com/flutter/flutter/issues/128793 --- packages/go_router/CHANGELOG.md | 4 + packages/go_router/lib/src/builder.dart | 112 ++++++++--------- packages/go_router/lib/src/delegate.dart | 82 ++++++------ packages/go_router/lib/src/route.dart | 29 +++-- packages/go_router/pubspec.yaml | 2 +- packages/go_router/test/delegate_test.dart | 68 ++++++++++ packages/go_router/test/go_route_test.dart | 138 +++++++++++++++++++++ 7 files changed, 318 insertions(+), 117 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index b0df45c5fe..b530a9a0a6 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 c98f2f8bd1..26d70ae921 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; + if (route is GoRoute) { + page = + _buildPageForGoRoute(context, state, match, route, pagePopContext); - keyToPages.putIfAbsent(goRouteNavKey, () => >[]).add(page); + 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'); - // The key for the Navigator that will display this ShellRoute's page. - final GlobalKey parentNavigatorKey = navigatorKey; + _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. - keyToPages.putIfAbsent(parentNavigatorKey, () => >[]); + // 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; - // 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; + // 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); - // Get the current sub-route of this shell route from the match list. - final RouteBase subRoute = matchList.matches[startIndex + 1].route; + keyToPages.putIfAbsent(shellNavigatorKey, () => >[]); - // The key to provide to the shell route's Navigator. - final GlobalKey shellNavigatorKey = - route.navigatorKeyForSubRoute(subRoute); + // Build the remaining pages + _buildRecursive(context, matchList, startIndex + 1, pagePopContext, + routerNeglect, keyToPages, shellNavigatorKey, registry); - // Add an entry for the shell route's navigator - keyToPages.putIfAbsent(shellNavigatorKey, () => >[]); + final HeroController heroController = _goHeroCache.putIfAbsent( + shellNavigatorKey, () => _getHeroController(context)); - // Build the remaining pages - _buildRecursive(context, matchList, startIndex + 1, pagePopContext, - routerNeglect, keyToPages, shellNavigatorKey, registry); + // 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, + ); + } - 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, ); + + // 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); } - - // 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); } if (page != null) { registry[page] = state; diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index 2ba3729ef9..37986714a3 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -148,73 +148,61 @@ 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; + + 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/lib/src/route.dart b/packages/go_router/lib/src/route.dart index ba656326e3..9d33e88662 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 db489edff2..ded370a7c2 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/delegate_test.dart b/packages/go_router/test/delegate_test.dart index 859a4f2d70..1b1d5e575f 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,73 @@ 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'); + }); + + 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', () { diff --git a/packages/go_router/test/go_route_test.dart b/packages/go_router/test/go_route_test.dart index 31361aa653..f7ffe7001b 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); + }); }