Decompose ReportActionsList: 10 - restore useEffectEven#95478
Conversation
|
@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] |
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
|
Updated with latest main |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-07-09.at.15.13.46.movAndroid: mWeb ChromeScreen.Recording.2026-07-09.at.14.56.09.moviOS: HybridAppScreen.Recording.2026-07-09.at.15.13.07.moviOS: mWeb SafariScreen.Recording.2026-07-09.at.14.57.44.movMacOS: Chrome / SafariScreen.Recording.2026-07-08.at.18.22.04.mov |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! 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". |
|
🚧 rlinoz has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.4.33-0 🚀
|
|
🤖 No help site changes required. I reviewed this PR against the help site content under
Because there's no documentation impact, I did not create a draft docs PR. |
Explanation of Change
Converted the two mark-as-read effects in
src/hooks/useMarkAsRead.tsfrom plainuseEffect(witheslint-disable react-hooks/exhaustive-deps) touseEffectEventhandlers called from thin effects:handleReportChangeMarkAsRead()handleAppVisibilityMarkAsRead(isFocused)—isFocusedis passed as an argumentReact runs its passive effect but skips the commit phase that refreshes the Effect Event closure, so
isFocusedstays stuck at the staletrue. The guard then passed andreadNewestActionre-marked a report the user had just marked unread. Probably related to react-freeze.Passing
isFocusedas an argument uses the trigger-render value (falseon switch-away), so the guard bails correctly.isVisibleis not passed — it tracks global tab visibility, which does not diverge per-screen, so reading it from the closure is safe. (Verified on web:isVisibleArgalways equaled the closure value; onlyisFocusedwent stale.)Fixed Issues
$ #89771
$ #95103
PROPOSAL:
Tests
Test: LHN - Marking an unread message as unread does not mark the conversation as unread
Offline tests
QA Steps
Same as tests
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.mov
Android: mWeb Chrome
android.web.mov
iOS: Native
ios.mov
iOS: mWeb Safari
ios.web.mov
MacOS: Chrome / Safari
web.mov