-
Notifications
You must be signed in to change notification settings - Fork 196
@W-19751635 SPM UX changes in checkout #3365
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
@W-19751635 SPM UX changes in checkout #3365
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
...s/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-payment-form.jsx
Outdated
Show resolved
Hide resolved
...s/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-payment-form.jsx
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-payment.jsx
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-payment.jsx
Outdated
Show resolved
Hide resolved
// Ensure basket reflects removal before rendering form | ||
await currentBasketQuery.refetch() | ||
} catch (_e) { | ||
// Ignore, user can still attempt to change method |
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.
So what is the experience in this case? User clicks edit, the payment method isn't removed from the basket due to some error. User selects a new payment method. Is the removal attempted a second time? Are they left with double payment?
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 are not attempting a removal a second time. Any suggestion on how to deal with this use case? @sf-mkosak
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.
Should we show an error message and prevent them from changing their payment method?
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.
Should we show an error message and prevent them from changing their payment method?
I think this is a decent experience since the user would be able to 'retry' or simply place the order with existing payment method.
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.
Done. I added an error message.
packages/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-payment.jsx
Show resolved
Hide resolved
...s/template-retail-react-app/app/pages/checkout-one-click/partials/one-click-payment-form.jsx
Show resolved
Hide resolved
|
||
const savedCount = savedPaymentInstruments?.length || 0 | ||
const totalItems = savedCount + 2 // saved + credit card + paypal | ||
const n = showAllPaymentInstruments ? totalItems : INITIAL_DISPLAYED_SAVED_PAYMENT_INSTRUMENTS |
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.
Nit: a more readable/meaningful variable name
const totalItems = savedCount + 2 // saved + credit card + paypal | ||
const n = showAllPaymentInstruments ? totalItems : INITIAL_DISPLAYED_SAVED_PAYMENT_INSTRUMENTS | ||
|
||
const displayedSavedCount = Math.min(savedCount, n) |
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.
This isn't the savedCount is it? n could be total items which is saved + hardcoded methods paypall and new CC
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.
It's the count of SPMs shown in the collapsed view. Min of SPMs and n which is either 3 when collapsed or total(SPMs + CC + PayPal) when expanded.
Description
Changes to make the UX better in the Saved Payment Method section of one click checkout
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization