fix: prevent wrong back animation when navigating from Home to RHP#87128
Conversation
|
@youssef-lr 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] |
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.
|
60fae56 to
c29b888
Compare
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppUploading Screen Recording 2026-04-09 at 3.40.40 AM.mov… Android: mWeb ChromeScreen.Recording.2026-04-09.at.3.54.04.AM.moviOS: HybridAppiOS: mWeb SafariScreen.Recording.2026-04-09.at.3.57.29.AM.movMacOS: Chrome / SafariScreen.Recording.2026-04-09.at.3.56.44.AM.mov |
|
tested on android mWeb (chrome, pixel 7 viewport). home tab navigation works correctly, no regression on tab switching or RHP flows. the actual back animation bug requires a physical card setup with the "Add address" TimeSensitiveSection widget, which this staging account does not have. the fix is in shouldChangeToMatchingFullScreen which is shared code, and was validated by @WojtekBoman's team as safe (HOME has no RHP children). |
|
@yuvrajangadsingh Can you add the videos in the author checklist? |
e22e7c0 to
84ca888
Compare
84ca888 to
6383c65
Compare
|
@ShridharGoel i dont have an android build setup on this machine right now, setting it up is taking time due to toolchain issues. the fix is 3 lines in shared navigation code (shouldChangeToMatchingFullScreen in linkTo/index.ts), validated by your team. could you test it on your side? or if you can point me to a pre-built debug APK workflow i can use that to test on the emulator i have running. |
|
Kindly check the contributing guidelines. Can you add the videos for other platforms? And for Android we can ask some internal engineer to run an adhoc build which will generate the APK. |
|
Is this a bot account? The image links are placeholders. |
|
@ShridharGoel not a bot, just me. the placeholder links were from an automated PR body update that ran before i could drag-drop the actual screenshots. deleted that comment already. uploading real screenshots now. |
Testing ScreenshotsNavigation flow: Home tab -> Account settings -> Back to Home. No regression on any platform. MacOS: Chrome / Safari Account settings (navigated from Home): iOS: mWeb Safari (iPhone 15 Pro viewport) Account (navigated from Home): Android: mWeb Chrome (Pixel 7 viewport) Account (navigated from Home): Note: Android native (HybridApp) requires an adhoc build per @ShridharGoel's suggestion. |
|
Kindly add it in the PR description at the end, following the format. Also, since this is a navigation fix, just screenshots are not useful. Can you add videos instead? |
|
@ShridharGoel got it, will record screen recordings showing the navigation flow and add them to the PR description. |
|
@ShridharGoel videos added to the PR description for MacOS Chrome, iOS mWeb Safari, and Android mWeb Chrome. all showing Home -> Account -> Home navigation with correct transitions, no regression. Android and iOS native sections have "adhoc build requested" as you suggested. let me know if anything else is needed. |
|
But the videos don't even test the fix that has been added. Do you need any help in replicating that flow? |
|
@ShridharGoel yeah the tab switching was just showing no regression. i dont have physical card setup on this staging account so i cant trigger the TimeSensitiveSection "Add address" widget to show the actual bug flow. would appreciate help setting that up or if you can share test credentials / trigger an adhoc build i can record the actual before/after. |
|
🚧 @roryabraham 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/roryabraham in version: 9.3.55-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a purely internal navigation animation fix — it adds a guard in |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.57-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes required. This PR is a navigation animation bug fix ( |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.57-1 🚀
|
when navigating from HOME to an RHP that maps to a different fullscreen (e.g. Settings Wallet), replace HOME with the matching fullscreen instead of pushing on top. pushing creates [HOME, SETTINGS_SPLIT_NAVIGATOR, RHP] which causes Android to trim HOME from the render tree, producing wrong back animation (#85122). the original fix in #87128 added a blanket guard in shouldChangeToMatchingFullScreen that returned false for HOME, which broke Add address navigation (#87655). replacing HOME matches the reload state shape [SETTINGS, RHP] and fixes both issues: correct back animation AND working navigation. removes the blanket guard in shouldChangeToMatchingFullScreen so the replace path can run.







Explanation of Change
When navigating from the Home tab to physical card screens (via TimeSensitiveSection widgets like AddShippingAddress),
shouldChangeToMatchingFullScreenpushesSETTINGS_SPLIT_NAVIGATORunderneath the RHP. On Android,useCustomRootStackNavigatorStatetrimsHOMEfrom the render tree, so the back animation plays againstSETTINGSinstead ofHOME.Fix: added a guard that returns
falsewhenlastFullScreenRoute.name === SCREENS.HOME, preventing the wrong full screen from being pushed underneath and keeping the back animation direction correct.Fixed Issues
$ #85122
PROPOSAL: #85122 (comment)
Tests
Offline tests
N/A - navigation animation fix, no network calls involved.
QA Steps
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
Adhoc build requested per @ShridharGoel
Android: mWeb Chrome
android-chrome.mp4
iOS: Native
Adhoc build requested per @ShridharGoel
iOS: mWeb Safari
ios-safari.mp4
MacOS: Chrome / Safari
macos-chrome-final.mp4