Fix deep link route transition#2986
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines deep link navigation to make route transitions smoother and avoid stacking duplicate screens when opening links (including cold-start links), and adds a widget test to cover the de-duplication behavior.
Changes:
- Handle the cold-start initial link after the first frame and navigate without animation.
- Centralize deep link navigation via
_pushDeepLinkRoute, with logic to replace the current top route when it’s the same screen type. - Add a widget test ensuring
PuzzleScreenis replaced (not stacked) when opening the daily puzzle deep link twice.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/src/app_links_service.dart | Adds initial-link post-frame handling, animated routing support, and _pushDeepLinkRoute/no-transition helpers to prevent duplicate top screens. |
| test/app_links_service_test.dart | Adds a regression test verifying that repeated daily puzzle deep links don’t stack duplicate PuzzleScreens. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return MaterialScreenRoute( | ||
| screen: route.screen, | ||
| settings: route.settings, | ||
| fullscreenDialog: route.fullscreenDialog, |
There was a problem hiding this comment.
_withNoTransition rebuilds a ScreenRoute as a new MaterialScreenRoute, but it doesn't preserve route configuration like maintainState and allowSnapshotting from the original route. This can subtly change behavior for routes that rely on non-default values. Consider copying those properties from the original route when constructing the no-transition route.
| fullscreenDialog: route.fullscreenDialog, | |
| fullscreenDialog: route.fullscreenDialog, | |
| maintainState: route.maintainState, | |
| allowSnapshotting: route.allowSnapshotting, |
| return ElevatedButton( | ||
| onPressed: () { | ||
| ref.read(appLinksServiceProvider).handleDailyPuzzleLink(context, null); | ||
| }, | ||
| child: const Text('go to puzzle'), | ||
| ); | ||
| }, |
There was a problem hiding this comment.
In this test, handleDailyPuzzleLink is async but the onPressed callback doesn't await it. That can hide async failures and make pumpAndSettle() timing-dependent. Consider making onPressed async and awaiting the call (or explicitly using unawaited(...) if fire-and-forget is intended).
|
|
||
| // Second call from the home context (still mounted below PuzzleScreen): | ||
| // should replace rather than push. | ||
| capturedService!.handleDailyPuzzleLink(capturedContext!, null); |
There was a problem hiding this comment.
capturedService!.handleDailyPuzzleLink(...) returns a Future, but it's not awaited here. Awaiting it will make the test deterministic and ensures any thrown exceptions fail the test at the correct point (instead of surfacing as unhandled async errors).
| capturedService!.handleDailyPuzzleLink(capturedContext!, null); | |
| await capturedService!.handleDailyPuzzleLink(capturedContext!, null); |
This pull request improves deep link handling in the app, ensuring that navigation is smoother and more predictable when opening the app via a link or when tapping links while the app is already running.
Deep link navigation improvements:
PuzzleScreen) from piling up._pushDeepLinkRoutemethod to centralize navigation logic for deep links, including support for instant (no-animation) transitions when the app is cold-started from a link.handleDailyPuzzleLinkandhandleAppLinkto use the new navigation logic and accept ananimatedparameter, allowing for instant transitions when appropriate.App startup and link handling:
startmethod to handle the initial app link after the first frame, ensuring the navigator is ready and using an instant transition for a smoother user experience.