Fix: onboarding Track-business users land on Home instead of Inbox - #97841
Fix: onboarding Track-business users land on Home instead of Inbox#97841marcochavezf wants to merge 2 commits into
Conversation
The guard at the onboarding call sites used isReportTopmostSplitNavigator(), which only checks whether the Reports tab is topmost, not whether it actually shows a report. When a fresh onboarding user's base tab happened to be the empty Inbox, the guard treated that as a report worth preserving and skipped Navigation.navigate(ROUTES.HOME), leaving the user stranded on Inbox. Adds isReportRevealedInTopmostSplitNavigator() in src/libs/Navigation/helpers/, which also requires a SCREENS.REPORT route in the split's inner routes before treating it as revealed. Replaces the guard at both call sites in RHPVariantTest/index.ts and navigateAfterOnboarding.ts. isReportTopmostSplitNavigator is left untouched, since IOU flows and SignInModal still call it directly for unrelated checks. Fixes Expensify/Expensify#663126 Recreated from #97009 on a clean branch off main. #97009's branch had ingested unrelated, still-in-progress commits from a separate PR (#96508, Concierge optimistic reveal cadence) through repeated cross-branch merges, so this is the same change isolated from that entanglement rather than a new fix.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
(Marco's agent): @codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb4ddf44a8
ℹ️ 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".
| const innerRoutes: ReadonlyArray<{name: string}> | undefined = | ||
| topmostFullScreenRoute.state?.routes ?? (topmostFullScreenRoute.key ? getPreservedNavigatorState(topmostFullScreenRoute.key)?.routes : undefined); | ||
|
|
||
| return !!innerRoutes?.some((route) => route.name === SCREENS.REPORT); |
There was a problem hiding this comment.
When the active Reports split has already mounted on a wide layout from /inbox, the split router injects its default central SCREENS.REPORT route even when no chat/report is selected, and that placeholder can have an empty or missing reportID. This check treats that placeholder exactly like a revealed report, so the Track-business/RHP onboarding paths skip Navigation.navigate(ROUTES.HOME) and can still leave the user on the empty Inbox instead of Home; please only preserve when the Report route has a non-empty report ID (or otherwise distinguish the placeholder route).
Useful? React with 👍 / 👎.
|
(Marco's agent): @claude review |
1 participant
Explanation of Change
After a user completes onboarding with "Track expenses for my business,"
navigateAfterOnboarding()andhandleRHPVariantNavigation()can leave the user on an empty Inbox. The onboarding guards callisReportTopmostSplitNavigator(), which checks only whether the Reports tab is topmost. A topmost Reports tab can contain only the Inbox sidebar, so the guard skipsNavigation.navigate(ROUTES.HOME)even though no report is open.This PR adds
isReportRevealedInTopmostSplitNavigator(). It checks the topmost Reports split navigator's inner routes forSCREENS.REPORTand uses preserved navigator state when onboarding has removed the live state before the check. The three onboarding call sites now use this helper. Regression tests cover the RHP variants and the larger-screen onboarding path, including preserving a report when one is revealed.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/663126
PROPOSAL: https://github.com/Expensify/Expensify/issues/663126#issuecomment-5074382682
Tests
Automated coverage checks both onboarding navigation paths.
RHPVariantTest.test.tscovers the RHP variants when no report is revealed, andnavigateAfterOnboardingTest.tsadds the larger-screen case where the Reports tab contains only the Inbox while retaining coverage for a revealed report.Offline tests
N/A. This change only reads local navigation state.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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