[go_router] Refactors GoRouter.pop to handle pageless route (#2879)
* Refactors GoRouter.pop to handle pageless route
* update
diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md
index 333e564..ff7402e 100644
--- a/packages/go_router/CHANGELOG.md
+++ b/packages/go_router/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 5.2.1
+
+- Refactors `GoRouter.pop` to be able to pop individual pageless route with result.
+
## 5.2.0
- Fixes `GoRouterState.location` and `GoRouterState.param` to return correct value.
diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart
index 42989d7..04c6656 100644
--- a/packages/go_router/lib/src/builder.dart
+++ b/packages/go_router/lib/src/builder.dart
@@ -54,7 +54,7 @@
Widget build(
BuildContext context,
RouteMatchList matchList,
- VoidCallback pop,
+ PopPageCallback onPopPage,
bool routerNeglect,
) {
if (matchList.isEmpty) {
@@ -69,14 +69,14 @@
try {
final Map<Page<Object?>, GoRouterState> newRegistry =
<Page<Object?>, GoRouterState>{};
- final Widget result = tryBuild(context, matchList, pop,
+ final Widget result = tryBuild(context, matchList, onPopPage,
routerNeglect, configuration.navigatorKey, newRegistry);
_registry.updateRegistry(newRegistry);
return GoRouterStateRegistryScope(
registry: _registry, child: result);
} on _RouteBuilderError catch (e) {
- return _buildErrorNavigator(
- context, e, matchList.uri, pop, configuration.navigatorKey);
+ return _buildErrorNavigator(context, e, matchList.uri, onPopPage,
+ configuration.navigatorKey);
}
},
),
@@ -91,7 +91,7 @@
Widget tryBuild(
BuildContext context,
RouteMatchList matchList,
- VoidCallback pop,
+ PopPageCallback onPopPage,
bool routerNeglect,
GlobalKey<NavigatorState> navigatorKey,
Map<Page<Object?>, GoRouterState> registry,
@@ -99,9 +99,9 @@
return builderWithNav(
context,
_buildNavigator(
- pop,
- buildPages(
- context, matchList, pop, routerNeglect, navigatorKey, registry),
+ onPopPage,
+ buildPages(context, matchList, onPopPage, routerNeglect, navigatorKey,
+ registry),
navigatorKey,
observers: observers,
),
@@ -114,15 +114,15 @@
List<Page<Object?>> buildPages(
BuildContext context,
RouteMatchList matchList,
- VoidCallback onPop,
+ PopPageCallback onPopPage,
bool routerNeglect,
GlobalKey<NavigatorState> navigatorKey,
Map<Page<Object?>, GoRouterState> registry) {
try {
final Map<GlobalKey<NavigatorState>, List<Page<Object?>>> keyToPage =
<GlobalKey<NavigatorState>, List<Page<Object?>>>{};
- _buildRecursive(context, matchList, 0, onPop, routerNeglect, keyToPage,
- navigatorKey, registry);
+ _buildRecursive(context, matchList, 0, onPopPage, routerNeglect,
+ keyToPage, navigatorKey, registry);
return keyToPage[navigatorKey]!;
} on _RouteBuilderError catch (e) {
return <Page<Object?>>[
@@ -135,7 +135,7 @@
BuildContext context,
RouteMatchList matchList,
int startIndex,
- VoidCallback pop,
+ PopPageCallback onPopPage,
bool routerNeglect,
Map<GlobalKey<NavigatorState>, List<Page<Object?>>> keyToPages,
GlobalKey<NavigatorState> navigatorKey,
@@ -163,8 +163,8 @@
keyToPages.putIfAbsent(goRouteNavKey, () => <Page<Object?>>[]).add(page);
- _buildRecursive(context, matchList, startIndex + 1, pop, routerNeglect,
- keyToPages, navigatorKey, registry);
+ _buildRecursive(context, matchList, startIndex + 1, onPopPage,
+ routerNeglect, keyToPages, navigatorKey, registry);
} else if (route is ShellRoute) {
// The key for the Navigator that will display this ShellRoute's page.
final GlobalKey<NavigatorState> parentNavigatorKey = navigatorKey;
@@ -184,12 +184,12 @@
final int shellPageIdx = keyToPages[parentNavigatorKey]!.length;
// Build the remaining pages
- _buildRecursive(context, matchList, startIndex + 1, pop, routerNeglect,
- keyToPages, shellNavigatorKey, registry);
+ _buildRecursive(context, matchList, startIndex + 1, onPopPage,
+ routerNeglect, keyToPages, shellNavigatorKey, registry);
// Build the Navigator
final Widget child = _buildNavigator(
- pop, keyToPages[shellNavigatorKey]!, shellNavigatorKey);
+ onPopPage, keyToPages[shellNavigatorKey]!, shellNavigatorKey);
// Build the Page for this route
final Page<Object?> page =
@@ -203,7 +203,7 @@
}
Navigator _buildNavigator(
- VoidCallback pop,
+ PopPageCallback onPopPage,
List<Page<Object?>> pages,
Key? navigatorKey, {
List<NavigatorObserver> observers = const <NavigatorObserver>[],
@@ -213,13 +213,7 @@
restorationScopeId: restorationScopeId,
pages: pages,
observers: observers,
- onPopPage: (Route<dynamic> route, dynamic result) {
- if (!route.didPop(result)) {
- return false;
- }
- pop();
- return true;
- },
+ onPopPage: onPopPage,
);
}
@@ -393,10 +387,14 @@
);
/// Builds a Navigator containing an error page.
- Widget _buildErrorNavigator(BuildContext context, _RouteBuilderError e,
- Uri uri, VoidCallback pop, GlobalKey<NavigatorState> navigatorKey) {
+ Widget _buildErrorNavigator(
+ BuildContext context,
+ _RouteBuilderError e,
+ Uri uri,
+ PopPageCallback onPopPage,
+ GlobalKey<NavigatorState> navigatorKey) {
return _buildNavigator(
- pop,
+ onPopPage,
<Page<Object?>>[
_buildErrorPage(context, e, uri),
],
diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart
index 2ccbd4c..3bb303c 100644
--- a/packages/go_router/lib/src/delegate.dart
+++ b/packages/go_router/lib/src/delegate.dart
@@ -11,6 +11,7 @@
import 'configuration.dart';
import 'match.dart';
import 'matching.dart';
+import 'misc/errors.dart';
import 'typedefs.dart';
/// GoRouter implementation of [RouterDelegate].
@@ -59,38 +60,19 @@
final Map<String, int> _pushCounts = <String, int>{};
final RouteConfiguration _configuration;
+ _NavigatorStateIterator _createNavigatorStateIterator() =>
+ _NavigatorStateIterator(_matchList, navigatorKey.currentState!);
+
@override
Future<bool> popRoute() async {
- // Iterate backwards through the RouteMatchList until seeing a GoRoute with
- // a non-null parentNavigatorKey or a ShellRoute with a non-null
- // parentNavigatorKey and pop from that Navigator instead of the root.
- final int matchCount = _matchList.matches.length;
- for (int i = matchCount - 1; i >= 0; i -= 1) {
- final RouteMatch match = _matchList.matches[i];
- final RouteBase route = match.route;
-
- if (route is GoRoute && route.parentNavigatorKey != null) {
- final bool didPop =
- await route.parentNavigatorKey!.currentState!.maybePop();
-
- // Continue if didPop was false.
- if (didPop) {
- return didPop;
- }
- } else if (route is ShellRoute) {
- final bool didPop = await route.navigatorKey.currentState!.maybePop();
-
- // Continue if didPop was false.
- if (didPop) {
- return didPop;
- }
+ final _NavigatorStateIterator iterator = _createNavigatorStateIterator();
+ while (iterator.moveNext()) {
+ final bool didPop = await iterator.current.maybePop();
+ if (didPop) {
+ return true;
}
}
-
- // Use the root navigator if no ShellRoute Navigators were found and didn't
- // pop
- final NavigatorState navigator = navigatorKey.currentState!;
- return navigator.maybePop();
+ return false;
}
/// Pushes the given location onto the page stack
@@ -117,29 +99,25 @@
/// Returns `true` if the active Navigator can pop.
bool canPop() {
- // Loop through navigators in reverse and call canPop()
- final int matchCount = _matchList.matches.length;
- for (int i = matchCount - 1; i >= 0; i -= 1) {
- final RouteMatch match = _matchList.matches[i];
- final RouteBase route = match.route;
- if (route is GoRoute && route.parentNavigatorKey != null) {
- final bool canPop =
- route.parentNavigatorKey!.currentState?.canPop() ?? false;
-
- // Continue if canPop is false.
- if (canPop) {
- return canPop;
- }
- } else if (route is ShellRoute) {
- final bool canPop = route.navigatorKey.currentState?.canPop() ?? false;
-
- // Continue if canPop is false.
- if (canPop) {
- return canPop;
- }
+ final _NavigatorStateIterator iterator = _createNavigatorStateIterator();
+ while (iterator.moveNext()) {
+ if (iterator.current.canPop()) {
+ return true;
}
}
- return navigatorKey.currentState?.canPop() ?? false;
+ return false;
+ }
+
+ /// Pops the top-most route.
+ void pop<T extends Object?>([T? result]) {
+ final _NavigatorStateIterator iterator = _createNavigatorStateIterator();
+ while (iterator.moveNext()) {
+ if (iterator.current.canPop()) {
+ iterator.current.pop<T>(result);
+ return;
+ }
+ }
+ throw GoError('There is nothing to pop');
}
void _debugAssertMatchListNotEmpty() {
@@ -150,14 +128,16 @@
);
}
- /// Pop the top page off the GoRouter's page stack.
- void pop() {
+ bool _onPopPage(Route<Object?> route, Object? result) {
+ if (!route.didPop(result)) {
+ return false;
+ }
_matchList.pop();
assert(() {
_debugAssertMatchListNotEmpty();
return true;
}());
- notifyListeners();
+ return true;
}
/// Replaces the top-most page of the page stack with the given one.
@@ -187,7 +167,7 @@
return builder.build(
context,
_matchList,
- pop,
+ _onPopPage,
routerNeglect,
);
}
@@ -204,6 +184,84 @@
}
}
+/// An iterator that iterates through navigators that [GoRouterDelegate]
+/// created from the inner to outer.
+///
+/// The iterator starts with the navigator that hosts the top-most route. This
+/// navigator may not be the inner-most navigator if the top-most route is a
+/// pageless route, such as a dialog or bottom sheet.
+class _NavigatorStateIterator extends Iterator<NavigatorState> {
+ _NavigatorStateIterator(this.matchList, this.root)
+ : index = matchList.matches.length;
+
+ final RouteMatchList matchList;
+ int index = 0;
+ final NavigatorState root;
+ @override
+ late NavigatorState current;
+
+ @override
+ bool moveNext() {
+ if (index < 0) {
+ return false;
+ }
+ 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<NavigatorState> parentNavigatorKey =
+ route.parentNavigatorKey!;
+ final ModalRoute<Object?>? 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;
+ }
+
+ current = parentNavigatorKey.currentState!;
+ return true;
+ } else if (route is ShellRoute) {
+ // Must have a ModalRoute parent because the navigator ShellRoute
+ // created must not be the root navigator.
+ final ModalRoute<Object?> parentModalRoute =
+ ModalRoute.of(route.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;
+ }
+ current = route.navigatorKey.currentState!;
+ return true;
+ }
+ }
+ assert(index == -1);
+ current = root;
+ return true;
+ }
+}
+
/// The route match that represent route pushed through [GoRouter.push].
// TODO(chunhtai): Removes this once imperative API no longer insert route match.
class ImperativeRouteMatch extends RouteMatch {
diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart
index 120bfd2..548747e 100644
--- a/packages/go_router/lib/src/router.dart
+++ b/packages/go_router/lib/src/router.dart
@@ -142,7 +142,7 @@
String get location => _location;
String _location = '/';
- /// Returns `true` if there is more than 1 page on the stack.
+ /// Returns `true` if there is at least two or more route can be pop.
bool canPop() => _routerDelegate.canPop();
void _handleStateMayChange() {
@@ -272,13 +272,16 @@
);
}
- /// Pop the top page off the GoRouter's page stack.
- void pop() {
+ /// Pop the top-most route off the current screen.
+ ///
+ /// If the top-most route is a pop up or dialog, this method pops it instead
+ /// of any GoRoute under it.
+ void pop<T extends Object?>([T? result]) {
assert(() {
log.info('popping $location');
return true;
}());
- _routerDelegate.pop();
+ _routerDelegate.pop<T>(result);
}
/// Refresh the route.
diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml
index 5655bd6..64eb598 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: 5.2.0
+version: 5.2.1
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/builder_test.dart b/packages/go_router/test/builder_test.dart
index ba4f7f5..04e7543 100644
--- a/packages/go_router/test/builder_test.dart
+++ b/packages/go_router/test/builder_test.dart
@@ -346,7 +346,7 @@
@override
Widget build(BuildContext context) {
return MaterialApp(
- home: builder.tryBuild(context, matches, () {}, false,
+ home: builder.tryBuild(context, matches, (_, __) => false, false,
routeConfiguration.navigatorKey, <Page<Object?>, GoRouterState>{}),
// builder: (context, child) => ,
);
diff --git a/packages/go_router/test/delegate_test.dart b/packages/go_router/test/delegate_test.dart
index 3974223..7df8504 100644
--- a/packages/go_router/test/delegate_test.dart
+++ b/packages/go_router/test/delegate_test.dart
@@ -36,24 +36,21 @@
testWidgets('removes the last element', (WidgetTester tester) async {
final GoRouter goRouter = await createGoRouter(tester)
..push('/error');
+ await tester.pumpAndSettle();
- goRouter.routerDelegate.addListener(expectAsync0(() {}));
final RouteMatch last = goRouter.routerDelegate.matches.matches.last;
- goRouter.routerDelegate.pop();
+ await goRouter.routerDelegate.popRoute();
expect(goRouter.routerDelegate.matches.matches.length, 1);
expect(goRouter.routerDelegate.matches.matches.contains(last), false);
});
- testWidgets('throws when it pops more than matches count',
+ testWidgets('pops more than matches count should return false',
(WidgetTester tester) async {
final GoRouter goRouter = await createGoRouter(tester)
..push('/error');
- expect(
- () => goRouter.routerDelegate
- ..pop()
- ..pop(),
- throwsA(isAssertionError),
- );
+ await tester.pumpAndSettle();
+ await goRouter.routerDelegate.popRoute();
+ expect(await goRouter.routerDelegate.popRoute(), isFalse);
});
});
diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart
index d301742..35198e0 100644
--- a/packages/go_router/test/go_router_test.dart
+++ b/packages/go_router/test/go_router_test.dart
@@ -2549,15 +2549,6 @@
});
group('Imperative navigation', () {
- testWidgets('pop triggers pop on routerDelegate',
- (WidgetTester tester) async {
- final GoRouter router = await createGoRouter(tester)
- ..push('/error');
- router.routerDelegate.addListener(expectAsync0(() {}));
- router.pop();
- await tester.pump();
- });
-
group('canPop', () {
testWidgets(
'It should return false if Navigator.canPop() returns false.',
@@ -2736,7 +2727,55 @@
expect(router.canPop(), true);
},
);
+ testWidgets('Pageless route should include in can pop',
+ (WidgetTester tester) async {
+ final GlobalKey<NavigatorState> root =
+ GlobalKey<NavigatorState>(debugLabel: 'root');
+ final GlobalKey<NavigatorState> shell =
+ GlobalKey<NavigatorState>(debugLabel: 'shell');
+
+ final GoRouter router = GoRouter(
+ navigatorKey: root,
+ routes: <RouteBase>[
+ ShellRoute(
+ navigatorKey: shell,
+ builder:
+ (BuildContext context, GoRouterState state, Widget child) {
+ return Scaffold(
+ body: Center(
+ child: Column(
+ children: <Widget>[
+ const Text('Shell'),
+ Expanded(child: child),
+ ],
+ ),
+ ),
+ );
+ },
+ routes: <RouteBase>[
+ GoRoute(
+ path: '/',
+ builder: (_, __) => const Text('A Screen'),
+ ),
+ ],
+ ),
+ ],
+ );
+
+ await tester.pumpWidget(MaterialApp.router(routerConfig: router));
+
+ expect(router.canPop(), isFalse);
+ expect(find.text('A Screen'), findsOneWidget);
+ expect(find.text('Shell'), findsOneWidget);
+ showDialog(
+ context: root.currentContext!,
+ builder: (_) => const Text('A dialog'));
+ await tester.pumpAndSettle();
+ expect(find.text('A dialog'), findsOneWidget);
+ expect(router.canPop(), isTrue);
+ });
});
+
group('pop', () {
testWidgets(
'Should pop from the correct navigator when parentNavigatorKey is set',
@@ -2815,6 +2854,74 @@
expect(find.text('Shell'), findsNothing);
},
);
+
+ testWidgets('Should pop dialog if it is present',
+ (WidgetTester tester) async {
+ final GlobalKey<NavigatorState> root =
+ GlobalKey<NavigatorState>(debugLabel: 'root');
+ final GlobalKey<NavigatorState> shell =
+ GlobalKey<NavigatorState>(debugLabel: 'shell');
+
+ final GoRouter router = GoRouter(
+ initialLocation: '/a',
+ navigatorKey: root,
+ routes: <GoRoute>[
+ GoRoute(
+ path: '/',
+ builder: (BuildContext context, _) {
+ return const Scaffold(
+ body: Text('Home'),
+ );
+ },
+ routes: <RouteBase>[
+ ShellRoute(
+ navigatorKey: shell,
+ builder: (BuildContext context, GoRouterState state,
+ Widget child) {
+ return Scaffold(
+ body: Center(
+ child: Column(
+ children: <Widget>[
+ const Text('Shell'),
+ Expanded(child: child),
+ ],
+ ),
+ ),
+ );
+ },
+ routes: <RouteBase>[
+ GoRoute(
+ path: 'a',
+ builder: (_, __) => const Text('A Screen'),
+ ),
+ ],
+ ),
+ ],
+ ),
+ ],
+ );
+
+ await tester.pumpWidget(MaterialApp.router(routerConfig: router));
+
+ expect(router.canPop(), isTrue);
+ expect(find.text('A Screen'), findsOneWidget);
+ expect(find.text('Shell'), findsOneWidget);
+ expect(find.text('Home'), findsNothing);
+ final Future<bool?> resultFuture = showDialog<bool>(
+ context: root.currentContext!,
+ builder: (_) => const Text('A dialog'));
+ await tester.pumpAndSettle();
+ expect(find.text('A dialog'), findsOneWidget);
+ expect(router.canPop(), isTrue);
+
+ router.pop<bool>(true);
+ await tester.pumpAndSettle();
+ expect(find.text('A Screen'), findsOneWidget);
+ expect(find.text('Shell'), findsOneWidget);
+ expect(find.text('A dialog'), findsNothing);
+ final bool? result = await resultFuture;
+ expect(result, isTrue);
+ });
});
});
}
diff --git a/packages/go_router/test/test_helpers.dart b/packages/go_router/test/test_helpers.dart
index be35707..9ead9a6 100644
--- a/packages/go_router/test/test_helpers.dart
+++ b/packages/go_router/test/test_helpers.dart
@@ -130,7 +130,7 @@
bool popped = false;
@override
- void pop() {
+ void pop<T extends Object?>([T? result]) {
popped = true;
}
}
