-
Notifications
You must be signed in to change notification settings - Fork 216
Show unsupported payment methods at the end of the payment method list #4523
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
base: develop
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR implements sorting of payment methods to prioritize those that support the store currency, moving unsupported payment methods to the end of the list. The implementation extracts existing currency logic into a reusable helper function and adds a new hook to handle the sorting logic.
Key changes:
- Refactored
usePaymentMethodCurrencies
hook by extracting core logic intogetPaymentMethodCurrencies
helper function - Implemented
usePaymentMethodsSortedByStoreCurrencySupport
hook to sort payment methods based on store currency support - Updated payment methods list component to use the sorted payment method IDs instead of the original order
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
client/utils/use-payment-method-currencies.js | Extracted payment method currency logic into reusable getPaymentMethodCurrencies helper function |
client/settings/general-settings-section/payment-methods-list.js | Added sorting hook and updated component to display payment methods sorted by currency support |
client/settings/general-settings-section/payment-methods-list.js
Outdated
Show resolved
Hide resolved
client/settings/general-settings-section/payment-methods-list.js
Outdated
Show resolved
Hide resolved
client/settings/general-settings-section/payment-methods-list.js
Outdated
Show resolved
Hide resolved
if ( | ||
paymentMethodCurrencies.length === 0 || | ||
paymentMethodCurrencies.includes( storeCurrencyCode ) | ||
) { |
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.
Question: Is this check sufficient? Are there more conditions we should be using (or attaching to the data) to push payment methods into the trailing section?
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.
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.
I am torn on this one, to be honest. If they disable the other payment gateway, the payment method will be available immediately, so it could "move" unexpectedly for that case. So that case feels subtly different to me, and feels like adding it could lead to even weirder UX. 😐
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.
The code changes look good... and it tests very well.
We are just missing the case of BNPL payment methods that might be disabled for "plugin conflicts"
--
As a side note, we might also want to consider hiding unavailable payment methods by default and a "Show unavailable payment methods" (or show all, or similar) checkbox at the top to show them. The list is getting pretty long, and half of them are usually unavailable because of currency restrictions:

I think this is something we probably should do, but I think we might need some basic design for that UX from @orcungogus. Maybe that makes sense for a follow-up PR and/or effort? |
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 looks good to me and tests well. I really like Diego's idea on the toggle, but it makes sense to introduce that in a separate PR.
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.
Code looks good, and tests well 🚢
We then sort the list by available payment methods, then payment methods with a conflict, and then payment methods unavailable due to the store currency not being supported.
And payment methods in each category can be re-ordered only among them, as expected:

Adding the show/hide unavailable payment methods toggle in a follow-up PR makes sense to me 👍🏼
Changes proposed in this Pull Request:
This PR ensures that we sort the payment method list based on whether the payment method can be enabled at present. The underlying implementation centralises the logic for availability so we're always using the same core logic for whether a payment method is available (or not). The new logic is implemented in
getPaymentMethodUnavailableReason()
, with the possible return types declared as part of aPAYMENT_METHOD_UNAVAILABLE_REASONS
constant. At present, we check for two conditions:We then sort the list by available payment methods, then payment methods with a conflict, and then payment methods unavailable due to the store currency not being supported.
At an implementation level, I have tackled the following:
usePaymentMethodCurrencies()
hook into a newgetPaymentMethodCurrencies()
helpergetPaymentMethodUnavailableReason()
helper to identify why a payment is (or is not) available, as well as a lightweightusePaymentMethodUnavailableReason()
wrapper hookusePaymentMethodsSortedByAvailability()
hook defined locally in the payment method list code, which relies ongetPaymentMethodUnavailableReason()
to determine whether each payment method is available (or not), and then performs the sort based on the result.Open questions and concerns
Screenshots
These screenshots are for a store that is configured to accept USD.
Before
After
Testing instructions
Changelog entry
Changelog Entry Comment
Comment
Post merge