fix: The Wallet Settings page is displayed instead of the settings menu page after opening "Add address" modal on the Home page on mobile web#88949
Conversation
…nu page after opening "Add address" modal on the Home page on mobile web
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.
|
|
The changes looks fine to me |
|
@ZhenjaHorbach 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-04-27.20.32.08.movAndroid: mWeb Chrome2026-04-27.20.30.11.moviOS: HybridApp2026-04-27.20.32.08.moviOS: mWeb Safari2026-04-27.20.30.11.movMacOS: Chrome / Safari2026-04-27.20.29.04.mov |
|
Let me review it before we merge as it's a change in one of the main functions in |
|
@dmkt9 some explanation of changes would be great 😄 |
|
For now we can modify |
|
🚧 @luacmartins has triggered a test Expensify/App build. You can view the workflow run here. |
|
@WojtekBoman I merged this PR since it seemed like it was ok from your previous comment. Let's clean this up as a follow up. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
fix: The Wallet Settings page is displayed instead of the settings menu page after opening "Add address" modal on the Home page on mobile web (cherry picked from commit ddef0ba) (cherry-picked to staging by luacmartins)
|
🚀 Cherry-picked to staging by https://github.com/luacmartins in version: 9.3.64-4 🚀
|
|
No help site documentation changes are required for this PR. This is a navigation bug fix in |
|
🚀 Cherry-picked to staging by https://github.com/luacmartins in version: 9.3.64-10 🚀
Bundle Size Analysis (Sentry): |
|
@adamgrzybowski @WojtekBoman @luacmartins I have added an explanation of the changes for this PR |
Explanation of Change
When we navigate to an RHP target route, we usually add an under–fullscreen router overlay. Currently, we only push the
lastRouteInMatchingFullScreenof the matching fullscreen (e.g.,Settings_Wallet) into the root state.After the TabBar PR was merged, the
sidebarScreenroute is no longer automatically added to the root state, so the state becomes[Settings_Wallet].If we're on a narrow layout and navigate to that matching fullscreen route via the navigation tab bar, the
sidebarScreengets added to the root state (becoming[Settings_Wallet, Settings_Root]). However, when navigating back again, the state gets rehydrated into[Settings_Root, Settings_Wallet]due to the logic here:App/src/libs/Navigation/AppNavigator/createSplitNavigator/SplitRouter.ts
Lines 55 to 68 in c71784d
which leads to the current issue.
Therefore, this PR ensures that
sidebarScreenis added immediately when adding the under–fullscreen router overlay. This prevents the current issue becauseSettings_Walletalready hasSettings_Rootin front, so no rehydration is needed. It also avoids a UI glitch whereSettings_Walletbriefly appears whenSettings_Rootis added and the state transitions from[Settings_Wallet]to[Settings_Wallet, Settings_Root].Fixed Issues
$ #88953
PROPOSAL:
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Prerequisite: The user is assigned a physical Expensify card and has not yet entered their personal information.
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.hybrid.mp4
Android: mWeb Chrome
android.chrome.mp4
iOS: Native
ios.hybrid.mp4
iOS: mWeb Safari
ios.safari.mp4
MacOS: Chrome / Safari
mac.safari.mp4