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 924 forks source link

Not restoring contentOffset? #84

Closed Janne-M closed 9 years ago

Janne-M commented 10 years ago

Hi,

I ran into an issue that after the keyboard were closed, the whole view were shifted upwards aprox 85px (in my particular case...).

In closer inspection of the code, the TPKeyboardAvoiding_idealOffsetForView method would modify the contentOffset and it would never return to the previous (0,0) value after closing the keyboard.

I solved it in the code by adding a priorContentOffset property in the TPKeyboardAvoidingState class, and setting and resetting it in sync with the other "prior properties".

Being a total novice using git and not being sure that this IS a good solution in all cases, I've not committed this change. But I'd like someone with more knowledge to perhaps take a look at why this weren't done in the first place?

osklar0328 commented 10 years ago

I think this is a similar problem that I'm having, but as you I'm a total novice using git. So I hope someone with better knowledge can help us!

michaeltyson commented 10 years ago

Hey guys - Hmm, I'm having trouble visualising the problem. Any chance you could attach a screenshot so I can see what's going on? I'd prefer not to restore the content offset, as it might be confusing seeing the view scroll around after the keyboard disappears, especially if the user's moved through a number of views with the 'next' button.

Janne-M commented 10 years ago

I thought You might have a reason no to restore it... but here is some screenshots: skarmavbild 2014-01-31 kl 09 29 07 skarmavbild 2014-01-31 kl 09 29 16 skarmavbild 2014-01-31 kl 09 29 24

osklar0328 commented 10 years ago

img1 img2 img3

michaeltyson commented 10 years ago

Hmm, very interesting. Tell me, if you scroll the view after the keyboard's disappeared, does it snap back to how it should be, or does that gap remain?

Janne-M commented 10 years ago

No, the gap remains... and at first I thought it was the content area that were modified since the scroll indicator will also adjust to 'the new size'. But after inspecting that and the content offset I noticed that the content area were correct, but the offset were modified and never restored...

michaeltyson commented 10 years ago

Hmm, so the contentSize property doesn't change? What about contentInset, is that restored correctly? It seems to me that the only way that massive gap at the bottom could remain would be that the contentSize - or the contentInset - would be increased... I can't reproduce that behaviour with the sample app, so it's a bit baffling! Do you think you might be able to find a way to make the sample app behave the same way?

859988748 commented 9 years ago

I have this problem too as @Janne-M screen shot shows

859988748 commented 9 years ago

@michaeltyson From my debug, when the keyboard is disappearing, TPKeyboardAvoiding_keyboardWillShow is called 2 times, since TPKeyboardAvoidingScrollView add notification observer for UIKeyboardWillChangeFrameNotification ([[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(TPKeyboardAvoiding_keyboardWillShow:) name:UIKeyboardWillChangeFrameNotification object:nil]; in TPKeyboardAvoidingScrollView.m line 18)

so I just fixed that by replacing UIKeyboardWillChangeFrameNotification with UIKeyboardWillShowNotification, then TPKeyboardAvoiding_keyboardWillShow will only be called once when keyboard disappearing.

I am not sure if that will lead any other problems.

Plus I also suspect -(CGFloat)TPKeyboardAvoiding_idealOffsetForView:(UIView *)view withViewingAreaHeight:(CGFloat)viewAreaHeight calculates a wrong offset, cause there is a button above the scroll view at the bottom of the screen, which is not a subview of scroll (i.e. a superview has two subviews, which are TPKeyboardAvoidingScrollView and UIbutton. The uibutton is above TPKeyboardAvoidingScrollView in view hierarchy)

michaeltyson commented 9 years ago

Hmm, looks like @jrmiddle changed this to WillChangeFrame rather than WillShow in SHA 01090ae54795... can you comment, @jrmiddle ?

jrmiddle commented 9 years ago

The rationale behind the change to WillChangeFrame was that we needed to handle frame changes such as expanding/collapsing the iOS8 autocorrect accessory.

I spent some time bisecting, and found that:

But UITableViewController automatically automatically manages contentInsets for keyboard appearance, so it looks like what's happening is that if the tableview is hosted inside a UITVC, then the contentInset manipulation is being compounded.

This is readily visible when running with the changes in the sample app. If you comment out all of the -[NSNotificationCenter addObserver:selector:name:object] lines in TPKeyboardAvoidingScrollView and run the sample, the problem goes away. I suspect that I tested 01090ae in the context of a tableView being hosted by a plain UIVC, and not in the sample app, so I didn't catch this problem. Very sorry about that.

One solution would be to make optional whether TPKeyboardAvoidingTableView should pay attention to keyboard notifications or not. Since most people will probably be using it inside a UITVC, the default should probably be NO. UITVC does not give you tap-out for free, so that will still be useful in any case.

screen shot 2015-05-09 at 9 51 17 am

michaeltyson commented 9 years ago

Ah, so many special cases! No worries with missing this; I missed it too! Looks like an iOS update (would love to know which one...) has changed UITableViewController's behaviour, as you say - looks like it doesn't even need this class any more, as it's doing keyboard avoiding all by itself. Not UICollectionView, though, interestingly. Thanks for the consistent approach, Apple ;-)

So I guess TPKeyboardAvoidingTableView is only useful for its dismiss-on-tap feature, as you mentioned.

Perhaps the way to sort this out is to figure out what specific iOS version introduced the auto-keyboard-avoid behaviour for UITableViewController, then add some stuff in TPKeyboardAvoidingTableView that only watches for those keyboard notifications if it either (a) doesn't have a delegate that's a UITableViewController, or (b) is running on an iOS version prior to the one that introduces the auto-keyboard-avoiding.

Anyone feel like taking care of that and doing a pull request? ;-)

jrmiddle commented 9 years ago

I think using the tableview's delegate class to determine behavior would be problematic. The delegate and the hosting controller class are only usually the same by coincidence. Taking the class of the delegate as a proxy for the class of the hosting controller would lead to unexpected behavior for those who split those responsibilities. It's probably not common, but there's nothing contractually tying the two together. It would probably be wise not to change that relationship.

For tapping out, -[UIView endEditing] can handle the responder resignation, so I think all one needs to do is place a tap recognizer on the tableview itself and enable it whenever the keyboard is showing (this also handles arbitrary nesting of UITextViews/UITextFields). endEditing has been around since the early days, and it's always worked well for me, but there may be edge cases that make alternate methods necessary.

Given this, I think one could argue that using TPKeyboardAvoidingTableView in post-iOS6 adds quite a bit of code just to solve tap-out. Doing it manually is still a minor pain however (you still have to pay attention to keyboard show/hide events), so another strategy might be to refactor it into a TPTapoutTableView, simplify the responder handling, and have TPKeyboardAvoidingTableView subclass that. The former would be usable everywhere, the latter provided for older versions, and discretion regarding which to use being left up to the developer.

For pre-iOS6, TPKeyboardAvoidingTableView prior to 01090ae seems like the right way to go. If you really wanted to, you could #ifdef the TPKATV code so that it essentially no-ops on later OSes.

All of that said, this isn't something to which I'll be able to dedicate time anytime soon. I definitely on't be offended if you want to revert the TPKeyboardAvoidingTableView portion of 01090ae, as obviously it's problematic. 8914c9a should probably be looked at again as well.