[CP Staging] Revert "Fix reveal navigation under RHP + re-land reveal workspace route under RHP before dismissing on workspace creation "#90982
Conversation
…ute under RHP before dismissing on workspace creation "
melvin-bot
Bot
requested review from
danieldoglas and
joekaufmanexpensify
and removed request for
a team
|
@danieldoglas Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
Asking QA to confirm the testers cannot reproduce on the adhoc build |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1955751f32
ℹ️ 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".
| const rehydrated = rehydrate(newState, configOptions); | ||
| return applyRevealPaddingOffset(state, rehydrated); | ||
| // @ts-expect-error newState may be partial, but getRehydratedState handles both partial and full states correctly. | ||
| return getRehydratedState(newState, configOptions); |
There was a problem hiding this comment.
When Navigation.revealRouteBeforeDismissingModal() runs on web, it dispatches REPLACE_FULLSCREEN_UNDER_RHP and then DISMISS_MODAL; this branch freezes history only for the replace, but the following dismiss falls through here and rebuilds history from the shortened route stack. In the common [fullscreen, RHP] -> [target] reveal flow, useLinking observes a negative history delta and pops browser history instead of replacing the current RHP entry, so the browser Back stack skips or loses the previous fullscreen entry after submitting/navigating from an RHP.
Useful? React with 👍 / 👎.
| ...r, | ||
| ...(focusedTargetTab.params !== undefined ? {params: focusedTargetTab.params} : {}), | ||
| ...(mergedNestedState !== undefined ? {state: mergedNestedState as typeof r.state} : {}), |
There was a problem hiding this comment.
For tab-target reveals, spreading r keeps any existing params.screen/params.params deep-link hints on the tab route when the new focused tab has no params. If the tab was previously hydrated from a deep link, React Navigation will process those stale params after mount and issue a nested navigate that can override the freshly spliced state, so revealing a workspace/search tab under an RHP can land back on the old nested screen instead of the requested route.
Useful? React with 👍 / 👎.
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
Reproduced on the above ad-hoc (post-revert). repro.mp4 |
|
No product review needed |
|
This was not the offending PR in the end, the reproduction steps were inconsistent |
3 participants
Reverts #90044
Fixed issues
$ #90880