Fix: restore Wallet background for missing personal details RHP after reload#95363
Fix: restore Wallet background for missing personal details RHP after reload#95363MelvinBot wants to merge 1 commit into
Conversation
…eload Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates from your fork, run: Then you can close this PR and open a new one from your fork. Otherwise, feel free to push directly to this upstream branch if you have access. |
|
The only failing check is It fails because the checklist section of the PR body is incomplete (the workflow reports Everything else is green or still running:
No code change is warranted here, so I haven't pushed anything. If the pending jobs above actually fail once they finish, re-tag me and I'll dig into those specifically. |
|
@MelvinBot Mark the PR ready |
|
🤖 Done — PR #95363 is now marked as Ready for review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d289cb1d12
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| SCREENS.MISSING_PERSONAL_DETAILS, | ||
| SCREENS.MISSING_PERSONAL_DETAILS_CONFIRM_MAGIC_CODE, |
There was a problem hiding this comment.
Keep home-launched details flows on Home after reload
When the missing-personal-details RHP is opened from the Home time-sensitive card or issue card (AddVirtualCardPersonalDetails.tsx:31, IssueCardMessage.tsx:63), the URL still has no backTo parameter, so after a browser reload getMatchingFullScreenRoute() will now hit this new RHP_TO_SETTINGS entry and rebuild Settings > Wallet behind the RHP instead of preserving Home. This fixes the Wallet entry point but regresses the existing Home-launched flow; the mapping needs a way to distinguish Wallet launches from Home/default launches rather than assigning this screen globally to Wallet.
Useful? React with 👍 / 👎.
|
Re-checked — nothing to fix in the code. The two remaining
I'm not permitted to tick either of those on a human's behalf, so they clear through the normal review flow, not a code push. Every other check (tests, typecheck, builds, lint, etc.) is now green, so I haven't pushed any changes. |
|
@DylanDylann do I need to review this? |
|
Let's wait until @chuckdries responds to my comment in the issue: #95362 (comment) |
Explanation of Change
Deploy blocker fix for #95362.
When the missing-personal-details flow is opened as an RHP from the Wallet (e.g. Settings → Wallet → virtual card → Reveal, introduced in #91634), reloading the browser while the RHP is open changed the background page from Wallet to the Inbox.
Root cause: on reload the navigation state is rebuilt from the URL.
getMatchingFullScreenRoutedetermines which full-screen page belongs behind an RHP by looking the RHP screen up in theRHP_TO_*relation maps (derived from the*_TO_RHPmaps).SCREENS.MISSING_PERSONAL_DETAILS/SCREENS.MISSING_PERSONAL_DETAILS_CONFIRM_MAGIC_CODEwere not present in any of those maps and the route carries nobackToparam, so the lookup returnedundefinedand the background fell back to the defaultHOME(Inbox).Fix: register both screens under
SCREENS.SETTINGS.WALLET.ROOTinSETTINGS_TO_RHP.ts, matching every other Wallet-launched RHP (e.g.DOMAIN_CARD,REPORT_VIRTUAL_CARD_FRAUD).RHP_TO_SETTINGSis the inverse of this map, so the Wallet page is now correctly restored as the background after reload.Fixed Issues
$ #95362
PROPOSAL: #95362 (comment)
Tests
Automated checks run by MelvinBot on this change:
npm run typecheck-tsgo— passedoxfmt(npm run fmt) — cleantests/navigation+DeeplinkRouteNormalizationTest) — 429 passedOffline tests
Same as Tests (navigation/linking behavior is not network-dependent).
QA Steps
PR Author Checklist
### Fixed Issuessection aboveScreenshots/Videos
N/A — navigation linking config change; behavior verified via the navigation unit-test suite.