deeplink handling enhancement#2269
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa473472e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| host: String, | ||
| method: LinkOpenDefaultMethod, | ||
| ) { | ||
| launch { |
There was a problem hiding this comment.
Save defaults outside the picker composition
When this helper is called from the deep-link account picker, the click handler calls setAccountDefault/setBrowserDefault and then immediately dismisses the sheet. Because this write is launched in the presenter's rememberCoroutineScope, dismissing the picker removes the presenter from composition and can cancel the scope before updateAppSettings finishes, so the “Always open this way” choice may not persist. Use a longer-lived scope or complete the save before dismissing.
Useful? React with 👍 / 👎.
| url: String, | ||
| mapping: Map<UiAccount, List<PlatformDeepLink<*>>>, | ||
| ): ImmutableMap<UiAccount, DeeplinkRoute> { | ||
| ): ImmutableMap<UiAccount, Match> { |
There was a problem hiding this comment.
Unwrap matcher results for existing tests
Changing matches to return ImmutableMap<UiAccount, Match> means existing common tests that index matches[account] now compare a Match wrapper to the expected DeeplinkRoute (for example PlatformDeepLinkMatcherTest.matchesReturnsAccountProfileRoute), so CI common tests will fail despite the route still being present inside .route. Update the tests/callers to unwrap .route or keep a route-facing API.
Useful? React with 👍 / 👎.
fix #2264