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

AutoInsetter - Update contentOffset unexpectedly #300

Closed
lowip opened this issue Jun 14, 2018 · 3 comments
Closed

AutoInsetter - Update contentOffset unexpectedly #300

lowip opened this issue Jun 14, 2018 · 3 comments
Labels

Comments

@lowip
Copy link

lowip commented Jun 14, 2018

New Issue Checklist

Issue Description

I have a TabmanViewController with the currentViewController having prefersStatusBarHidden set to true.
When selecting one of the cells in my currentViewController, a modal view controller is presented with prefersStatusBarHidden set to false.

This triggers a viewDidLayoutSubviews() -> setNeedsChildAutoInsetUpdate() -> autoInsetter.inset(...)

It goes in here: https://git.io/vhiyH

if scrollView.contentInset != requiredContentInset {
                  
  let isTopInsetChanged = requiredContentInset.top != scrollView.contentInset.top
  
  scrollView.contentInset = requiredContentInset
  scrollView.scrollIndicatorInsets = requiredContentInset
                  
  // only update contentOffset if the top contentInset has updated.
  if isTopInsetChanged {
      var contentOffset = scrollView.contentOffset
      contentOffset.y = -requiredContentInset.top // ⚠️We lose the current content offset here
      scrollView.contentOffset = contentOffset
  }
}

As you can see, the scrollView.contentOffset.y is updated regardless of the current contentOffset.

I propose the following change:

if scrollView.contentInset != requiredContentInset {
                
  let isTopInsetChanged = requiredContentInset.top != scrollView.contentInset.top
  let topInsetDelta = requiredContentInset.top - scrollView.contentInset.top
  
  scrollView.contentInset = requiredContentInset
  scrollView.scrollIndicatorInsets = requiredContentInset
                  
  // only update contentOffset if the top contentInset has updated.
  if isTopInsetChanged {
      var contentOffset = scrollView.contentOffset
      contentOffset.y -= topInsetDelta
      scrollView.contentOffset = contentOffset
  }
}

Which updates the contentOffset to the delta of the inset change.

I can prepare a pull request if need be.

Other useful things

A video of the issue: https://gfycat.com/PleasedIcyBichonfrise

@msaps
Copy link
Member

msaps commented Jun 14, 2018

@lowip great find! And thanks so much for the detailed report/fix.

I've opened a PR in AutoInsetter and will try to get a release done ASAP 🥇

@msaps
Copy link
Member

msaps commented Jun 14, 2018

@lowip released AutoInsetter 1.2.4 😄

Sent with GitHawk

@lowip
Copy link
Author

lowip commented Jun 14, 2018

Great! Thanks for your reactivity, and your suite of libraries :)

@msaps msaps closed this as completed Jun 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants