Migrate InteractionManager - batch 6#91715
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
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.
|
91dda4f to
489c0b0
Compare
|
@ikevin127 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f381c56c3
ℹ️ 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".
heyjennahay
left a comment
There was a problem hiding this comment.
Product review not required
Reviewer Checklist
Screenshots/VideosAndroid: mWeb Chrome screen-20260601-173009.mp4MacOS: Chrome / Safari Screen.Recording.2026-06-01.at.17.33.49.mov |
|
@mrejdak 🔴 CRITICAL ISSUE — Missing The branch is missing the Concrete verification: git show HEAD:src/pages/inbox/report/ReportActionsList.tsx | grep -c "reportAttributes"
# Returns: 0
git show main:src/pages/inbox/report/ReportActionsList.tsx | grep -c "reportAttributes"
# Returns: 7Impact if not fixed: The Required action: The branch must be rebased on the latest main, or main must be merged into this branch again, ensuring the |
The feature was added to main after this PRs merge base, and this branch contributes a zero-diff for that file - so the merge would keep main's version untouched - no regression. That said, I've merged latest main into this branch to clear up any doubt :) |
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM
Thanks for the merge!
|
🚧 @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.3.98-0 🚀
Bundle Size Analysis (Sentry): |
Help site review: no changes required 📚I reviewed the changes in this PR against the help site articles under Why: This is Batch 6 of the Per-file breakdown
The help articles document user-facing features, settings, tabs, buttons, and workflows. None of those are added, removed, or changed here — the changes only swap the internal scheduling mechanism for scroll/layout timing and delete unreachable code. @mrejdak, no help site PR was created because these changes are internal-only with no user-facing impact. If you believe a documented behavior is affected here, let me know and I'll take another look. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.98-3 🚀
|
Explanation of Change
Batch 6 of InteractionManager migration
ReportActionItemEventHandler/was removed, as it was unreachable. The handler was no-op on every platform other than Android, while it's only use was inReportActionItemMessageEdit.tsx- a component which is only used with wide screen (so it is never rendered on mobile).Fixed Issues
$ #71913
$ #83067
PROPOSAL:
Tests
FormWrapper.tsx(web)WorkspacesNew->New workspaceand verify that the modal scrolls down so thatConfirmbutton is fully visibleMoneyRequestReportActionsList.tsxSpend->ReportsReportActionsList.tsxInboxReportActionsList.tsx(safari only)ReportActionsList.tsx(chrome dev tools required)Inboxand open a 1:1 chat (e.g. personal chat) with enough messages to scrollChrome dev toolsset throttling toOfflineAccount->TroubleshootChrome dev toolsset throttling back toNo throttlingReportActionsList.tsxSpend->ExpensesOffline tests
N/A
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
ReportActionsList.tsxbatch6_RAL_scroll_on_mnt.mov
batch6_RAL_scroll_on_mnt_new.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
FormWrapper.tsxbatch6_formWrapper.mov
MoneyRequestReportActionsList.tsxbatch6_MoneyReqAction_scroll.mov
batch6_MoneyReqAction_loadOld.mov
ReportActionsList.tsxbatch6_RAL_scroll_on_mount.mov
batch6_RAL_scroll_on_mount_new.mov
ReportActionsList.tsx(safari)batch6_RAL_safari.mov
ReportActionsList.tsx(Chrome dev tools required)Screen.Recording.2026-05-26.at.17.41.39.mov
ReportActionsList.tsxbatch6_RAL_newer_messages.mov