Repository navigation
Conversation
…ers.ts Move mapTransactionItemToSelectedEntry, mapEmptyReportToSelectedEntry and prepareTransactionsList out of Search/index.tsx and deriveSelectedReports out of SearchSelectionProvider.tsx into a shared selectionBuilders.ts, deduping deriveSelectedReports. Pure relocation, no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a dedicated SearchRowSelectionActionsContext carrying stable-identity toggle/toggleAll/clearAll/reconcileSelection plus a screenContextRef populated by the upcoming SearchSelectionController. The write actions read screen-derived inputs from the ref and the latest committed selection from selectedTransactionsRef at call time, so they keep a stable identity and never close over high-churn selection state. Coordination state (isRefreshingSelection, the select-all check) lives here too. Kept on its own context rather than the shared SearchSelectionActionsValue to avoid leaking an internal ref into action payload types and churning test fixtures; the stability/separation intent is preserved. Inert until wired: <Search> still uses its own handlers. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Introduce SearchSelectionController, a renderless child of <Search> that owns the selection-only Onyx subscriptions (OUTSTANDING_REPORTS_BY_POLICY_ID, selfDM, env, focus) and composes named hooks — useReconcileSelectionWithData, useTurnOffSelectionModeWhenEmpty, useSyncMobileSelectionModeWithScreenSize and useSyncSelectedReports — plus publishes the screen-derived inputs the provider's write actions read at call time. Returns null. Mounted in the main list branch, where selectable rows exist; selection state lives in the root provider and survives mount/unmount, and the reconcile effect re-syncs on remount. <Search> still runs its own copies of these effects in this commit (idempotent: reconcile is deepEqual-guarded, mobile-mode sync is a no-op when already settled); the next commit removes them. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…roller <Search> no longer reads selectedTransactions or owns the selection write logic. Deleted from it: the selectedTransactions/areAllMatchingItemsSelected context read, the ~190-line refresh-selection effect, both mobile-selection-mode effects, the isRefreshingSelection ref + reset, updateSelectAllMatchingItemsState, toggleTransaction, toggleAllTransactions, the useSyncSelectedReports call, and the now-orphaned outstandingReportsByPolicyID/selfDMReport/isProduction subscriptions. onSelectRowInMobileSelectionMode now calls the stable toggle from useRowSelectionActions(). SearchList sources toggle/toggleAll from useRowSelectionActions() (passing toggle to rows via the existing onSelectionButtonPress) and reads selectedTransactions from context for its visible select-all count, so onCheckboxPress/onAllCheckboxPress/ selectedTransactions props are dropped from its public API. Updated the render-count test accordingly. This is the change that moves the metric: a checkbox press no longer re-renders <Search> or recreates its handlers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rows self-hydrate their selected state via useRowSelection(keyForList) (read side, already shipped), so the row re-renders on its own context subscription when selection changes. The selectedTransactions entry in BaseSearchList's FlashList extraData is therefore redundant and only forced extra row diffing — remove it, along with the now-unused prop on BaseSearchList and the prop pass from SearchList. SearchList keeps reading selectedTransactions from context for its visible select-all count. Isolated commit: it is the one piece with an independent row re-render surface, so it reverts cleanly if a platform regresses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
|
- Extract the repeated TransactionListItemType[] | ... row-data union into a single exported SearchData type in types.ts; reuse it across the controller, provider, and selectionBuilders. - Drop the unused SearchRowSelectionActionsContext re-export from SearchContext.tsx (knip). - Reword the SearchSelectionController doc comment to drop "Renderless" (cspell). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ 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". |
…riteside # Conflicts: # src/components/Search/index.tsx
|
@mananjadhav 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] |
![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: 4af4f444b1
ℹ️ 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".
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
PR doesn’t need product input as a performance PR. Unassigning and unsubscribing myself. |
…read Replace the screenContextRef mutable channel with a functional applySelection setter and a screen-level SearchWriteActionsProvider; fold in the renderless SearchSelectionController and de-dup the reconcile build onto selectionBuilders. Isolate SearchPage's selectedTransactions read into a self-subscribing SearchSelectionFooter leaf so a checkbox press no longer cascades through SearchPage -> SearchPageNarrow/Wide into <Search>. Profiler: <Search> now renders in 0 press commits (was every press). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…riteside Resolve conflicts in src/components/Search/index.tsx: the write-side selection functions removed by this branch conflicted with upstream's flatten-params + archivedReportsIDSet changes. Kept our deletions and ported upstream's archivedReportsIDSet (threaded into canChangeReport) plus the object-param signature into the extracted selectionBuilders.ts and SearchWriteActionsProvider.tsx. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No product review needed |
|
Should be good now @ZhenjaHorbach |
|
resolving conflicts 👀 |
…on-writeside # Conflicts: # src/components/Search/SearchList/BaseSearchList/index.tsx # src/components/Search/SearchList/BaseSearchList/types.ts # src/components/Search/SearchList/index.tsx # src/components/Search/index.tsx
|
I'm checking 😅 |
|
Yeah sure, just kindly bumping ❤️ |
|
LGTM |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.10-0 🚀
Bundle Size Analysis (Sentry): |
Help site review — no changes requiredI reviewed the changes in this PR against the help site docs under Why: This PR is a purely internal refactor of the Search selection write-path. It moves selection logic off the
Since help site articles document user-facing behavior, and this PR changes only the internal component architecture (the same selection, footer totals, "select all", and bulk actions behave identically), there is nothing to document or update. Because no changes are required, I did not create a draft docs PR. @adhorodyski, please confirm whether you agree no help site changes are needed. If you believe a user-facing behavior did change and warrants documentation, reply with |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.12-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 I reviewed this PR against the help site articles in This is a pure internal refactor of the Search component's selection write-path — it moves selection state/actions off
Since the help site documents user-facing behavior and product features — none of which change here — there is nothing to update under @adhorodyski, please confirm this assessment. If you believe any user-facing behavior did change and warrants a help site update, reply with the details and I'll create the draft PR. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.14-0 🚀
Bundle Size Analysis (Sentry): |
|
@adhorodyski This PR is failing in step 6 because of a regression issue #93911 
The issue is reproducible in: Android Bug7184107_1781737576612.1.mp4 |
Oh 2026-06-18.08.04.19.mov |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.14-1 🚀
|
Explanation of Change
<Search>previously owned the entire selection write-path inline: toggling individual rows, toggling groups, toggling all, reconciling stale keys when the result set changed, wiringselectedTransactionsintoFlashList'sextraData, and driving mobile selection mode. All of that lived inside one ~800-line component that also handled data-fetching, routing, and rendering — so every checkbox press re-rendered the whole screen subtree (SearchPage→<Search>→ the list).This PR moves the write-side out of
<Search>entirely and splits selection into single-responsibility units, each on its own context so a checkbox press only re-renders what genuinely depends on the new selection.New / changed units
selectionBuilders.ts(new) — pure, side-effect-free functions that take the current selection map and return the next one:mapTransactionItemToSelectedEntry,mapEmptyReportToSelectedEntry,prepareTransactionsList, andderiveSelectedReports. Easy to unit-test in isolation.SearchSelectionProvider(now state-only) — owns selection state and the pure setters. AddsapplySelection: a functional read-modify-write commit ((prev) => next) so callers mutate the selection without subscribing to it (and therefore without re-rendering on every change). PassingdataderivesselectedReportsin the same commit; passingtotalSelectableItemsCountkeeps "select all matching" in lock-step.SearchWriteActionsProvider(new) — the screen-level owner of the write path. Exposestoggle/toggleAllon a dedicatedSearchRowSelectionActionsContext, committing exclusively throughapplySelectionso dispatching an action re-renders neither the provider's stable children nor the rows. It also co-locates the effects that respond to data: the reconcile-on-data-change effect (rebuilds the selection when an Onyx push adds/removes rows, with the deep-equal bail-out that prevents the [Due for payment 2026-05-26] [$250] Spend - The app crashes after selecting a group on the search page. #89588 infinite-update loop) and the mobile-selection-mode sync effects.SearchSelectionFooter(new) — a self-subscribing footer leaf. It owns theselectedTransactionsread and the footer-totals computation that used to live inSearchPage, so toggling a checkbox re-renders only this footer rather than the page and the list it contains.Wiring / read-side isolation
SearchListno longer receivesonCheckboxPress,onAllCheckboxPress, orselectedTransactionsas props — it pullstoggle/toggleAllfromuseSearchRowSelectionActions()andselectedTransactionsfrom context directly.SearchPagedrops itsuseSearchSelectionContext()read and thefooterData/shouldShowFooterderivation (moved intoSearchSelectionFooter), so a selection change no longer re-renders the page.SearchContext,SearchContextDefinitions,types.ts): addsSearchRowSelectionActionsContext+ theuseSearchRowSelectionActionshook, theapplySelectionaction, and theSearchData/SearchRowSelectionActionsValuetypes.extraDatacleanupselectedTransactionsis removed fromFlashList'sextraData(and fromBaseSearchList's props). Rows already re-render off item-reference identity — Onyx delivers a new object for a row on selection change — so the extra full-list double-render thatextraDataforced on every toggle is gone.Net effect
<Search>drops its entire selection machinery (~600 lines net) and becomes a thin router/shell that mountsSearchWriteActionsProvideraround the list. The selection logic is testable, isolated, and a checkbox press now re-renders only the affected row(s) and the footer — not the page, not the full list. No user-facing behavior changes.Fixed Issues
$ #93677
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