Fix navigation race condition when closing Wide RHP after the deletion of last transaction#80606
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
blazejkustra
left a comment
There was a problem hiding this comment.
Fix looks solid, just one question:
What happens if popstate never fires for some reason? In that scenario, onModalHide() would never be called. For example, if there's no history to go back to (empty history stack), would history.back() still trigger a popstate event? Or could the listener end up waiting indefinitely?
@blazejkustra I’ve looked into the edge case where popstate might not fire. While it's true that window.history.back() does nothing if the history stack length is 0 or 1, that scenario is impossible in our current implementation. The modal pushes a metadata entry to the history stack as soon as it opens. If that entry is missing for any reason, the modal logic is designed to resolve immediately upon closing anyway. Therefore, an extra safety guard isn't necessary here. |
|
@ZhenjaHorbach @mountiny One of you needs to 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] |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ 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". |
|
@collectioneur Can you please make sure to react and respond/ mark as resolved on all AI comments to make it clear that you addressed them? thanks! |
|
@ZhenjaHorbach can you please review this PR? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
mountiny
left a comment
There was a problem hiding this comment.
Thanks, lets give it a go
|
🚧 @mountiny 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/mountiny in version: 9.3.13-1 🚀
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.15-0 🚀
|
Explanation of Change
A race condition occurred when the modal button was pressed: the modal promise resolved before
window.history.back()finished. This caused React Navigation to move to the chat while the history stack was still updating. Consequently, the delayedback()action triggered after the navigation, incorrectly landing the user back on the report page. Nowwindow.history.back()completes before the modal promise resolves. This guarantees the browser history is stable before the application initiates the transition to the chat.Fixed Issues
$ #79732
Tests
Offline tests
QA Steps
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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-01-27.at.11.39.25.mov