Fix bugs around STPPaymentCardTextField becomeFirstResponder
#855
Conversation
This file contains 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
Previous implementation of `nextFirstResponderField` would return `nil` if all fields were valid, which blocked programatic calls to `becomeFirstResponder`. It *also* used to cycle through the fields every time it was called, which doesn't seem right. It feels more natural for the `nextFirstResponderField` to be either the first invalid field *or* the last field. This puts the insertion point at the right place to start deleting. It also feels better if the user manually changes to the expiration field before entering the card number: once the expiration is valid the firstResponder jumps back to numbers. Then, once they complete numbers, it jumps to the CVC. I also added a test for this behavior. Some of the things it verifies: * becomeFirstResponder should not change the current first responder if the current field isn't valid yet * becomeFirstResponder should take the user to the first invalid field when called * becomeFirstResponder should make the last sub-field first responder when all the fields are valid
|
We sync'ed offline on this. Dan is going to make some tweaks. |
…e next field, even if others are invalid It's good to have the explicit progression through fields. Keep the previous behavior that when they're on the last field and finish filling it out, take them back to the first invalid field. Also, update `canBecomeFirstResponder` and `becomeFirstResponder` to *never* change which field is the first responder, if one of the subfields already is. If there isn't one, use either the first invalid field or the last field. Updates tests to reflect the new expected behavior.
danj-stripe
deleted the
danj/bugfix/841-paymentcardtextfield-becomeFirstResponder
branch
danj-stripe
added a commit
that referenced
this pull request
Dec 15, 2017
danj-stripe
added a commit
that referenced
this pull request
Dec 18, 2017
mludowise-stripe
pushed a commit
that referenced
this pull request
Mar 15, 2022
* Add parameters * Migrate to the new API * Remove Link payment details from authentication context * Cleanup and add test * Cleanup * Use testing publishable key * Allow sending CVC * Cleanup * Handle client-side error
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Previous implementation of
nextFirstResponderFieldwould returnnilif all fields werevalid, which blocked programatic calls to
becomeFirstResponder.It also used to cycle through the fields every time it was called, which doesn't seem
right. It feels more natural for the default first responder to be either the first
invalid field or the last field. This puts the insertion point at the right place
to start deleting.
We do want to move to the next field (ignoring whether fields are valid or not) when the user fills the current field with valid text. This matches the web elements behavior, and feels better as they proceed through, instead of jumping around (possibly earlier, possibly to the end)
Motivation
Fixes #841
Testing
Manual testing through the Standard Integration. I also added a unit test for this
behavior. Some of the things it verifies: