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

Fixing custom contentInsets #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixing custom contentInsets #16

wants to merge 1 commit into from

Conversation

fjcaetano
Copy link

Showing and hiding the keyboard breaks custom contentInsets for the tableView.

@datwelk
Copy link
Owner

datwelk commented Jul 24, 2014

I haven't tested myself yet but are you sure this works? It doesn't seem to be a good idea to increment the contentInset because this will cause the contentInset to grow indefinitely when the method is called over and over again.

@fjcaetano
Copy link
Author

I understand your skepticism, but it is working. You are forgetting that I also changed the multiplier for the bottomInset:

bottomInset *= RDRKeyboardIsFullyHidden(keyboardFrame) ? -1 : 1;

The problem with the current implementation is that you add to the contentInset the height of the keyboard, but when it's dismissed, you don't subtract it back. You are setting something based on the size of the view, disregarding that it may have a custom contentInset setted.

@thepelican
Copy link

I've tried as well, and as @datwelk was saying, it will make the contentInset to grow indefinetely

@datwelk
Copy link
Owner

datwelk commented Jul 25, 2014

Then I cannot merge this ^^. Maybe we can check scrollView's initial contentInset inside the initWithScrollView method of RDRStickyKeyboardView. This will however not work if the scrollView's contentInset is changed afterwards.

@fjcaetano
Copy link
Author

The project has no tests, but let me see if I can poke around more and try duplicating the issues you're mentioning.

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