Fix: Report quick action on Home should not redirect to Inbox#87787
Fix: Report quick action on Home should not redirect to Inbox#87787nabi-ebrahimi wants to merge 6 commits intoExpensify:mainfrom
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
melvin-bot
Bot
requested review from
ZhenjaHorbach and
flaviadefaria
and removed request for
a team
|
@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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| type GetCreateReportRouteParams = { | ||
| reportID: string; | ||
| shouldUseNarrowLayout?: boolean; |
There was a problem hiding this comment.
❌ CONSISTENCY-4 (docs)
The shouldUseNarrowLayout property is declared in GetCreateReportRouteParams but is never used in the getCreateReportRoute function. The function destructures only {reportID} on line 20, ignoring shouldUseNarrowLayout entirely.
Remove shouldUseNarrowLayout from the type definition, or use it in the function body if it was intended to affect routing logic:
type GetCreateReportRouteParams = {
reportID: string;
};Reviewed at: cb0eddc | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| groupPoliciesWithChatEnabled.length > 1) | ||
| ) { | ||
| Navigation.navigate(ROUTES.NEW_REPORT_WORKSPACE_SELECTION.getRoute()); | ||
| // Navigate to the Reports page first so workspace selection continues from |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The "navigate to Reports root, then navigate via microtask queue to workspace selection" pattern is duplicated identically in both CreateReportMenuItem.tsx and QuickCreationActionsBar/index.tsx:
Navigation.navigate(getReportsRootRoute());
Navigation.setNavigationActionToMicrotaskQueue(() => {
Navigation.navigate(ROUTES.NEW_REPORT_WORKSPACE_SELECTION.getRoute());
});Consider extracting this into a shared helper in getCreateReportRoute.ts (e.g., navigateToWorkspaceSelection()) to consolidate this duplicated navigation logic alongside the existing getCreateReportRoute and getReportsRootRoute helpers.
Reviewed at: cb0eddc | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![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: cb0eddc4b4
ℹ️ 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".
| ); | ||
| // Navigate to the Reports page first so getCreateReportRoute() resolves against | ||
| // the Search/Reports fullscreen context before opening the created report modal. | ||
| Navigation.navigate(getReportsRootRoute()); |
There was a problem hiding this comment.
handleCreateWorkspaceReport() always calls Navigation.navigate(getReportsRootRoute()) before opening the new report, but when isReportInSearch is true only the second navigation is forceReplace. Because Navigation.navigate() defaults to a push when forceReplace is omitted (src/libs/Navigation/helpers/linkTo/index.ts), this leaves the previous search/r/:reportID route in the stack and back navigation from the newly created report can return to that stale report context instead of the Reports list this change is trying to enforce.
Useful? React with 👍 / 👎.
| if (isSearchTopmostFullScreenRoute()) { | ||
| return ROUTES.SEARCH_MONEY_REQUEST_REPORT.getRoute({reportID, backTo: activeRoute}); | ||
| } | ||
|
|
||
| if (isHomeTopmostFullScreenRoute()) { | ||
| return ROUTES.SEARCH_MONEY_REQUEST_REPORT.getRoute({ | ||
| reportID, | ||
| backTo: getReportsRootRoute(), | ||
| }); | ||
| } | ||
|
|
||
| return ROUTES.REPORT_WITH_ID.getRoute(reportID, undefined, undefined, activeRoute); |
There was a problem hiding this comment.
I'm a bit confused
Wouldn't ROUTES.REPORT_WITH_ID.getRoute(reportID, undefined, undefined, activeRoute) just open the new report on the current screen?
Without navigating to the Reports tab?
There was a problem hiding this comment.
Yes, that’s correct for the fallback branch.
For the Home quick action and FAB flow, we navigate to Reports first, then call getCreateReportRoute(), so those paths resolve through the SEARCH_MONEY_REQUEST_REPORT branch. The REPORT_WITH_ID(..., activeRoute) branch is just the default fallback for unrelated callers.
| } | ||
|
|
||
| export default getCreateReportRoute; | ||
| export {getReportsRootRoute, navigateToReportsRoot, navigateToCreateReportWorkspaceSelection}; |
There was a problem hiding this comment.
Can you write tests for these new utils?
| const isHomeTopmostFullScreenRoute = (): boolean => { | ||
| const rootState = navigationRef.getRootState() as State<RootNavigatorParamList>; | ||
|
|
||
| if (!rootState) { | ||
| return false; | ||
| } | ||
|
|
||
| return rootState.routes.findLast((route) => isFullScreenName(route.name))?.name === SCREENS.HOME; | ||
| }; | ||
|
|
||
| export default isHomeTopmostFullScreenRoute; |
There was a problem hiding this comment.
No, we don’t need it anymore. I missed that after the latest navigation changes, thanks for catching it, I’ll remove it.
| ); | ||
| // Navigate to the Reports page first so getCreateReportRoute() resolves against | ||
| // the Search/Reports fullscreen context before opening the created report modal. | ||
| Navigation.navigate(getReportsRootRoute()); |
There was a problem hiding this comment.
Why don't you use navigateToReportsRoot here?
Or just remove navigateToReportsRoot from your utilities and use this format in another place
There was a problem hiding this comment.
Good point. I’ll update QuickCreationActionsBar to use the helper for consistency. Thanks
There was a problem hiding this comment.
Let's better remove navigateToReportsRoot
Because it doesn't shorten the code much 😅
|
@ZhenjaHorbach I have addressed all the feedbacks. Please take another look when you have a moment. Thanks |
What about this? |
Oh, I just missed that comment. That sounds good too, I’ll remove |
@ZhenjaHorbach addressed that as well. Please take another look when you have a chance. Thanks |
|
I suppose in this case, we also need to navigate to the Reports tab with the created report 2026-04-14.16.59.49.mov |
flaviadefaria
left a comment
flaviadefaria
left a comment
There was a problem hiding this comment.
The changes proposed in this PR look good to me.
|
@ZhenjaHorbach anything you need from me on this PR? |
@JmillsExpensify |
Yes, I agree with "we also need to navigate to the Reports tab with the created report" |
@ZhenjaHorbach I handled this case as well. Please let me know if there’s any other case remaining that I should cover. Thanks |
|
@ZhenjaHorbach gentle bump. thanks |
|
I'm not sure about this behaviour CC: @JmillsExpensify On dev 2026-04-22.12.28.43.movOn staging 2026-04-22.12.29.12.mov |
4 participants
Explanation of Change
This PR fixes the
Create reportnavigation flow so it always opens in theReportspage context instead of keeping the previously active fullscreen tab.When a report is created from Home quick actions or the FAB menu, the app now transitions into the
Reportspage first, then opens the newly created report, or the workspace-selection flow if needed, from that context. This keeps the experience consistent with the intended product behavior and prevents users from landing in the wrong navigation context.Fixed Issues
$ #87073
PROPOSAL: #87073 (comment)
Tests
Navigate to
Home.Reportbutton in the quick actions bar.Reportspage instead ofInbox.Return to
Home.Create report.Reportspage instead of staying on the previous tab or redirecting toInbox.Navigate to another tab, such as
Workspaces.Create report.Reportspage.Force the workspace-selection flow by using an account/setup that requires choosing a workspace before report creation.
Home, click theReportquick action.Reportspage and shows the workspace list in the RHP.Reportscontext.Offline tests
Same as Tests.
QA Steps
Same as Tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
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-04-14.at.1.01.15.AM.mov
Android: mWeb Chrome
Screen.Recording.2026-04-14.at.1.03.40.AM.mov
iOS: Native
Screen.Recording.2026-04-14.at.12.43.51.AM.mov
iOS: mWeb Safari
Screen.Recording.2026-04-14.at.1.06.56.AM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-04-14.at.10.11.28.AM.mov
Screen.Recording.2026-04-14.at.10.11.57.AM.mov
Screen.Recording.2026-04-14.at.10.12.31.AM.mov
Screen.Recording.2026-04-14.at.10.24.46.AM.mov