-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Update Payment flow on clicking Pay #50640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Payment flow on clicking Pay #50640
Conversation
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
This reverts commit e45613d.
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
Hey @rezkiy37, could you please update this PR with the main so I can start testing the backend changes locally? 🙏 |
…ture/45171-invoicing-payment-flow
@cristipaval, done 🙂 |
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
…ture/45171-invoicing-payment-flow
…ortHeader and ReportPreview components. Update IOU.ts to align with new parameter structure for improved readability and maintainability.
@ZhenjaHorbach I don't see the bug on the video because:
|
It is unrelated to the PR because this blinking issue was known before and associated with the SettlementButton dropdown. |
Oh |
Strange |
🚧 @cristipaval has triggered a test app build. You can view the workflow run here. |
Oh, my bad; I should have done one more manual step to trigger the workflows. Just did it, sorry. It may take around 1 hour. |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
Crap I understand that this is a issue with implementation of the modal But I think we can fix it in a separate issue What do you think ? 2025-03-17.23.30.15.mov |
I definitely agree with it and I can take it into work. |
…ture/45171-invoicing-payment-flow
I've synced it up. |
@cristipaval |
yeah this is a very minor visual issue, it's fine to fix it in a follow-up to not keep extending the scope of this PR |
Nice In that case I have no more questions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Thank you @rezkiy37 for pushing this for a long time.
I tested and can confirm that the payments work with real bank accounts.
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
I've created a follow-up issue - #58705. |
Nice |
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.1.16-0 🚀
|
QA passed in staging Screen.Recording.2025-03-21.at.10.53.40.mov |
🚀 Deployed to production by https://github.com/luacmartins in version: 9.1.16-4 🚀
|
text: formattedPaymentMethod?.title ?? '', | ||
description: formattedPaymentMethod?.description ?? '', | ||
icon: formattedPaymentMethod?.icon, | ||
onSelected: () => onPress(CONST.IOU.PAYMENT_TYPE.EXPENSIFY, payAsBusiness, formattedPaymentMethod.methodID, formattedPaymentMethod.accountType), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to pass iconStyles
, iconHeight,
and iconWidth
here, otherwise the bank icons are too small. More details: #60579 (comment)
Details
The PR introduces new ways for the user to pay invoices.
Fixed Issues
$ #45171
PROPOSAL: N/A
Tests
Preconditions:
Sender
Payer
pay/new/add-debit-card
.Sender
Offline tests
Same as tests (payer only).
QA Steps
Same as Tests. This needs real bank accounts and payments. Ping @cristipaval, he's more than willing to pay 0.1 USD to test and ship this feature 😄
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android.mp4
Android: mWeb Chrome
Android.Chrome.mp4
iOS: Native
IOS.mp4
iOS: mWeb Safari
IOS.Safari.mp4
MacOS: Chrome / Safari
Chrome.mp4
MacOS: Desktop
Desktop.mp4