Skip to content

Commit 4160755

Browse files
committed
[go_router] Add maybePop for safe navigation pops
Introduce maybePop on GoRouter, GoRouterDelegate, and GoRouterHelper to mirror Navigator.maybePop and BackButton while calling restore() when a pop completes synchronously. Adds comprehensive tests for maybePop, restore, canPop, and mixed Navigator.pop usage.
1 parent 6ce00a8 commit 4160755

12 files changed

Lines changed: 1494 additions & 2 deletions

packages/go_router/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,4 @@ migrate_working_dir/
2828
.dart_tool/
2929
.packages
3030
build/
31+
/coverage/

packages/go_router/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 17.4.0
2+
3+
- Adds `maybePop` to `GoRouter`, `GoRouterDelegate`, and `GoRouterHelper`. This method mirrors [Navigator.maybePop] and [BackButton] by returning `false` instead of throwing when there is nothing to pop, and calls [GoRouter.restore] when a pop completes synchronously.
4+
15
## 17.3.0
26

37
- Updates minimum supported SDK version to Flutter 3.38/Dart 3.10.

packages/go_router/lib/src/delegate.dart

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,21 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> with ChangeNotifie
103103
states.first.pop(result);
104104
}
105105

106+
/// Calls [NavigatorState.maybePop] on the current navigator stack.
107+
///
108+
/// Returns `true` if a route was popped and `false` otherwise. This method
109+
/// does not throw if there is nothing to pop.
110+
Future<bool> maybePop<T extends Object?>([T? result]) async {
111+
final Iterable<NavigatorState> states = _findCurrentNavigators();
112+
for (final NavigatorState state in states) {
113+
final bool didPop = await state.maybePop<T>(result);
114+
if (didPop) {
115+
return true;
116+
}
117+
}
118+
return false;
119+
}
120+
106121
/// Get a prioritized list of NavigatorStates,
107122
/// which either can pop or are exit routes.
108123
///

packages/go_router/lib/src/misc/extensions.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,16 @@ extension GoRouterHelper on BuildContext {
6767
/// Returns `true` if there is more than 1 page on the stack.
6868
bool canPop() => GoRouter.of(this).canPop();
6969

70+
/// Pop the top page off the Navigator's page stack if possible.
71+
///
72+
/// Returns `true` if a route was popped and `false` otherwise. This method
73+
/// does not throw if there is nothing to pop.
74+
///
75+
/// See also:
76+
/// * [pop], which throws if there is nothing to pop.
77+
/// * [canPop], which can be used to check whether a pop is possible.
78+
Future<bool> maybePop<T extends Object?>([T? result]) => GoRouter.of(this).maybePop<T>(result);
79+
7080
/// Pop the top page off the Navigator's page stack by calling
7181
/// [Navigator.pop].
7282
void pop<T extends Object?>([T? result]) => GoRouter.of(this).pop(result);

packages/go_router/lib/src/router.dart

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,10 @@ class GoRouter implements RouterConfig<RouteMatchList> {
552552
///
553553
/// Ensure that the `value` of `routeInformationProvider` is synced
554554
/// with `routerDelegate.currentConfiguration`.
555+
///
556+
/// See also:
557+
/// * [maybePop], which returns `false` instead of throwing if there is
558+
/// nothing to pop.
555559
void pop<T extends Object?>([T? result]) {
556560
assert(() {
557561
log('popping ${routerDelegate.currentConfiguration.uri}');
@@ -566,6 +570,25 @@ class GoRouter implements RouterConfig<RouteMatchList> {
566570
}
567571
}
568572

573+
/// Pop the top-most route off the current screen if possible.
574+
///
575+
/// This method calls [NavigatorState.maybePop] on the underlying navigators,
576+
/// similar to [BackButton]. It returns `true` if a route was popped and
577+
/// `false` otherwise. Unlike [pop], this method does not throw if there is
578+
/// nothing to pop.
579+
///
580+
/// When a pop completes synchronously, this method also calls [restore] to
581+
/// keep the [routeInformationProvider] in sync with
582+
/// [GoRouterDelegate.currentConfiguration].
583+
Future<bool> maybePop<T extends Object?>([T? result]) async {
584+
final RouteMatchList configBeforePop = routerDelegate.currentConfiguration;
585+
final bool didPop = await routerDelegate.maybePop<T>(result);
586+
if (didPop && !identical(routerDelegate.currentConfiguration, configBeforePop)) {
587+
restore(routerDelegate.currentConfiguration);
588+
}
589+
return didPop;
590+
}
591+
569592
/// Refresh the route.
570593
void refresh() {
571594
assert(() {

packages/go_router/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: go_router
22
description: A declarative router for Flutter based on Navigation 2 supporting
33
deep linking, data-driven routes and more
4-
version: 17.3.0
4+
version: 17.4.0
55
repository: https://github.com/flutter/packages/tree/main/packages/go_router
66
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22
77

packages/go_router/test/delegate_test.dart

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,111 @@ void main() {
442442
expect(goRouter.routerDelegate.currentConfiguration.matches.length, 0);
443443
expect(goRouter.routerDelegate.canPop(), false);
444444
});
445+
446+
testWidgets('It should return false on screen A and true on screen B after push', (
447+
WidgetTester tester,
448+
) async {
449+
final GoRouter goRouter = GoRouter(
450+
initialLocation: '/a',
451+
routes: <GoRoute>[
452+
GoRoute(path: '/a', builder: (_, _) => const Text('Screen A')),
453+
GoRoute(path: '/b', builder: (_, _) => const Text('Screen B')),
454+
],
455+
);
456+
addTearDown(goRouter.dispose);
457+
await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter));
458+
await tester.pumpAndSettle();
459+
460+
expect(find.text('Screen A'), findsOneWidget);
461+
expect(goRouter.routerDelegate.canPop(), isFalse);
462+
463+
goRouter.push('/b');
464+
await tester.pumpAndSettle();
465+
466+
expect(find.text('Screen B'), findsOneWidget);
467+
expect(goRouter.routerDelegate.canPop(), isTrue);
468+
469+
goRouter.pop();
470+
await tester.pumpAndSettle();
471+
472+
expect(find.text('Screen A'), findsOneWidget);
473+
expect(goRouter.routerDelegate.canPop(), isFalse);
474+
expect(goRouter.routerDelegate.currentConfiguration.matches.length, 1);
475+
expect(goRouter.routerDelegate.currentConfiguration.uri.path, '/a');
476+
});
477+
478+
testWidgets('It should check shell navigator when root navigator cannot pop', (
479+
WidgetTester tester,
480+
) async {
481+
final shellNavigatorKey = GlobalKey<NavigatorState>();
482+
final GoRouter goRouter = GoRouter(
483+
initialLocation: '/a',
484+
routes: <RouteBase>[
485+
ShellRoute(
486+
navigatorKey: shellNavigatorKey,
487+
builder: (_, _, Widget child) => child,
488+
routes: <GoRoute>[
489+
GoRoute(path: '/a', builder: (_, _) => const Text('Screen A')),
490+
GoRoute(path: '/b', builder: (_, _) => const Text('Screen B')),
491+
],
492+
),
493+
],
494+
);
495+
addTearDown(goRouter.dispose);
496+
await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter));
497+
await tester.pumpAndSettle();
498+
499+
expect(goRouter.routerDelegate.canPop(), isFalse);
500+
expect(goRouter.routerDelegate.navigatorKey.currentState?.canPop(), isFalse);
501+
502+
goRouter.push('/b');
503+
await tester.pumpAndSettle();
504+
505+
expect(goRouter.routerDelegate.canPop(), isTrue);
506+
expect(shellNavigatorKey.currentState?.canPop(), isTrue);
507+
expect(goRouter.routerDelegate.navigatorKey.currentState?.canPop(), isFalse);
508+
509+
goRouter.pop();
510+
await tester.pumpAndSettle();
511+
512+
expect(find.text('Screen A'), findsOneWidget);
513+
expect(goRouter.routerDelegate.canPop(), isFalse);
514+
expect(shellNavigatorKey.currentState?.canPop(), isFalse);
515+
});
516+
});
517+
518+
group('willHandlePopInternally', () {
519+
testWidgets('does not remove route when page handles pop internally', (
520+
WidgetTester tester,
521+
) async {
522+
final GoRouter goRouter = GoRouter(
523+
initialLocation: '/',
524+
routes: <GoRoute>[
525+
GoRoute(
526+
path: '/',
527+
builder: (_, _) => const Text('Home'),
528+
routes: <GoRoute>[
529+
GoRoute(
530+
path: 'internal',
531+
pageBuilder: (_, _) => const _InternalPopPage(),
532+
),
533+
],
534+
),
535+
],
536+
);
537+
addTearDown(goRouter.dispose);
538+
await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter));
539+
540+
goRouter.push('/internal');
541+
await tester.pumpAndSettle();
542+
expect(find.text('Internal'), findsOneWidget);
543+
544+
goRouter.pop();
545+
await tester.pumpAndSettle();
546+
547+
expect(find.text('Internal'), findsOneWidget);
548+
expect(find.text('Home'), findsNothing);
549+
});
445550
});
446551

447552
group('pushReplacement', () {
@@ -752,3 +857,45 @@ class _DummyStatefulWidgetState extends State<DummyStatefulWidget> {
752857
@override
753858
Widget build(BuildContext context) => Container();
754859
}
860+
861+
class _InternalPopPage extends Page<void> {
862+
const _InternalPopPage();
863+
864+
@override
865+
Route<void> createRoute(BuildContext context) => _InternalPopRoute(this);
866+
}
867+
868+
class _InternalPopRoute extends PageRoute<void> {
869+
_InternalPopRoute(_InternalPopPage page) : super(settings: page);
870+
871+
@override
872+
Color? get barrierColor => null;
873+
874+
@override
875+
String? get barrierLabel => null;
876+
877+
@override
878+
bool get opaque => true;
879+
880+
@override
881+
bool get maintainState => true;
882+
883+
@override
884+
Duration get transitionDuration => Duration.zero;
885+
886+
@override
887+
bool get willHandlePopInternally => true;
888+
889+
@override
890+
// ignore: must_call_super
891+
bool didPop(void result) => false;
892+
893+
@override
894+
Widget buildPage(
895+
BuildContext context,
896+
Animation<double> animation,
897+
Animation<double> secondaryAnimation,
898+
) {
899+
return const Text('Internal');
900+
}
901+
}

0 commit comments

Comments
 (0)