Skip to content

Do not consider keyboardDistanceFromTextField for scrollView contentInset#1655

Closed
daniil108 wants to merge 4 commits into
hackiftekhar:masterfrom
daniil108:master
Closed

Do not consider keyboardDistanceFromTextField for scrollView contentInset#1655
daniil108 wants to merge 4 commits into
hackiftekhar:masterfrom
daniil108:master

Conversation

@daniil108
Copy link
Copy Markdown
Contributor

@daniil108 daniil108 commented Nov 15, 2019

Fixed incorrect behavior when TextFields are on scrollView and using a custom keyboardDistanceFromTextField. Now scrolling consider keyboardDistanceFromTextField. @hackiftekhar please confirm it. If you have any suggestions on how this can be done differently and more correctly, please let me know.

Please see gifs
Before:
before

After:
after

@hackiftekhar
Copy link
Copy Markdown
Owner

Hi, can you please share this demo with me?

@daniil108
Copy link
Copy Markdown
Contributor Author

@hackiftekhar please see the demo, you can try reproduce the issue as on the gif
https://github.com/daniil108/IQKeyboardTest

ezgif-5-7d867e64633d

@hackiftekhar
Copy link
Copy Markdown
Owner

ok, I'll see it @daniil108 once I get some time.

@witekbobrowski
Copy link
Copy Markdown

@daniil108 Thank you so much for solving this issue! I used the solution in my project and it now works as expected. Thank you!

@hackiftekhar Any chances of merging it anytime soon?

@hackiftekhar
Copy link
Copy Markdown
Owner

I will be checking this demo project soon @witekbobrowski.

@daniil108
Copy link
Copy Markdown
Contributor Author

@hackiftekhar hey! What about this pull request? Do you have questions?

@hackiftekhar
Copy link
Copy Markdown
Owner

@daniil108 I checked the demo and found that you are correct. The behavior should be like your suggested changes. However, I have to make some more changes after yours to show the scroll indicator at correct position. I'll be merging your changes soon.

@hackiftekhar
Copy link
Copy Markdown
Owner

I cherry-pick your c96e135 and added some more changes here 6755980 to fully fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants