Implement GrabPay at Checkout #10336
Conversation
|
@pierorocca Can you please validate the screenshots provided in the PR description and confirm that everything looks as expected? Thank you. |
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +30 B (0%) Total Size: 1.29 MB
ℹ️ View Unchanged
|
|
|
Thanks for the feedback @pierorocca !
Should we display only the logo or both the logo and text? e.g.
This is correct, the contents of that section are inside Stripe's iframe. |
@danielmx-dev thanks for confirming the Stripe content. For the logo on order confirmation, I believe we've been using just logos but there are some issues: The Logo is a bit small here and I can see there's extra white space in the logo causing the Visa portion to be too small.
Visa's SVG asset: IIRC we didn't put the text because on this layout the text would wrap but on a vertical orientation it worked fine. I think with text is better if it doesn't create formatting issues.
|
I'll take a look but this probably would need to be captured in a new issue.
Can you please share which theme is this ?
Got it. |
|
Tsubaki theme is the one with the horizontal layout in desktop mode @danielmx-dev. proccaatomic.wpcomstaging.com |
|
Thanks @pierorocca At least for that theme I tried several resolutions and didn't have any overflow issues:
However, there's no guarantee it'll look like that in 100% of the scenarios. If for some reason the size of that component were limited by the theme, it would look like this:
If it's a genuine concern, we could stick to just showing the name as you previously suggested. |
It is as the Payment section has so little space to work with. The block template doesn't provide sufficient room. There'd be a lot more space if the cells weren't equally sized or if the payment info was moved to a different area in closer proximity to the billing information. As you mentioned, we'll take this up in a different issue (actually project for order confirmation). Let's go ahead with the logo and ensure that we have alt-text applied. |
|
@yjailin FYI a data point for a future Order Confirmation block template redesign. |
danielmx-dev
requested review from
a team and
frosso
and removed request for
a team
There was a problem hiding this comment.
Changes look good and test well. A couple of questions:
- The duplication of the
show_lpm_payment_method_name()/show_payment_method_logo()methods - Should a mapping be created also in the
client/payment-details/payment-method/index.jsfile to show any additional payment method details?
| if ( $payment_method->get_id() === Payment_Method::GRABPAY ) { | ||
| $output = $this->show_payment_method_logo( $gateway, $payment_method ); | ||
| return false !== $output ? $output : $payment_method_title; | ||
| } |
There was a problem hiding this comment.
I see that the show_payment_method_logo() method was added, with an optional $show_payment_method_name argument - false by default.
But since show_payment_method_logo() is only used here, and since $show_payment_method_name will be false - what is the difference with the show_lpm_payment_method_name() method? Can we just use show_lpm_payment_method_name()?
There was a problem hiding this comment.
You are right, we can re-use the existing function, I have updated the tests as well and included the title attribute so it displays the payment method name on hover.
There was a problem hiding this comment.
Nice, thank you! I like that this addition will also improve existing LPMs!
Fixes #10101
Changes proposed in this Pull Request
In #10100, by adding GrabPay to some of the existing files related to Payment Methods, it became automatically integrated into checkout.
This PR will focus on making sure the whole Checkout flow is satisfactory, as well as other aspects like the Edit Order and Transactions pages for admins.
Regarding Automated Testing: Most additions to support checkout are just including GrabPay in a new list/map of items since the functionality to support redirect payment methods was already implemented, most functionality is covered by existing tests.
Testing instructions
Pre-requisite: follow the instructions defined in the following server PR to enable GrabPay payments locally 7231-gh-Automattic/transact-platform-server
Place the order, you'll be redirect to the hosted Test Payment Page.
Authorize the Payment, you'll now be redirected to the Order Received page.
Validate that the correct payment method logo is shown, as well as the
titleandaltattributes in the image:Go to My Account > Orders and open the details of the last order.
Validate that the correct payment method name is listed:
Go to WP Admin and open the last order.
Validate that the correct Payment Method is listed in the subheader:
Open the transaction details and validate that the correct payment method icon is displayed.
Open the transactions list and verify that the last transaction shows the correct payment method:
Other Scenarios
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge