michaeltyson / TPKeyboardAvoiding

A drop-in universal solution for moving text fields out of the way of the keyboard in iOS
http://atastypixel.com/blog/a-drop-in-universal-solution-for-moving-text-fields-out-of-the-way-of-the-keyboard/
zlib License
5.81k stars 925 forks source link

Fix for a scrolling bug when the first responder is a tall text input #212

Closed sibljon closed 8 years ago

sibljon commented 8 years ago

When the keyboard frame changes, TPKeyboardAvoiding attempts to center the first responder view in the visible space above the keyboard. This results in a jarring change in scroll position (see the "before" video below) in certain circumstances (for example: an auto-growing text view). I believe that a better approach is to, instead of centering the first responder view, center the cursor (when possible).

This is slightly challenging because the cursor is not accurate for a UITextView first enters its editing state. Accordingly, you'll see in my change that I've delayed the initial contentOffset adjustment due to a changing keyboard frame until a future runloop. I'm not entirely happy with this approach because of the natural hand-wavy nature of delaying to future run-loops. In addition, I found that a simple dispatch_async() was not sufficient to ensure that the UITextView's actual cursor position is measured, so I was forced to resort to a dispatch_after(0.01 seconds), which is an even-less precise method of targeting a future run-loop in which the text view's cursor will be set. I'm hopeful the there exists a better method to deterministically target the correct run-loop that I haven't yet tried.

For reference, I've added a tab to the example project to demonstrate a non-scrolling UITextView whose height is much larger than the

Before my change: http://cl.ly/2Y3l0S3X001A/tall_text_view_demonstration_before.mov

After my change: http://cl.ly/0p343Y2E0R2k/tall_text_view_demonstration_after.mov

Warnings:

michaeltyson commented 8 years ago

Thanks @sibljon! Just checking this out - looks like there's a bug: https://www.dropbox.com/s/hliazn0bmbfe2ty/bug.mov?dl=0

sibljon commented 8 years ago

@michaeltyson thanks for testing, good find, and my apologies :)

The cause of the bug you pointed out was an ordering issue, where upon keyboard dismissal, previously the statements in -TPKeyboardAvoiding_keyboardWillShow: were executed before -TPKeyboardAvoiding_keyboardWillHide:; and my changes reversed that order (do to the fact that I dispatched the statements in *keyboardWillShow: to be executed in the future).

I managed to return the ordering to the way it was before by making the state updates synchronous in -TPKeyboardAvoiding_keyboardWillShow:, even though the changes to the scroll view offset/inset happen asynchronously.

In general, it always makes me nervous to apply the broad stroke which is dispatch_async for operations where order is important. It occurred to me that we might benefit from an operation queue or performSelector:withDelay:. Although, even if that were the best solution long term (right now, I have no idea), that would be sure to be a lot of work, introduce bugs, and fix something that ain't broke :)

michaeltyson commented 8 years ago

Cool, nice one =)

I'm a bit skittish about merging this in directly without a bit more testing, so I'm going to leave it open and put out a request on Twitter to encourage others to try it out, I think, for now.

sibljon commented 8 years ago

Good idea. No rush. On Sat, Mar 5, 2016 at 6:14 PM Michael Tyson notifications@github.com wrote:

Cool, nice one =)

I'm a bit skittish about merging this in directly without a bit more testing, so I'm going to leave it open and put out a request on Twitter to encourage others to try it out, I think, for now.

— Reply to this email directly or view it on GitHub https://github.com/michaeltyson/TPKeyboardAvoiding/pull/212#issuecomment-192781446 .

michaeltyson commented 8 years ago

Merged - no takers on the testing, but I wanted to close this one, so we'll see how it goes in the wild.

sibljon commented 8 years ago

Sure. Feel free to call on me for fixes (I can at least try :) ), should someone report a bug. On Sat, Mar 19, 2016 at 4:38 AM Michael Tyson notifications@github.com wrote:

Merged - no takers on the testing, but I wanted to close this one, so we'll see how it goes in the wild.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/michaeltyson/TPKeyboardAvoiding/pull/212#issuecomment-198626915

michaeltyson commented 8 years ago

Cheers =)