-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add bnpl logos to thank you page. #8650
Add bnpl logos to thank you page. #8650
Conversation
* Adds the bnpl logo to the thank you page. * Generalizes and reuses CSS class for both WooPay and BNPL * Add max-height to bnpl logos
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: +28 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
…b.com/Automattic/woocommerce-payments into 8249/add-bnpl-icons-to-thank-you-page
This is to allow users to change the logo URL if needed. A specific use case is if we've returned the `dark_icon` URL but the regular icon is preferred/required.
I've left a few comments, but I'm to regretfully reassign my review, since I won't have time to give this a second look this afternoon, will be travelling tomorrow, and do not want to be a blocker for this PR. 🙏 |
…le to share some functionality
…hod for a given location and context.
…xists before trying to use it.
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.
Testing pretty well for me. Nice one! Just have a couple more comments about a few little things I stumbled upon along the way.
Co-authored-by: Samir Merchant <[email protected]>
Co-authored-by: Samir Merchant <[email protected]>
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.
Sorry, I should have gotten to this earlier today as I only have one small suggestion this time! There's a duplicate conditional we should probably remove; the second item, I'll leave at your own discretion.
This is still testing as expected for me, so if you can have one last look tomorrow morning, hopefully we can merge this in shortly after. 🙏
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.
LGTM. Thanks for responding to all of my minor suggestions on this PR! 🙌 🚢
Co-authored-by: Guilherme Pressutto <[email protected]>
Co-authored-by: Guilherme Pressutto <[email protected]>
Co-authored-by: Guilherme Pressutto <[email protected]>
Fixes #8249
Changes proposed in this Pull Request
This PR adds the bnpl payment method logo to the thank you page. I'm utilizing the
get_theme_icon()
to get the light/dark icon based on whichever checkout upe transient has been set (classic or block checkout). WooPay also adds a logo in this spot so I borrowed the approach used there but then generalized the wrapper class so I didn't have to repeat CSS. I also added a WooPay-specific class to that wrapper as well as the new bnpl wrappers in case we ever need to scope CSS to them individually. This implementation also adds awc_payments_thank_you_page_bnpl_payment_method_logo_url
filter to allow users to change the logo URL in case our transient specifies the dark logo but a light logo works better on the thank you page. It also falls back to the payment method title (how it currently works) if it can't get the logo URL for some reason.Testing instructions
npm run changelog
to add a changelog file, choosepatch
to 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