[Odometer] Improvement to DiscardChangesConfirmation usage#87269
Conversation
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.
|
…for_editing_from_confirmation
…age with unsaved changes
…for_editing_from_confirmation
…w proper cleanup of backup transaction
|
@DylanDylann 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: 13f1f51dd2
ℹ️ 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".
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
|
@codex review |
|
Could you revisit this comment? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8581be41a0
ℹ️ 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".
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
@DylanDylann I asked @Expensify/design to discuss this topic on slack here |
|
The rest looks great |
…for_editing_from_confirmation
…for_editing_from_confirmation
|
@DylanDylann Do you mean to replace this whole condition with a single |
|
I think there is some potential to that idea - I will look into that in the morning |
I mean replace this whole condition with a single isSaved (and remove isFocused condition) |
We might not be able to do that completely without covering all edge cases but I will do my best (those two |
|
@jakubkalinski0 This is just a nit (a refactor for better readability). If there are any technical concerns, I’m fine keeping it as is. |
I understand, but I think that it's a good idea and that we should be able to implement this approach partially and make it at least more readable |
…for_editing_from_confirmation
|
@DylanDylann Hi! I looked into this and unfortunately we cannot really replace any part of this condition with
Screen.Recording.2026-04-21.at.13.48.55.mov
But we cannot replace it with
So in the end I think that keeping the original consistent two-ref approach is better than replacing one of those refs with |
|
I will now check this one out
|
…ce it with isEditing
…mation in IOURequestStepDistanceOdometer
|
@DylanDylann I've applied |
|
@codex review |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-04-22.at.16.03.43.movAndroid: mWeb ChromeScreen.Recording.2026-04-22.at.15.56.06.moviOS: HybridAppScreen.Recording.2026-04-22.at.15.58.41.moviOS: mWeb SafariScreen.Recording.2026-04-22.at.15.56.06.movMacOS: Chrome / SafariScreen.Recording.2026-04-22.at.15.54.46.mov |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ 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". |
|
Thanks for checking out that condition improvement, regardless! |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.3.62-0 🚀
Bundle Size Analysis (Sentry): |

Explanation of Change
Part 1
This PR narrows the scope of discard-navigation handling by reverting the overextended changes from commit fb26c40 and restoring screen-level ownership of guard conditions. In the shared discard hooks (
useBeforeRemove,useDiscardChangesConfirmationweb/native),isEnabled/isFocusedwas removed so listeners stay consistently attached and rely ongetHasUnsavedChanges()for final blocking decisions making the caller responsible for controlling when the modal is active by utilizing the following patter:For the odometer distance flow, unsaved-state detection was tightened in
IOURequestStepDistanceOdometer. The modal is blocked when the screen is unfocused, when editing existing expenses, after trying to leave when editing-from-confirmation, and after skip-confirmation submit.When removing
isEnabledchange was required for MFA authorize-transaction, thebeforeRemovehandler now exits early when navigation is already allowed, no transaction is loaded, or the deny outcome screen is shown, preventing unnecessary cancel/discard prompts while preserving the intended confirmation when a pending transaction is being reviewed.In order to ensure users are not navigated to confirmation before the backup transaction is restored and prevent stitching of discarded images.
useDiscardChangesConfirmationnow supports asynconConfirmand defers navigation untilonConfirmsettlesrestoreOriginalTransactionFromBackupWithImageCleanupnow returns aPromise<void>so callers can await rollback completionIOURequestStepDistanceOdometer, discard from confirmation now awaits rollback before navigating backPart 2
At the same time, this PR also introduces Discard Changes functionality when odometer readings are edited from the confirmation step, leaving with real unsaved changes triggers the discard-changes modal. (it was agreed here - #86022 (comment))
Fixed Issues
$ #79502
PROPOSAL: N/A
Tests
We are testing the odometer expense creation flow which you start by doing the following:
Press
FAB-> go to"Track distance"-> choose"Odometer"tabDistancefield to edit odometer values, and change either start or end reading or any of the images<back buttonCanceland verify that you stay on the odometer page with the unsaved value still visibleDiscardand verify that you are brought back to the confirmation page and the changes were in fact discarded.Saveto save and return to confirmationOffline 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
output1.mp4
Android: mWeb Chrome
output2.mp4
iOS: Native
Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2026-04-10.at.17.44.04.mov
iOS: mWeb Safari
output3.mp4
MacOS: Chrome / Safari
output.mp4