feat: use standardized payment button on search page#68526
feat: use standardized payment button on search page#68526grgia merged 8 commits intoExpensify:mainfrom
Conversation
|
@hungvu193 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] |
|
🚧 @joekaufmanexpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
I tested and found some bugs! |
|
Still discussing pay with workspace bug here: |
|
🚧 @joekaufmanexpensify 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, Desktop, and Web. Happy testing! 🧪🧪
|
|
🚧 @joekaufmanexpensify 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, Desktop, and Web. Happy testing! 🧪🧪
|
|
The loading state for the dropdown button (NOT the split button) seems broken: Here is a full vid: |
Bulk payments is planned to be handled separately. Is this different behavior from staging? |
|
Ah ok, I see. @joekaufmanexpensify let me know when we have bulk payment and I'll happily test it on my test account. |
|
Will do! Yep, this is just adding the new pay button pattern on the reports page for single payments, and then we'll handle bulk pay in a follow up PR. |
Interesting. I'm not seeing the loading state on the button when I mark an IOU as paid. Maybe it's only with an active wallet? I will test some more. 2025-08-22_12-36-00.mp4 |
|
@joekaufmanexpensify it has to be the dropdown button I think, not the split button:
|
|
Tested the PR again today and everything else looks good from my perspective 🫡 |
|
Small UI bug: While paying for the first time, the loading indicator is too close to the right arrow bug1.mov |
| selectedTransactions: SelectedTransactions, | ||
| canSelectMultiple: boolean, | ||
| shouldAnimateInHighlight: boolean, | ||
| hash?: number, |
There was a problem hiding this comment.
I see comments about the hash being used for Search payments, can you explain a little more what the function of this argument is and how we use hash in this function?
There was a problem hiding this comment.
We decided to use payMoneyRequestOnSearch, and hash is a required parameter. I’m not exactly sure what it does, since I didn’t dive into it.
|
|
||
| function getLatestBankAccountItem() { | ||
| if (!hasVBBA(policy?.id)) { | ||
| if (!policy?.achAccount?.bankAccountID) { |
There was a problem hiding this comment.
why are we not using hasVBBA anymore?
There was a problem hiding this comment.
It is impure function which uses the deprecated getPolicy function
grgia
left a comment
There was a problem hiding this comment.
Code is looking good, I have a few questions for clarity. Also would you merge main 🙏
…tton-reports-page
That looks much better! This is mostly a question for the @Expensify/design team, but I'm wondering if we should bump the size of that loader in the button down to be a bit smaller? Feels a little big in there. |
|
@dannymcclain I think you're the only design team online today, do you want to change it in this PR? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-08-30.at.11.18.08.movAndroid: mWeb ChromeScreen.Recording.2025-08-30.at.11.02.48.moviOS: HybridAppScreen.Recording.2025-08-30.at.10.50.35.moviOS: mWeb SafariScreen.Recording.2025-08-29.at.22.02.14.movMacOS: Chrome / SafariScreen.Recording.2025-08-29.at.21.45.30.movMacOS: DesktopScreen.Recording.2025-08-29.at.21.46.49.mov |
hungvu193
left a comment
There was a problem hiding this comment.
LGTM. I'm having uploading issues but will update the rest of screenshots ASAP.
@grgia let's leave it be for now. It's working fine as-is, so when the rest of the design team is on we can talk about it, and if we need to we can always just make a little follow-up issue. |
|
@hungvu193 please ping when screenshots are up and I will merge |
|
I will approve for product as I have extensively tested this one 👍 |
|
All yours @grgia 😄
|
|
Wooo, exciting! Thank you all for the effort here. |
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.2.1-0 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.2.1-20 🚀
|
|
|
||
| if (action === CONST.SEARCH.ACTION_TYPES.PAY && !isInvoiceReport(iouReport)) { | ||
| return ( | ||
| <SettlementButton |
There was a problem hiding this comment.
As this component is wrapped with SearchScopeProvider (as inside <Search />), this caused #73722 which came from inconsistency between Onyx data and snapshot data.



Explanation of Change
Fixed Issues
$ #68283
PROPOSAL:
Tests
Same as QA
Offline tests
QA Steps
Preconditions
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))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
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-08-14.at.16.09.42.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2025-08-14.at.2.37.28.in.the.afternoon.mov
MacOS: Desktop