fix: The LHN within the Spend tab is not remembered when navigating to Inbox#93396
fix: The LHN within the Spend tab is not remembered when navigating to Inbox#93396nkdengineer wants to merge 1 commit into
Conversation
|
@mkhutornyi 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: ddfbf2ceaf
ℹ️ 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".
| let rootTabRoute = rootState.routes.findLast((route) => { | ||
| if (route.name !== NAVIGATORS.TAB_NAVIGATOR) { | ||
| return false; | ||
| } | ||
| const tabState = getTabState(route); | ||
| return tabState?.routes?.[tabState.index ?? 0]?.name === navigator; |
There was a problem hiding this comment.
Don't restore reports from an older tab stack
When an older TAB_NAVIGATOR with Inbox/Reports active exists below the current tab stack, this findLast selects that older stack before considering the topmost stack's inactive Reports state. For example on wide layout, if a user opens report A, goes to Spend, opens report B from Spend (pushing a newer Reports tab stack), then switches to Home and presses Inbox, this predicate matches the older report-A stack and navigateToChats restores A instead of the report B state the user just left in the topmost tab navigator. The active-tab preference needs to avoid overriding a newer tab navigator that already has the target navigator state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think it's not a valid case. If the Reports tab here means the report split navigator, it will be the focused route in tabState then we can find them as the last. If Reports tab means the full-screen search navigator, navigateToChats restores the report A is correct.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
ReviewThe change correctly implements the approved proposal and fixes the reported Spend-LHN bug: it prefers the The Codex P2 concern is legitimate, though —
Suggested alternative: select by what we actually want — preserved state — rather than using active-tab as a proxy. Why the active-tab heuristic can pick the wrong stackThe bot's scenario: open report A → Spend → open report B from Spend (newer tab stack) → Home → press Inbox. The newest
One open question: it's not fully confirmed that this exact sequence actually produces two stacked This review is advisory — I didn't open this PR and I'm not an assignee, so I haven't pushed any changes. Reply with |
mkhutornyi
left a comment
There was a problem hiding this comment.
Please check potential regressions above.
Regression scanThe fix works — I verified Flow 1 in a wide-layout browser: select "Reports" in the Spend LHN → switch to Inbox → switch back, and the Spend tab correctly restores "Reports" (would previously reset to the default "Expenses"). The simple Inbox round-trip is not polluted by Spend state. But because this change alters which 1. Stale report restored by 2. No unit tests were added for Lower-severity / notes
Advisory only — I didn't open this PR and I'm not an assignee, so I haven't pushed changes. Reply with |
trjExpensify
left a comment
There was a problem hiding this comment.
You sure this is the right RCA? It's not all the tabs, just the Todos.
2026-06-12_18-55-50.mp4
|
@trjExpensify also |
|
@trjExpensify For the tab that doesn't use live data, we save the param to Onyx here App/src/hooks/useSearchPageSetup.ts Lines 56 to 70 in 1765e75 Then, after that for other tabs, we can't find App/src/components/Navigation/NavigationTabBar/SearchTabButton.tsx Lines 40 to 75 in 1765e75 |
|
CC'ing @JS00001 for vis on this. |
Explanation of Change
fix: The LHN within the Spend tab is not remembered when navigating to Inbox
Fixed Issues
$ #92460
PROPOSAL: #92460 (comment)
Tests
Offline tests
QA Steps
Same as test
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
Screen.Recording.2026-06-12.at.15.28.16.mov
Android: mWeb Chrome
Screen.Recording.2026-06-12.at.15.24.25.mov
iOS: Native
Screen.Recording.2026-06-12.at.15.25.00.mov
iOS: mWeb Safari
Screen.Recording.2026-06-12.at.15.26.02.mov
MacOS: Chrome / Safari
Screen.Recording.2026-06-12.at.15.21.32.mov