Update checkout payment methods design. #3439
Conversation
We need it to traget that element with CSS.
|
Size Change: -23.1 kB (-2%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
|
Gave this some early testing, this is looking good and working well – I really like the new radio button UI. I found a few gaps / tweaks, I'm guessing most of these are already on your radar.
I mostly tested in Safari and Firefox, with a range of payment methods, saved cards (or none) and window sizes (responsive modes seem to work ok). Tested saving payment methods to account, and as anon or logged-in user. Didn't see any payment issues, apart from being able to submit with no payment method selected 😄 Will need to do another test pass on the styling but it's all looking very much like the designs and working well 🚀 |
There was a problem hiding this comment.
@budzanowski I did a quick investigation on why the defaulting isn't working - added clues in the comments. This isn't a full review, apologies for not helping more (yet!). I'll check back in tomorrow and push some fixes (if needed) 👍
| ); | ||
| const onChange = useCallback( | ||
| const [ savedMethodsToken, setSavedMethodsToken ] = useState( '' ); | ||
| const [ paymentMethodsName, setPaymentMethodsName ] = useState( '' ); |
There was a problem hiding this comment.
I don't think you'll need state for these. The payment method context should handle all this (and provide it to the component). Take a look at how usePaymentMethodDataContext and usePaymentMethodInterface hooks are used in trunk to get and change the active payment method.
There was a problem hiding this comment.
@budzanowski I ended up removing lots of this when I moved saved payment method token (saved card) state into context.
I don't think this caused problems, but there may be more tidying we can do here. There's some preexisting local state in SavedPaymentMethodOptions which I don't fully understand. We might be able to move that logic to context too, since the active payment method / token are closely related (i.e. same set of radio buttons).
| id="wc-block-payment-methods" | ||
| <RadioControlAccordion | ||
| id={ 'wc-payment-method-options' } | ||
| selected={ selectedMethod } |
There was a problem hiding this comment.
You'll likely want to use activePaymentMethod here (ultimately powered by context - PaymentMethodDataContext).
That gives you the defaulting behaviour (ensure a payment method is selected at all times).
I hacked this change in my local dev and it fixes the missing default on load, and also handles if payment methods disappear at runtime (e.g. COD dependent on shipping).
There was a problem hiding this comment.
I've pushed a fix for this: 5381cfc
- using `activePaymentMethod` from context - this ensures there is a default selected on initial render - and handles any dynamic changes to available payment methods - e.g. COD disappearing when change shipping option - remove unused / redundant selectedMethod prop - context is best
|
The section labels weight was taken directly from the design doc: |
|
Payments label size fixed. Gosh, that was embarrassing. I did not have PayPal enabled the whole time 🤦🏻 |
# Conflicts: # assets/js/base/components/payment-methods/test/payment-methods.js
There was a problem hiding this comment.
Thanks for addressing some of the feedback Bartek, testing seemed to go well.
Regarding section label weight: I see it matches designs, let's leave as is for now then.
There are still a few issues not addressed:
- My comment about test coverage.
- The regression in padding around the "Add a note to your order" checkbox.
- The inconsistent spacing between payment method option text and the radio.
Screenshots for the last two issues:
In this PR:
In trunk:
Also, can you please verify you've:
- tested accessibility of the ui.
- tested with mobile views (seemed to work okay in my testing other than the order note checkbox issue which persists here).
|
I have noticed that Chrome has an accessibility helper in inspect tool: |
There was a problem hiding this comment.
Changes look good and test well. Designs in figma still have borders around the add notes checkbox:
However, I'm fine with the current implementation - styles can be revisited in future work on things like the return to cart link and payment method button design mismatch.
nerrad
added
type: enhancement
This PR implements the updated checkout payments UX.
Fixes #2628
Latest design available at pca54o-wS-p2 n90hX6hG3wwQbYwsYsrJnj-fi-4703%3A43711
Screenshots
How to test the changes in this Pull Request:
To test this one needs to use a combination of available and saved payment methods:
No saved payment methods.
One saved payment methods.
Multiple saved payment methods.
No payment methods.
One payment method.
Multiple saved payment methods.
The test should consist of checking the UI using combinations of saved and non-saved payment methods.
The other part of the test is ensuring that the payment methods still work.
Changelog