refactor: Remove manual memoization from SettlementButton and add UI tests#81818
refactor: Remove manual memoization from SettlementButton and add UI tests#81818roryabraham merged 11 commits intoExpensify:mainfrom
Conversation
|
@thesahindia 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!
|
…up-SettlementButton
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 843ae81054
ℹ️ 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".
|
I’ll review this based on the discussion in Slack |
|
Ah, it looks like @roryabraham wants to review this PR instead of assigning it to |
Apologies 😓 I didn't look into it before posting. |
…up-SettlementButton
|
@linhvovan29546 can you please review this PR? |
|
Reviewing... |
There was a problem hiding this comment.
getPaymentSubItems and paymentButtonOptions are not automatically memoized after applying this check. Could you please re-check? I used react complier playground(and react-compiler-marker extension) tool to verify the auto memoization
There was a problem hiding this comment.
Generally I think the pattern of defining a function in render to generate some object, then calling it once in render to generate that object is a bad practice in React Compiler world.
Because you're obfuscating the logic and wrapping it up into an unnecessary function that's called only once. If you instead inline the content of the function in render, I think React Compiler will do a better job memoizing it. The code tends to end up simpler too.
|
I'm blocked by this bug. I'll test the PR once it's resolved. |
|
@TaduJR Please add videos to the PR description. These tests are easy to record and attach. |
…up-SettlementButton # Conflicts: # src/components/SettlementButton/index.tsx
Done.
Done. |
|
I am having difficulties setting up VBBA. Raised a discussion here https://expensify.slack.com/archives/C01GTK53T8Q/p1772106257184279?thread_ts=1711049879.962529&cid=C01GTK53T8Q Feel free to reply if you have any suggestions. Thanks! |
I’ve replied. Could you merge main and resolve the conflicts? |
…up-SettlementButton # Conflicts: # src/components/SettlementButton/index.tsx
Done. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-02-27.at.17.17.55.movAndroid: mWeb ChromeScreen.Recording.2026-02-27.at.17.13.42.moviOS: HybridAppScreen.Recording.2026-02-27.at.17.07.15.moviOS: mWeb SafariScreen.Recording.2026-02-27.at.17.03.13.movMacOS: Chrome / SafariScreen.Recording.2026-02-27.at.16.58.11.movScreen.Recording.2026-02-27.at.17.00.58.mov |
|
@TaduJR looks like we've got some conflicts |
…up-SettlementButton # Conflicts: # src/components/SettlementButton/index.tsx
Done. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.28-0 🚀
|
|
Hi @TaduJR. QA team doesn't have a Gold tier. cc @roryabraham
|
|
@IuliiaHerets We don’t need the Gold tier |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.30-3 🚀
|


Explanation of Change
This PR addresses the first two goals of #79387: adding automated UI tests and removing manual memoization from
SettlementButton.Fixed Issues
$ #79387
PROPOSAL: #79387 (comment)
Tests
Prerequisites
Test 1 — Pay from Report Preview in Chat
Test 2 — Pay from Report Header
Test 3 — Invoice Room (Regression for #80189)
Test 4 — Search Results
Test 5 — Pay Someone (Quick Action)
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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-Native-1.mp4
Android-Native-2.mp4
Android: mWeb Chrome
iOS: Native
iOS-Native.mp4
iOS: mWeb Safari
iOS-Safari.mp4
MacOS: Chrome / Safari
Mac-Chrome-1.mp4
Mac-Chrome-2.mp4