[Odometer] Show discard changes modal when switching tabs#89612
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a624d5330
ℹ️ 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: 31b9b37c2f
ℹ️ 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: b2690b40a8
ℹ️ 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: b2690b40a8
ℹ️ 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: bd1d392ced
ℹ️ 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: f9086f72ef
ℹ️ 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: 4f59a798f4
ℹ️ 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: 62149d8334
ℹ️ 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: 62149d8334
ℹ️ 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. Hooray! ℹ️ 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". |
…causing double-restore
…_modal_when_switching_tabs
…itch Removes the browser-Back-button discard mechanism introduced in 4ab9a73: the tab-history replacement router, goForwardInBrowserHistory helper, and the page-level useActiveTabGuardDiscardConfirmation hook. Restores the per-step useDiscardChangesConfirmation (header/hardware back) and keeps the core tab-switch discard modal and the resync guard untouched.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ 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". |
|
Took a deep look at the PR since our flows now compose. It's great! But one architectural thought: Smaller things:
|
…_modal_when_switching_tabs
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd02532305
ℹ️ 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".
…ories Add a Growl.error toast in OnyxTabNavigator when a tab-switch onDiscard rejects (alongside the existing Log.warn + always-jump), so a failed reset isn't silent. Replace remaining no-unsafe-type-assertion casts in the odometer tests with typed factories: OdometerDraft literals are cast-free (all-optional), and Transaction fixtures build on createRandomTransaction. Removes the seatbelt rows for the Backup/UtilsTest/ReadingsState/TransactionBackup tests and trims the two DISTANCE_CREATE route casts to one each.
…_modal_when_switching_tabs
|
@codex review |
|
@TaduJR Great input there! I liked your idea and refactored the code in that direction. Could you please check it out if it's more to your liking? |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ 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". |
…_modal_when_switching_tabs
|
@DylanDylann All yours! (Don't get scared about the LOC, it's mostly tests 😅 ) |
| "../../tests/ui/ForYouSectionTest.tsx" "@typescript-eslint/no-unsafe-type-assertion" 3 | ||
| "../../tests/ui/IOURequestStartPageTest.tsx" "@typescript-eslint/no-unsafe-type-assertion" 2 | ||
| "../../tests/ui/IOURequestStepAmountDraftTest.tsx" "@typescript-eslint/no-unsafe-type-assertion" 2 | ||
| "../../tests/ui/IOURequestStepDistanceOdometerNextSyncTest.tsx" "@typescript-eslint/no-unsafe-type-assertion" 1 |
There was a problem hiding this comment.
No need to update it manually. It will be updated automatically when it is merged into main.
| return register({ | ||
| tabName, | ||
| getHasUnsavedChanges: () => guardCallbacksRef.current.getHasUnsavedChanges(), | ||
| onDiscard: () => guardCallbacksRef.current.onDiscard?.(), | ||
| onCancel: () => guardCallbacksRef.current.onCancel?.(), |
There was a problem hiding this comment.
@jakubkalinski0 When switching away from the distance odometer tab, this tab isn't always unmounted
Screen.Recording.2026-06-17.at.16.39.49.mov
| odometerEndImage?: string; | ||
|
|
||
| /** `lastModified` of the start image, preserved so the re-minted image keeps its identity (getOdometerImageIdentity) */ | ||
| odometerStartImageLastModified?: number; |
Explanation of Change
This PR adds a feature we agreed on in this discussion to show a Discard Changes confirmation modal when the user tries to switch away from the Odometer tab to another tab with unsaved changes.
The tab-switch guard is generic, not odometer-specific. A new
TabSwitchGuardContextlets any tab screen registergetHasUnsavedChanges/onDiscard/onCancelcallbacks keyed by tab name, andOnyxTabNavigatorinterceptstabPressto show the shared "Discard changes?" modal - on confirm it runsonDiscardand then jumps to the target tab (it always jumps, even ifonDiscardfails, surfacing a Growl error rather than stranding the user). It's wired through the existinguseDiscardChangesConfirmationhook via a newonTabSwitchDiscardoption that self-disables outside a tab navigator, and both the nav-away and tab-switch paths render an identical modal fromgetDiscardChangesModalConfig. On the Odometer tab, confirming clears the readings/images (or restores the confirmation-edit backup when editing) before the tab switch.The discard diff is hardened against false positives: image changes use a re-mint-invariant identity (
getOdometerImageIdentity,name|size|lastModified) that survives the draft round-trip but still detects a real swap, and the resync / unsaved-changes branching is extracted into unit-tested pure predicates (odometerResync.ts,getOdometerHasUnsavedChanges) that won't re-hydrate readings the user intentionally cleared.Diff overview
24 files changed · +2097 / −197 - breakdown of
main...HEADat 290f76aTests vs. non-test source
So roughly three quarters of the diff is tests - new unit + UI tests covering the generic tab-switch guard, the resync predicates, the odometer draft round-trip, and the edit-from-confirmation backup/restore path.
Inside the 519 non-test insertions
OnyxTabNavigator.tsx(modified)IOURequestStepDistanceOdometer.tsx(modified)odometerResync.ts(NEW)OdometerTransactionUtils.ts(modified)TabSwitchGuardContext.tsx(NEW)getDiscardChangesModalConfig.ts(NEW)OdometerImageUtils.ts(modified)useOdometerReadingsState.ts(modified)useDiscardChangesConfirmation/index.native.ts(modified)useDiscardChangesConfirmation/index.ts(modified)useDiscardChangesConfirmation/types.ts(modified)OdometerDraft.ts(modified)DistanceRequestStartPage.tsx(modified)Attachment.ts(modified)cspell.json+eslint.seatbelt.tsv(data)The tab-switch registration was added to both the web (
index.ts) and native (index.native.ts) variants ofuseDiscardChangesConfirmation- they already had a platform split (usePreventRemoveon native vsuseBeforeRemoveon web), and the new guard hooks into each identically. The newTabSwitchGuardContextand theOnyxTabNavigatorintegration are single cross-platform files: the guard lives at the navigator level, so there is no platform-specific behavior to split out.Summary
Deletions (197 lines)
OnyxTabNavigator.tsxtabPresslistener (rewrite, not removal)IOURequestStepDistanceOdometer.tsxgetOdometerHasUnsavedChangesuseOdometerReadingsState.tsOdometerTransactionUtils.tsisOdometerDraftPendingHydrationrewrite + comment removalindex.native.ts/index.tsgetDiscardChangesModalConfigOdometerImageUtils.tsDistanceRequestStartPage.tsx/Attachment.ts/eslint.seatbelt.tsvBottom line: the PR is overwhelmingly additive. Of the 197 deletions, ~104 are source code and the rest are comments (39), tests (52), one blank line, and config. Crucially, almost none of those 104 lines are dead-code removal - they are rewrites/relocations: the
OnyxTabNavigatorreturn block was restructured to add the guard, and the odometer discard logic was moved out and re-expressed as pure predicates. Net new "real" source logic is roughly +263 lines (367 added − 104 removed), concentrated in the generic tab-guard wiring (OnyxTabNavigator+TabSwitchGuardContext), theodometerResync/getOdometerHasUnsavedChangespredicates, and thelastModifiedimage-identity plumbing. Tests (1578 insertions / 52 deletions) are ~75% of the diff.Fixed Issues
$ #89940
PROPOSAL: N/A
Tests
OnyxTabNavigator-based flow that does not register a guard (e.g. the Map/Manual distance tabs with no unsaved odometer changes) and verify tab switches happen immediately with no modal.Odometer regression tests (optional)
Nav-away discard still works
Save-for-later draft
Offline tests
Same as Tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
untitled.webm
Android: mWeb Chrome
untitled2.webm
iOS: Native
Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2026-06-16.at.23.05.25.mov
iOS: mWeb Safari
Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2026-06-16.at.23.22.23.mov
MacOS: Chrome / Safari
Screen.Recording.2026-06-16.at.20.56.57.mov