perf: decompose ReportScreen into child components#84971
Draft
adhorodyski wants to merge 40 commits intoExpensify:mainfrom
Draft
perf: decompose ReportScreen into child components#84971adhorodyski wants to merge 40 commits intoExpensify:mainfrom
adhorodyski wants to merge 40 commits intoExpensify:mainfrom
Conversation
Create IsInSidePanelContext + useIsInSidePanel hook. SidePanelReport wraps children with the context provider. All consumers (ReportScreen, HeaderView, ReportFooter, ReportActionCompose) now call the hook directly instead of receiving the prop through multiple layers of components. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the useFocusEffect that resolves reportID and validates reportActionID from ReportScreen into a dedicated renderless component. This removes useArchivedReportsIdSet and usePermissions subscriptions from ReportScreen. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move emoji picker hiding on blur, notification clearing, telemetry span cancellation, and DeviceEventEmitter listener from ReportScreen into a dedicated renderless component. Also removes the dead isSkippingOpenReport ref. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create ReportHeader as a self-subscribing building block that owns: - Report type subscription for variant selection - onBackButtonPress derivation - OfflineWithFeedback wrapper - MoneyRequestHeader | MoneyReportHeader | HeaderView rendering This removes the headerView useMemo (ground rule violation: JSX in variable) and onBackButtonPress callback from ReportScreen. Also removes MoneyReportHeader, MoneyRequestHeader, HeaderView, OfflineWithFeedback, and useSidePanelActions imports from ReportScreen. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move all report fetching, transaction thread creation, leaving events, updateLastVisitTime, compose input init, and thread rejoin effects from ReportScreen into a self-subscribing renderless component. This removes 12 effects and 6 useOnyx subscriptions from ReportScreen (introSelected, onboarding, isLoadingReportData, isLoadingApp, transactionThreadReport, chatReport via the controller's own subs). Also removes unused refs (hasCreatedLegacyThreadRef, didSubscribeToReportLeavingEvents, prevIsAnonymousUser), isLinkedMessagePageReady chain, and many unused imports. ReportScreen: 952 → 704 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create WideRHPReceiptPanel as a self-subscribing building block that owns shouldShowWideRHP derivation, useShowWideRHPVersion call, and the receipt view rendering. This removes the WideRHP logic, transactionThreadReport subscription, and isSmallScreenWidth from ReportScreen. Also removes Animated, ScrollView, MoneyRequestReceiptView, useShowWideRHPVersion, and isTransactionThread imports from ReportScreen. ReportScreen: 704 → 671 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create ReportActionsList as a self-subscribing building block that owns reportActions, transactions, metadata, and view switching (skeleton | ReportActionsView | MoneyRequestReportActionsList). Create ReportComposer that owns lastReportAction, transactionThreadReportID, and wraps ReportFooter. This removes the data pass-throughs to ReportActionsView, MoneyRequestReportActionsList, and ReportFooter from ReportScreen. Also removes policy, chatReport, transactionThreadReportActions, and ~10 derived values from ReportScreen. ReportScreen: 671 → 541 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create ReportNotFoundGuard as a self-subscribing building block that owns all not-found page logic, linked action detection, deletion navigation, report removal navigation, and the FullPageNotFoundView + DragAndDropProvider wrappers. ReportScreen is now a pure JSX composition shell with 0 useOnyx, 0 useEffect, 0 useState, 0 useMemo, 0 useCallback. ReportScreen: 541 → 93 lines (92% reduction from original 1123). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both header components now accept reportID instead of report/policy/ reportActions/parentReportAction props. They subscribe to their own data internally via useOnyx, useParentReportAction, and usePaginatedReportActions. ReportHeader is slimmed to only pass reportID + onBackButtonPress. MoneyRequestReportView.tsx updated to pass new props. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HeaderView now accepts only reportID + onNavigationMenuButtonClicked. It subscribes to report and parentReportAction internally. ReportHeader is slimmed further — no longer needs useParentReportAction or shouldUseNarrowLayout. Tests updated to new prop interface. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ReportComposer now renders the footer UI directly (archived footer, anonymous footer, blocked footer, compose area) instead of delegating to a separate ReportFooter component. This eliminates the data- intermediary wrapper that only existed to pass props. MoneyRequestReportView updated to use ReportComposer directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The component renders archived footers, anonymous footers, blocked footers, banners, AND the composer — 'ReportFooter' better describes its full scope. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the interleaved boolean conditionals
({!isArchivedRoom && !isAnonymousUser && !isBlockedFromChat && ...})
with structured early returns for each mutually exclusive state.
Each rendering path (compose, archived, anonymous, blocked, system
chat, admins-only) is now self-contained with its own return. No more
manual boolean exclusion chains. Easy to scan, modify, and extend.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move lastReportAction derivation, onSubmitComment/handleCreateTask, concierge status indicator (useAgentZeroStatusIndicator), and all supporting hooks into ReportActionCompose. This eliminates ~14 subscriptions from ReportFooter including 6 collection-level ones from useAncestors that fired on every app event. ReportFooter now only owns footer variant selection (archived, anonymous, blocked, system chat, admins-only) and passes just reportID + isComposerFullSize + didHideComposerInput to ReportActionCompose. ReportActionCompose accepts only reportID, isComposerFullSize, didHideComposerInput, and optional composer focus/blur callbacks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ArchivedReportFooter now accepts reportID, subscribes to report and currentUserAccountID internally. AnonymousReportFooter now accepts reportID, subscribes to report and derives isSmallSizeLayout internally. ReportActionCompose now owns isComposerFullSize, didHideComposerInput, and the chatFooterStyles computation — accepts only reportID. ReportFooter is now a pure view-switcher (136 lines) with 0 data pass-throughs. It passes only reportID to each variant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Notification clearing and telemetry span cancellation only make sense when the report exists. Previously they fired with undefined reportID on the not-found page. Now they only run when the guard renders its children (report found). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eportActionsView Move useAgentZeroStatusIndicator, useParentReportAction, concierge side panel logic, hasUserSentMessage, sessionStartTime, and isReportTransactionThread from ReportActionsList into ReportActionsView where they're actually consumed. ReportActionsList drops from 135 to 106 lines and sheds 6 hooks (useIsInSidePanel, useCurrentUserPersonalDetails, useParentReportAction, useSidePanelState, useAgentZeroStatusIndicator, conciergeReportID sub). It's now a thin view-switcher: skeleton | table | list. Also inlines hasUserSentMessage (was an IIFE, now a plain ternary). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace boolean exclusion chains ({!isTransactionThreadView &&
isMoneyRequestOrInvoiceReport && ...}) with structured early returns.
Each header variant (MoneyRequestHeader, MoneyReportHeader, HeaderView)
is now a self-contained return with its own OfflineWithFeedback wrapper.
No impossible states.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
adhorodyski
changed the title
…-plan # Conflicts: # src/components/MoneyReportHeader.tsx # src/pages/inbox/ReportScreen.tsx
Move all linked-action not-found logic (VISIBLE_REPORT_ACTIONS subscription, usePaginatedReportActions, linking state, whisper checks, and 4 related effects) into a child LinkedActionNotFoundGuard component. This scopes high-frequency re-renders from these subscriptions to just the child instead of the entire report view tree (~50+ components). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move navigation-on-removal and navigation-on-deletion effects into a sibling ReportNavigateAwayHandler component. This removes 3 exclusive Onyx subscriptions (introSelected, conciergeReportID, useReportWasDeleted) and 2 large effects from the guard, leaving it focused purely on not-found rendering. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Relocate the task report readNewestAction effect from ReportNotFoundGuard to ReportFetchController where other fetch/init concerns already live. Guard now has 7 Onyx subscriptions, 1 effect, ~120 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ldShow prop Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…liance Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract ReportDragAndDropProvider as a self-subscribing wrapper so ReportNotFoundGuard becomes a pure guard and ReportScreen's composition tree is fully transparent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Split into a cheap outer guard that only reads the route, and an inner gate that holds all Onyx subscriptions. In the common case (no reportActionID in the route), ~8-9 Onyx subs and all effects are skipped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-plan # Conflicts: # src/components/MoneyReportHeader.tsx
src/components/MoneyRequestReportView/MoneyRequestReportView.tsx
Outdated
Show resolved
Hide resolved
src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx
Outdated
Show resolved
Hide resolved
src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx
Outdated
Show resolved
Hide resolved
…ltering, inline header JSX - Remove useCallback in ReportLifecycleHandler and ReportRouteParamHandler (React Compiler handles memoization) - Short-circuit transaction filtering when offline and combine filter+map into single pass in ReportActionCompose - Remove useMemo from reportHeaderView in MoneyRequestReportView, extract shared onBackButtonPress, inline header JSX Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@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: 38705c4c99
ℹ️ 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".
ReportActionsView now derives isConciergeSidePanel/hasUserSentMessage/sessionStartTime from context instead of props. Wrap test renders in IsInSidePanelContext and SidePanelStateContext providers, update mock to use sortedVisibleReportActions, and remove stale prop usage. HeaderView now self-subscribes to report via useOnyx, so seed report into Onyx before render and add evictableKeys to prevent canEvict crash. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The SwipeableView wrapper with onSwipeDown={Keyboard.dismiss} was
accidentally dropped during the ReportFooter refactor, removing the
swipe-down-to-dismiss-keyboard gesture on mobile.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4dfcae44b
ℹ️ 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".
src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx
Outdated
Show resolved
Hide resolved
Replace the per-instance useAgentZeroStatusIndicator hook with a single AgentZeroStatusProvider context. This fixes the Codex review item where the composer's kickoffWaitingIndicator() only set local state that ReportActionsView never saw. Key changes: - AgentZeroStatusProvider with cheap guard/gate pattern: non-Concierge reports short-circuit with zero subscriptions - Pusher subscription owned directly by the provider (removed subscribeToReportReasoningEvents/unsubscribeFromReportReasoningChannel action functions and ConciergeReasoningStore module) - ConciergeThinkingMessage self-subscribes to context (removed 3 prop pass-throughs from ReportActionsView → ReportActionsList) - isConciergeChat derived from reportID === conciergeReportID (no report object subscription needed) - Server label extracted into agentZeroProcessingIndicatorSelector in src/selectors/ReportNameValuePairs.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This reverts commit 7195bf4.
Resolve 2 conflicts: - ReportScreen.tsx: keep our thin shell, apply betas param in ReportFetchController - MoneyReportHeader.tsx: merge upstream imports (handleUnvalidatedAccount, sortPoliciesByName, hasRequestFromCurrentAccount) with our self-subscribing imports Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@codex review |
Contributor
Author
|
The new results should be even better, I was able to short-circuit all concierge-chat-only logic on non-concierge chats. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09ee10c3bf
ℹ️ 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".
Comment on lines
+115
to
+118
| Pusher.subscribe(channelName, Pusher.TYPE.CONCIERGE_REASONING, (data: Record<string, unknown>) => { | ||
| const eventData = data as {reasoning: string; agentZeroRequestID: string; loopCount: number}; | ||
| addReasoning(eventData); | ||
| }).catch((error: unknown) => { |
There was a problem hiding this comment.
This gate subscribes to Pusher.TYPE.CONCIERGE_REASONING every time it mounts, but cleanup later calls Pusher.unsubscribe(channel, event), which unbinds that event at the channel level; if the same concierge report is open in multiple mounted ReportScreen instances (for example split-pane/side-panel or modal variants), unmounting one instance removes the listener for the others, and the still-visible screen stops receiving reasoning/status updates. The previous flow avoided this with per-report subscription tracking, so this needs equivalent shared ref-counting before unbinding.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
valid, this only requires small tweaks on the pusher wrapper to enable multiple callbacks on the same channel
Contributor
Author
There was a problem hiding this comment.
addressed, definitely will need to be split down
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.
|
…bind The Pusher wrapper's unsubscribe() removed ALL callbacks for an event, breaking concurrent listeners (e.g. concierge chat open in main pane + side panel). subscribe() now returns a PusherSubscription handle with an unsubscribe() method that removes only that specific callback, with full cascading cleanup (event → channel) when the last listener unmounts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
This PR continues a series of refactors aimed at simplifying
ReportScreenby pushing Onyx subscriptions and business logic down into the child components that actually need them, rather than fetching data at the top level and passing it through props.Each commit in this series targets a specific concern:
ReportFooterintoReportActionCompose— props that were only needed by the compose area are now subscribed to directly insideReportActionCompose, removing them fromReportFooter's interface.ReportFooterare eliminated; each child subscribes to the data it needs.ReportLifecycleHandlerinsideReportNotFoundGuard— the lifecycle handler only makes sense when a report exists, so it's now rendered as a child of the not-found guard rather than alongside it.ReportActionsListintoReportActionsView— concierge and parent action logic that lived inReportActionsListmoves intoReportActionsView, which is the appropriate owner.ReportHeaderrendering with early returns — the header component is refactored to use early returns, making the rendering logic easier to follow and removing conditional complexity from the caller.The end result is a leaner
ReportScreenwith a narrower prop surface, and child components that are more self-contained and independently readable.⭐ This results in a ~5% net performance gain on all React tasks as well as code quality improvements, unlocking further refactors of core bottlenecks on this screen.
Sending a message (1st is baseline)
Open a report (1st is baseline)
Fixed Issues
$ #84895
PROPOSAL:
Tests
Offline tests
N/A
QA Steps
Same as tests
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari