Skip to content
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

Fix STPPaymentCardTextField touch handling bugs for small devices on iOS 11 #883

Merged
merged 2 commits into from
Jan 12, 2018

Conversation

danj-stripe
Copy link
Contributor

Summary

The maskView over STPPaymentCardTextField numbers no longer blocks touches on iOS 11.
Also, making the brandImage tappable to move focus/first responder to the numbers text
field.

Motivation

Found while testing #870, #855, etc. Filed as IOS-601

Testing

Manual testing in Standard Integration sample app, on small & large phones.

This view controls what part of the text field is visible, and so we need to be able to
tap the text field behind it when it exists (happens on 4" devices)

In iOS 11, this view appears in the view hierarchy (seen in Xcode Visual Debugger), and
it's capturing touches. Tapping/clicking directly on the card numbers does not cause the
numbers text field to become first responder, instead it dismisses first responder!

This change fixes that, and now you can go back to the numbers field from the month field.
…the numbers field.

Especially on 4" devices (in compact mode), there's a very small touch target for
activating the numbers field. The brand image is just as big, but taps on it will resign
first responder.

I considered doing something even more complicated: ideally any touch in the whole
STPPaymentCardTextField's bounds would give it first responder, and it would be the text
field that's closest to where the user tapped.

For this quick change, I don't really want to adjust the subfield bounds, or
change/understand the code that calculates the frames to increase their effective touch
targets.

Just making the brandImage tappable is a quick and easy win.
@danj-stripe
Copy link
Contributor Author

In the debugger view, you can see the frames of the fields:
screen shot 2018-01-11 at 4 58 20 pm

  • brandImage: selected (blue highlight), now has a tap recognizer to move focus to the numbers text field
  • maskView: shown in black. The only part of the numbers field that's drawn is what's covered by the maskView, which is once again a valid tap target for the numbers text field.
  • Similarly, you can see the bounds on the expiration & CVC fields (they go full height, but hug horizontally pretty tightly).
  • Taps in the white areas between the text fields will resignFirstResponder, which isn't great, but hasn't changed.

Copy link
Contributor

@bg-stripe bg-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@stripe-ci stripe-ci assigned danj-stripe and unassigned bg-stripe Jan 12, 2018
@danj-stripe danj-stripe merged commit 0dd2dc2 into master Jan 12, 2018
@danj-stripe danj-stripe deleted the danj/bugfix/paymentcardtextfield-touch-handling branch January 12, 2018 17:32
mludowise-stripe pushed a commit that referenced this pull request Mar 24, 2022
* Add `titleAttributes` to `Button`

* Remove redundant icon view

Prevent extra leading and trailing padding from being added to the
button.

* Update snapshots

* Hide image view if needed

* Reduce the number of constraints

* Simplify layout logic

* Update plain button insets

* Adjust insets

* Invalidate intrinsic content size when needed

* Prevent unnecessary constraint updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants