[NO QA] Reduce re-renders & remove global reports subscription in FloatingActionButtonAndPopover#83057
[NO QA] Reduce re-renders & remove global reports subscription in FloatingActionButtonAndPopover#83057
Conversation
|
@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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8a32622c2
ℹ️ 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".
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridApp2026-02-20.16.59.27.moviOS: mWeb Safari2026-02-20.16.58.31.movMacOS: Chrome / Safari2026-02-20.16.57.04.mov |
| return getWorkspaceChats(activePolicyID, [session?.accountID ?? CONST.DEFAULT_NUMBER_ID], reports); | ||
| }, | ||
| [activePolicyID, session?.accountID], | ||
| ); |
There was a problem hiding this comment.
I think we can move this selector in selectors folder
And then let's create a test for this selector!
There was a problem hiding this comment.
This method isn't used anywhere else, so I think it'd be better to keep it inline
There was a problem hiding this comment.
Actually, we use the same logic in BaseFloatingCameraButton also
There was a problem hiding this comment.
But no
Almost the same😅
There was a problem hiding this comment.
App/src/components/FloatingCameraButton/BaseFloatingCameraButton.tsx
Lines 42 to 51 in b164fa3
There was a problem hiding this comment.
Maybe we should reuse this selector in 2 places?
There was a problem hiding this comment.
I still dont think there would be a benefit to reusing this, especially since the selector relies on view-specific params. We would need to create a method inline using useCallback just to call the selector with the params as well.
There was a problem hiding this comment.
But in both places, we are trying to get policyChatForActivePolicy, but using different approaches
So why not make the approach the same in both places?
But I don't mind leaving everything as it is 😅
Explanation of Change
Removes the subscription to all reports in the FloatingActionButtonAndPopover to prevent excessive re-renders in the list
Fixed Issues
$ #83055
Tests
N/A
Offline 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/Videosundefined