Always show Express payment buttons for Pay for Order page #7535
Conversation
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: +2.52 kB (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
| @@ -499,6 +499,11 @@ public function should_show_payment_request_button() { | |||
| return false; | |||
| } | |||
|
|
|||
| // Order total doesn't matter for Pay for Order page. Thus, this page should always display payment buttons. | |||
| if ( $this->is_pay_for_order_page() ) { | |||
There was a problem hiding this comment.
shouldn't we check if all the products are supported by the payment request button ( most of the conditions from has_allowed_items_in_cart method ) ?
There was a problem hiding this comment.
Hmmm, I think you are right! I'm unfamiliar with the logic and helpers here, but it looks like $this->is_product_supported() does that check right?
There was a problem hiding this comment.
Moved the check a little down as discussed.
There was a problem hiding this comment.
I agree with @elazzabi, moving the check down LGTM.
There was a problem hiding this comment.
@elazzabi LGTM
@pierorocca @elizaan36 Pinging you here for the design input. Below is what I see on the pay-for-order page with the Google pay button. Does the button separation look good or should the button display next to the WooPay button?
|
Thank you @hsingyuc for looking! I agree that it should probably show close to the WooPay button. I'll take a look and see why it's showing there |
Most likely because we were under the assumption Pay for Order didn't support GPay and Apple Pay so we chose a button location based what we felt was the optimal location - after the invoice, and before the payment form. That pattern best matches the pattern found on cart and checkout. |
|
For clarity the GPay and Apple Pay button should render below the WooPay button. |
|
@pierorocca @elizaan36 I had some problems replicating this, but with help from @hsingyuc I got it working. So the problem is the Woo Pay button is showing only if WooCommerce Blocks is installed (at least in the Pay for Order page). This means, if a user doesn't have WooCommerce Blocks, it will only show Apple/Google Pay at the top. If they have the WC Blocks, it will show Apple/Google Pay at the top, and Woo Pay and the button like @hsingyuc shared ^. I've taken a look into the code and it still seems unclear to me why the handling is different for this page. So I'll need some help from folks working on WooCommerce Blocks to help with that. That being said, is this issue out of the scope of this PR, or would you like this to be fixed part of this PR? (Also, I'll be on a long AFK, so maybe someone else should pick it over if we decide to go with the second option) |
|
Thanks Ahmed. We definitely cannot go to market with a split scenario where buttons are showing up in multiple locations. Additionally I'd argue the existing button location at the top is not ideal placement. So we'd want 1) all buttons after the invoice 2) consistent behavior regardless if the merchant has shortcode or block checkout. |
|
+1 to what Piero shared here. |
|
I agree, we should have everything in one place to deliver a good UX. I have a proposal for a solution, but I'll need to check with other folks to make sure I'm not breaking something else 😅 
|
| @@ -381,8 +381,6 @@ public function display_pay_for_order_page_html( $order ) { | |||
| ]; | |||
|
|
|||
| wp_localize_script( 'WCPAY_PAYMENT_REQUEST', 'wcpayPaymentRequestPayForOrderParams', $data ); | |||
|
|
|||
| $this->display_payment_request_button_html(); | |||
There was a problem hiding this comment.
For future eyes: This is removed from here because it injects a div in the page with the same id expected for Apple/Google Pay creating the button in the top of the page instead of close to WooPay and other express payments.
There was a problem hiding this comment.
Looks good to me and tested well.
I created a follow-up PR to restructure the pay-for-order check for express buttons so that it works with older WC Blocks versions.
|
Thank you for testing, reviewing, and creating a follow-up change @hsingyuc 🙇 Given this has enough reviews now, I'll go ahead and merge it. |
Fixes #6650
Changes proposed in this Pull Request
A while ago, a regression was introduced making Express pay buttons not showing in the Pay for Order page. This fixes the issue.
Testing instructions
You'll need these requirements to test this PR:
Testing instructions:
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