Reduce search filters bar re-renders#82846
Conversation
- Add onBulkPaySelectedRef and keep it updated with latest onBulkPaySelected - Expose stableOnBulkPaySelected (useCallback with empty deps) that invokes ref.current - Pass stableOnBulkPaySelected to SearchPageNarrow (confirmPayment) and SearchPageWide (onBulkPaySelected) - Reduces SearchFiltersBar re-renders caused by unstable confirmPayment prop (plan step 1) Co-authored-by: Cursor <cursoragent@cursor.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.
|
|
@dominictb 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] |
|
@luacmartins please reassign |
|
I can help review |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.movbulk pay: Screen.Recording.2026-02-19.at.11.39.16.PM.mov |
|
Please fix conflict (came from #81659) |
|
PR looks good :) |
|
No product review needed |
|
Updated |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ 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". |
| const isOptions = Object.values(CONST.SEARCH.IS_VALUES).map((value) => ({text: translate(`common.${value}`), value})); | ||
| const is = isFilterValues ? isOptions.filter((option) => isFilterValues.includes(option.value)) : []; | ||
|
|
||
| const createDateDisplayValue = (filterValues: {on?: string; after?: string; before?: string}): [SearchDateValues, string[]] => { |
There was a problem hiding this comment.
createDateDisplayValue is a pure function. Could it move outside the component like DatePickerFilterPopup and MultiSelectFilterPopup?
There was a problem hiding this comment.
Yea, we can. I'll do that in a follow up. Thanks!
There was a problem hiding this comment.
Hmm actually it's not pure since it takes translate from the hook. I'll keep it in for now
|
🚧 @luacmartins 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/luacmartins in version: 9.3.24-1 🚀
|
|
Hi @luacmartins. Is it an internal QA? |
|
No QA |
|
@luacmartins I can check it off then, right? |
|
@IuliiaHerets yes, I already did |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.24-3 🚀
|
Explanation of Change
This PR does a few things:
confirmPaymentfunction to avoid re-renders of theSearchFiltersBarcomponent.DatePickerFilterPopupto allow React compiler to auto memoizeSearchFiltersBaruseMemoanduseCallbacksince the component is memoized by React compilerDatePickerFilterPopupoutsideSearchFiltersBarFixed Issues
$ #82419
Tests
applausetester@applause.expensifail.comInboxandReportspageManualNavigateToReportsTabspan in SentryOffline tests
N/A
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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