fix: Discard changes modal does't show up when going back with the browser back button#93268
Conversation
|
@bernhardoj 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] |
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d260aef32
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8097dc1a66
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b36d6aff1e
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 404919321c
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
…-changes-modal-doesnt-show-up-when-going-back-with-the-browser-back-button
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1be4583a9b
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1be4583a9b
ℹ️ 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".
|
🚧 MonilBhavsar has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.4.22-1 🚀
|
|
Deploy Blocker #94844 was identified to be related to this PR. |
|
Deploy Blocker #94845 was identified to be related to this PR. |
|
Deploy Blocker #94861 was identified to be related to this PR. |
|
This PR failing because of the issue #94863 |
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.4.24-0 🚀
|
|
Deploy Blocker #94910 was identified to be related to this PR. |
Explanation of Change
Browser back never showed the "Discard changes" confirmation. The back navigation arrives as a root-targeted
RESETthat keeps all route keys and only changes nested state, andshouldPreventRemoveonly inspects removed keys — sousePreventRemove/beforeRemovenever fired react-navigation/react-navigation#9031.The bug has two variants, each fixed before the navigation state commits:
Nested removal (Merchant, Description, Avatar): patched
@react-navigation/coreto propagate thebeforeRemovecheck into nested navigators on a key-preservingRESET.Focus-only change (tab switches — nothing is removed, so
beforeRemovecan't fire by design): on web, a newDiscardChangesGuardvetoes aRESETthat would unfocus a registered dirty screen, skipping the guard framework'sreplaceStatehistory sync (it would desynccreateMemoryHistory) in favor of ahistory.go(1)URL restore. On Android, the same switch arrives as a TabRouterGO_BACK, intercepted with aBackHandlerin the native hook — a router-level guard isn't possible becausecanGoBack()probes callgetStateForActionspeculatively.Also:
addRootHistoryRouterExtensionnow passes same-reference (blocked/no-op) router results through untouched; rehydrating them leaked a state event that rewrote the back-target history entry.Fixed Issues
$ #84246
PROPOSAL: #84246 (comment)
Tests
Scope: This PR covers the back button only (browser back, the phone back button, and the iOS edge-swipe). Clicking between tabs is handled by a separate PR (#89612) and is out of scope here — clicking a tab that has unsaved data is not expected to show a warning in this build, so do not raise that as a bug against this PR.
Setup: Sign in to an account with a workspace that has time tracking turned on and Receipt partners (Uber) turned on (admin access). Have one distance expense created from the Map and one ordinary expense already in a chat.
Scenario 1 — Create expense
Scenario 2 — Track distance
Scenario 3 — Edit distance
Scenario 4 — Split expense
Scenario 5 — Start chat
Scenario 6 — Receipt partner invites
Scenario 7 — Share to Expensify (mobile web only)
Scenario 8 — Entering data still warns before leaving (Track distance)
Scenario 9 — Entering data still warns before leaving (Time)
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android-Native.mp4
Android: mWeb Chrome
Android-mWeb.mp4
iOS: Native
iOS: mWeb Safari
iOS-Safari.mp4
MacOS: Chrome / Safari
Mac-Chrome.mp4