refactor: extract InboxTabButton with colocated last report subs#92916
Conversation
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.
|
|
@DylanDylann 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] |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Refactors the NavigationTabBar by extracting the Inbox tab pressable (including telemetry span + wide-layout “navigate to last report” deep-link behavior) into a dedicated InboxTabButton component, with the intent of isolating Onyx/root-navigation subscriptions so the whole tab bar doesn’t re-render when last-report data changes.
Changes:
- Extract Inbox tab UI + navigation logic into new
InboxTabButtoncomponent. - Move “last report exists” / “last report action exists” Onyx selectors/subscriptions into the extracted component.
- Keep selector identity stable via a module-scope selector factory memoized with
useMemo.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/components/Navigation/NavigationTabBar/index.tsx | Removes inline Inbox tab implementation and replaces it with InboxTabButton, trimming parent-level dependencies to reduce re-renders. |
| src/components/Navigation/NavigationTabBar/InboxTabButton.tsx | New component encapsulating Inbox tab pressable, accessibility, telemetry span, and wide-layout last-report deep-link routing logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
1 similar comment
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| statusIndicatorColor: string | undefined; | ||
| chatTabBrickRoad: BrickRoad; |
There was a problem hiding this comment.
We should self-subcribe two props
There was a problem hiding this comment.
Done 👍 moved the brick road sub into the component and compute the status color locally now, so NavigationTabBar no longer re-renders when the brick road changes.
| const [doesLastReportExist] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${lastReportRouteReportID}`, {selector: doesLastReportExistSelector}, [lastReportRouteReportID]); | ||
|
|
||
| const doesLastReportActionExistSelector = makeDoesLastReportActionExistSelector(lastReportRouteReportActionID); | ||
| const [doesLastReportActionExist] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${lastReportRouteReportID}`, {selector: doesLastReportActionExistSelector}, [ |
There was a problem hiding this comment.
This should only be processed in the wide layout, so let's remove this subscription from the narrow layout. I'd suggest creating 2 components for each screen type while being mindful to avoid code duplication, especially duplicate subscriptions.
There was a problem hiding this comment.
Went with gating instead of splitting to stay consistent with SearchTabButton's single-component shape. The last-report lookups are now gated on isWideLayout, so in narrow the report/report-action subs stay inert and tapping Inbox just routes to the inbox. Happy to split into two components instead if you'd still prefer that.
There was a problem hiding this comment.
@BartekObudzinski My concern here is that in the narrow layout, we're still subscribing to doesLastReportActionExist, which is unnecessary. I'd suggest removing it.
| const lastReportRouteReportID = useRootNavigationState((rootState) => { | ||
| if (!rootState) { | ||
| return undefined; | ||
| } | ||
| const route = getLastRoute(rootState, NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, SCREENS.REPORT); | ||
| return getStringParam(route?.params, 'reportID'); | ||
| }); | ||
|
|
||
| const lastReportRouteReportActionID = useRootNavigationState((rootState) => { | ||
| if (!rootState) { | ||
| return undefined; | ||
| } | ||
| const route = getLastRoute(rootState, NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, SCREENS.REPORT); | ||
| return getStringParam(route?.params, 'reportActionID'); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Could we combine into one hook call?
There was a problem hiding this comment.
Assuming this is the two useRootNavigationState calls keeping them separate on purpose. That hook compares results with Object.is, so merging into one call that returns an object would break the comparison and re-render the button on every nav state event, which defeats the goal of the PR. Two primitive-returning selectors stay stable
There was a problem hiding this comment.
Since this PR focuses on decomposition, let's maintain the current state.
|
@BartekObudzinski Let's also remove No QA label in the PR title. I believe that we should require a test here |
BartekObudzinski
changed the title
|
@DylanDylann updated |
|
No product review needed |
JS00001
left a comment
JS00001
left a comment
There was a problem hiding this comment.
Code lgtm, lets see what @DylanDylann has to say about the one outstanding comment
|
@codex review |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-06-12.at.16.16.48.movAndroid: mWeb ChromeScreen.Recording.2026-06-12.at.16.09.25.moviOS: HybridAppScreen.Recording.2026-06-12.at.16.17.09.moviOS: mWeb SafariScreen.Recording.2026-06-12.at.16.09.09.movMacOS: Chrome / SafariScreen.Recording.2026-06-12.at.15.55.02.mov |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
🚧 @JS00001 has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.4.7-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review: no changes required. This PR is a pure internal architecture refactor — it extracts the Inbox tab pressable, There is no user-facing behavior change, no new or renamed UI text, no new settings, and no feature/flow that an end user would interact with differently. The two changed files are both internal component code:
The help site articles under If you believe a specific help article should be updated as a result of this change, let me know which behavior/article and I'll take another look. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.7-4 🚀
|
Explanation of Change
Extracts the Inbox tab pressable,
startSpantrigger, and wide-layout deep-link handling fromNavigationTabBarinto a dedicatedInboxTabButtoncomponent. The twouseOnyxsubscriptions fordoesLastReportExistanddoesLastReportActionExistare moved inside the new component (wide layout only), soNavigationTabBarno longer re-renders when these subscriptions fire.The previously inline
doesLastReportActionExistSelectoris also moved to a module-scope factory (makeDoesLastReportActionExistSelector) called viauseMemo([actionID]), so the selector identity stays stable across renders instead of being a new function each time.Behavior preserved. Small expected
ManualNavigateToInboxTabimprovement from removing the unrelated re-renders onNavigationTabBarwhen last-report data changes.Fixed Issues
$ #93041
PROPOSAL:
Tests
Offline tests
Same as Tests. No new network code touched.
QA Steps
[No QA] - architecture refactor with no user-facing behavior change.
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-10.at.15.39.20.mov
Android: mWeb Chrome
Screen.Recording.2026-06-10.at.15.39.20.mov
iOS: Native
Screen.Recording.2026-06-10.at.15.22.33.mov
iOS: mWeb Safari
Screen.Recording.2026-06-10.at.15.37.21.mov
MacOS: Chrome / Safari
Screen.Recording.2026-06-10.at.15.27.05.mov