-
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
Open
daledupreez
wants to merge
19
commits into
develop
Choose a base branch
from
try/relevance-sort-in-payment-method-page
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+509
−137
Open
Changes from 8 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
c2565ec
Show payment methods without currency support at the end of the payme…
daledupreez dcaba9a
Add JSDoc
daledupreez 7e0455d
Merge branch 'develop' into try/relevance-sort-in-payment-method-page
daledupreez 7377fbf
Memoize and refactor sorting
daledupreez b3956c5
Fix JS unit tests
daledupreez 4b1506e
Merge branch 'develop' into try/relevance-sort-in-payment-method-page
daledupreez bf373e3
Add changelog
daledupreez f1b8d38
Use getSetting() instead of global reference; rename variable
daledupreez c966d88
Update unit tests to mock getSetting() and centralise currency mocking
daledupreez 9f3e05e
Return early when UPE is disabled; update notes
daledupreez a0f6d46
Merge branch 'develop' into try/relevance-sort-in-payment-method-page
daledupreez f3a981a
Refactor logic into central helper function
daledupreez 28c4eb3
Merge branch 'develop' into try/relevance-sort-in-payment-method-page
daledupreez a2230e5
Update unit tests
daledupreez 3089fce
Add tests for getPaymentMethodUnavailableReason
daledupreez a13b5c6
Sort plugin conflicts before currency issues
daledupreez 8416544
Merge branch 'develop' into try/relevance-sort-in-payment-method-page
daledupreez 0e5a1fe
Merge branch 'develop' into try/relevance-sort-in-payment-method-page
daledupreez f74b2bf
Fix changelog entry location
daledupreez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 is also the case of payment methods disabled because of a plugin conflict (Affirm/Klarna):
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. 😐