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

Adjust keyboard avoiding behavior #943

Merged
merged 2 commits into from
May 24, 2018

Conversation

joeydong-stripe
Copy link
Contributor

@joeydong-stripe joeydong-stripe commented May 23, 2018

simulator screen shot - iphone se - 2018-05-23 at 10 25 23

Summary

Stop adjusting scrollView.contentOffset and let the system natively scroll because of the scrollView.contentInset change. This will scroll the minimum amount until the text field is visible. Previously, we would scroll until the text field is vertically centered on screen.

Testing

Did some basic testing with iPhone SE 11.2 Portrait + Landscape Simulator but we should do additional testing on other configurations since I don't have a confident understanding of how UIViewController+Stripe_KeyboardAvoiding works.

#942

Stop adjusting `scrollView.contentOffset` and let the system natively scroll because of the `scrollView.contentInset` change
@Klingefjord
Copy link

👍 lgtm (only tested on iPhone 7 iOS 11 though)

@joeydong-stripe
Copy link
Contributor Author

Verified similar behavior on iOS 10 and iOS 9.

@joeydong-stripe joeydong-stripe force-pushed the joeydong/keyboardcontentinset branch from 9d802d7 to 6e3a943 Compare May 24, 2018 18:15
Copy link
Contributor

@danj-stripe danj-stripe left a comment

Choose a reason for hiding this comment

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

Looks good to me

@joeydong-stripe joeydong-stripe merged commit 85ebaa3 into master May 24, 2018
jaimepark-stripe pushed a commit that referenced this pull request Apr 11, 2022
Attempts to fix recurring flaky snapshot tests
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.

5 participants