Decompose ReportActionsList: 10 - #94149
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42e31234cd
ℹ️ 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".
42e3123 to
a4b5f43
Compare
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.
|
a4b5f43 to
56b3f3e
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e9d3d5a74
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ef776c33f
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51e9c89671
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ 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". |
|
@DylanDylann 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] |
27e998a to
90fba6d
Compare
|
Dead code left behind after deleting ListBoundaryLoader
|
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ 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". |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-06-30.at.15.30.24.movAndroid: mWeb ChromeScreen.Recording.2026-06-30.at.15.09.23.moviOS: HybridAppScreen.Recording.2026-06-30.at.15.27.55.moviOS: mWeb SafariScreen.Recording.2026-06-30.at.15.08.40.movMacOS: Chrome / SafariScreen.Recording.2026-06-30.at.15.06.28.mov |
|
@LukasMod One NIT: #94149 (comment). The rest looks good to me |
|
@DylanDylann Thanks, I will clean it soon and pull main again, I hope test units will be resolved soon |
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Removed ✅ |
|
The failed test looks like a flaky test |
rlinoz
left a comment
There was a problem hiding this comment.
PR looks good!
Related question, did we at some point had a skeleton at the top of the loaded messages when opening a report? I feel like we broke it along the way but not 100% sure.
Screen.Recording.2026-06-30.at.09.13.37.mov
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 rlinoz 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! 🧪🧪
|
I see the same behavior in production, so this could be an older bug, or it might be working as intended. There was definitely no intention to change the skeleton behavior as part of the decomposition plan, so it should work the same as before with every PR we shipped as part of this plan |
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.4.26-0 🚀
|
|
🤖 No help site changes required. I reviewed this PR and no updates to Expensify's help site files under Why: This is PR 10 of the
There are no new features, settings, UI labels, buttons, or user-facing copy introduced — and no existing help articles reference any of the removed strings or components (verified against @LukasMod, since no help site changes are required, no draft PR was created. If you believe a user-facing behavior here does warrant a docs update, let me know and I'll open one. |
|
Deploy Blocker #95103 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.27-2 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
PR 10 of the ReportActionsList decomposition series (tracker #88320, issue #89771). Behavior-preserving cleanup: drop every
react-hooks/*eslint-disable from the extracted pipeline hooks and simplify the header.What changed:
useMarkAsRead.ts- dropped 2react-hooks/exhaustive-depsdisables by moving the report-change and app-visibility bodies intouseEffectEventhandlers, each called from a thin effect keyed to its original trigger deps.useReportActionsScroll.ts- dropped 2 disables viauseEffectEvent: the initial scroll-to-bottom is now a mount-once effect, and the IOU-error scroll keeps its original[lastAction]trigger +usePreviousdedup wrapped in auseEffectEvent(so the latest error state is read without becoming a trigger), plus a!lastIOUActionWithErrorguard so clearing/resolving an error no longer scrolls.ReportActionsList.tsx- removed thecanShowHeader/hasHeaderRenderedone-render header gate, its cross-report reset effect, and the<View>spacer; the header now always renders. Removed now-unusedView/useRefimports.ReportActionsListHeader.tsx- simplified to a self-gating component: returnsnullwhen a Concierge draft is active, otherwise rendersConciergeThinkingMessage(now keyed byreportID). Dropped the deadListBoundaryLoaderbranch, theonRetryprop, and thereportOnyx subscription. Never theisLoadingNewerReportActions/hasErrorprops that gate its render branches, so it has returnedundefinedon every render since fix: infinite loaders in report section #56819 (2025-02-12) and still does onorigin/main. No footer consumer. Removing it changes nothing user-visible.Out of scope / unchanged: hook signatures stay frozen (tests pin them);
useUnreadMarker/useReportActionsPagination/useTransactionThreadwere audited but left untouched (noreact-hooks/*disables, already compiler-clean).Why: removes legacy effect patterns and the header gate so the pipeline hooks pass the React Compiler gate cleanly with zero eslint-disables. Dropping the header gate also removed a render-time ref read, taking
ReportActionsListfrom 2 to 1react-hooks/refsseatbelt errors.Fixed Issues
$ #89771
PROPOSAL:
Tests
Test 1: Header + top-pinning (visual)
Test 2: IOU error, fresh error scrolls to bottom
Goal: a newly failed action on the newest message must scroll the list to the bottom so the user sees it.
Steps:
Test 3: Concierge thinking (smoke test)
Test 4: Rapid report switching
Prereq: reports A and B both with unread content.
Offline tests
QA Steps
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
Screen.Recording.2026-06-26.at.12.28.27.mov
Android: mWeb Chrome
Screen.Recording.2026-06-26.at.12.31.01.mov
iOS: Native
Screen.Recording.2026-06-26.at.12.02.25.mov
iOS: mWeb Safari
Screen.Recording.2026-06-26.at.12.04.55.mov
MacOS: Chrome / Safari
Screen.Recording.2026-06-26.at.11.52.05.mov
Screen.Recording.2026-06-26.at.11.46.03.mov