Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[go_router] Fixes an issue where android back button pops wrong page. #7348

Merged
merged 4 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/go_router/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 14.2.5

- Fixes an issue where android back button pops pages in the wrong order.

## 14.2.4

- Updates minimum supported SDK version to Flutter 3.19/Dart 3.3.
Expand Down
41 changes: 20 additions & 21 deletions packages/go_router/lib/src/delegate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>

@override
Future<bool> 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) {
return state.maybePop();
}
Expand All @@ -75,7 +61,8 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
if (lastRoute.onExit != null && navigatorKey.currentContext != null) {
return !(await lastRoute.onExit!(
navigatorKey.currentContext!,
walker.buildState(_configuration, currentConfiguration),
currentConfiguration.last
.buildState(_configuration, currentConfiguration),
));
}
return false;
Expand All @@ -98,21 +85,33 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>

/// Pops the top-most route.
void pop<T extends Object?>([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;
}
RouteMatchBase walker = currentConfiguration.matches.last;
while (walker is ShellRouteMatch) {
if (walker.navigatorKey.currentState?.canPop() ?? false) {
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;
}
if (potentialCandidate.canPop()) {
state = walker.navigatorKey.currentState;
}
walker = walker.matches.last;
}
if (state == null) {
throw GoError('There is nothing to pop');
}
state.pop(result);
return state;
}

void _debugAssertMatchListNotEmpty() {
Expand Down
2 changes: 1 addition & 1 deletion packages/go_router/lib/src/router.dart
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
setLogging(enabled: debugLogDiagnostics);
WidgetsFlutterBinding.ensureInitialized();

navigatorKey ??= GlobalKey<NavigatorState>();
navigatorKey ??= GlobalKey<NavigatorState>(debugLabel: 'root');

_routingConfig.addListener(_handleRoutingConfigChanged);
configuration = RouteConfiguration(
Expand Down
2 changes: 1 addition & 1 deletion packages/go_router/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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

Expand Down
61 changes: 61 additions & 0 deletions packages/go_router/test/go_router_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<RouteBase> routes = <RouteBase>[
GoRoute(
path: '/',
builder: (_, __) => const Text('home'),
routes: <RouteBase>[
ShellRoute(
builder: (
BuildContext context,
GoRouterState state,
Widget child,
) {
return Column(
children: <Widget>[
const Text('shell'),
child,
],
);
},
routes: <GoRoute>[
GoRoute(
path: 'page',
builder: (BuildContext context, __) {
return TextButton(
onPressed: () {
Navigator.of(context, rootNavigator: true).push(
MaterialPageRoute<void>(
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.
Expand Down