Repository navigation
[No QA][LegendList 6/12] refactor: split the report actions list into focused modules - #105473
Conversation
Move ReportActionsList into a directory and extract row preparation, scroll wiring and telemetry into hooks. The list keeps the inverted FlashList and its current behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@roryabraham feel free to merge this right away, since it's just a refactor. |
|
PR doesn’t need product input as a refactor PR. Unassigning and unsubscribing myself. |
|
@MelvinBot review |
|
I found no behavior changes. The split is a faithful move of the old There are two optional nits:
What I checked
|
roryabraham
left a comment
There was a problem hiding this comment.
Ok, I'm digging in and might come to you with a suggestion for a broader simplification of the ReportActionsList data model, but for now since this PR just moves existing logic into more focused models, I think we can ship it.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 roryabraham 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! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.5.27-0 🚀
|
|
No help site update needed. This PR moves the report actions list code into separate modules and doesn't change behavior, so no help article sentence is now wrong. |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.5.27-2 🚀
Bundle Size Analysis (Sentry): |
ReportActionsList.tsxis a 729-line component that mixes row preparation, scroll handling, telemetry and rendering. The LegendList migration in #100733 changes most of it, so that diff shows the whole file as deleted and rewritten across new modules. This PR does the module split first, on the current FlashList list and without behavior changes, so the migration diff only shows what LegendList changes.@roryabraham @dmkt9
Explanation of Change
This is layer 6 of the twelve-part LegendList rollout. It moves
ReportActionsList.tsxinto aReportActionsListdirectory and splits it along the same lines as the migration:The code moves as it is. The list still uses the inverted FlashList, and existing imports of
@pages/inbox/report/ReportActionsListresolve toindex.tsx.Fixed Issues
$ #98994
This PR is part of the migration split from #98270. No approved proposal is linked.
Tests
All platforms means desktop web, mobile web, Android, and iOS.
Use a second test account in the same chat for the incoming-message case.
All platforms, history and incoming messages
All platforms, a link to an older message
All platforms, editing an older message
All platforms, collapsed updates in an expense report
All platforms, Concierge reply
Offline tests
All platforms, cached chats offline and history after reconnecting
QA Steps
None. This PR moves code without changing behavior.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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.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
Cold chat open and initial loading
FlashList is on the left and LegendList is on the right. FlashList leaves the report body blank during loading; LegendList shows the report-action skeleton before messages appear.
ColdOpenLoading.mp4
Cold expense-chat open
FlashList is on the left and LegendList is on the right. Both progress from the loading skeleton to an expense preview and its surrounding report actions.
Android screen recording TBD.
Warm expense-chat open
FlashList is on the left and LegendList is on the right. Both open the cached expense preview and report actions from the inbox.
Android screen recording TBD.
Warm chat open
Side-by-side opens of the same cached chat, ending on the same report-action history. The implementations are not labeled in this recording.
Android screen recording TBD.
Android: mWeb Chrome
iOS: Native
Cold chat open and initial loading
FlashList is on the left and LegendList is on the right. FlashList leaves the report body blank during loading; LegendList shows the report-action skeleton before messages appear.
LegendListLoadingSkeletonVsFlashList.mp4
Cold expense-chat open
FlashList is on the left and LegendList is on the right. Both progress from the loading skeleton to an expense preview and its surrounding report actions.
ExpenseChatCold.mp4
Warm expense-chat open
FlashList is on the left and LegendList is on the right. Both open the cached expense preview and report actions from the inbox.
ExpenseChatWarm.mp4
Warm chat open
Side-by-side opens of the same cached chat, ending on the same report-action history. The implementations are not labeled in this recording.
LegendListWarmReportOpen.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari