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

Broken in XCode 9 + iOS 11 #76

Closed
nickdnk opened this issue Sep 24, 2017 · 7 comments
Closed

Broken in XCode 9 + iOS 11 #76

nickdnk opened this issue Sep 24, 2017 · 7 comments

Comments

@nickdnk
Copy link

nickdnk commented Sep 24, 2017

Report

After updating XCode to 9.x this library seems to be broken. I add a .top refresher and experience the progress let starting at 0.6 when you reach the top of the scroll view and start pulling. Aside from that it is buggy in general when pulling down and releasing. The content insets no longer match when it returns to a normal state.

I should mention this is with a Custom Animation, not the default one, so the demo project still seems to work. Also it's within a UITableViewController, not a UIViewController with a UITableView inside it as in the demo project.

I previously submitted a bug report where I manage to fix some of this weird behaviour with automaticallyAdjustsScrollViewInsets and contentInsets, but it doesn't work anymore. I think Apple may have changed some logic with UIEdgeInsets for UITableViewControllers. I can't really make it work and I'm stuck. Currently my top inset logs out as 0 even though it's clearly 64 (height of navigation bar). I think this messes with all the calculations the library does.

Your Environment

  • Version of the component: 2.0.2
  • Swift version: 3
  • iOS version: 11
  • Device: iPhone 6S
  • Xcode version: 9
  • If you use Cocoapods: _run pod env | pbcopy and insert here:

Stack

   CocoaPods : 1.2.1
        Ruby : ruby 2.0.0p648 (2015-12-16 revision 53162) [universal.x86_64-darwin16]
    RubyGems : 2.0.14.1
        Host : Mac OS X 10.12.6 (16G29)
       Xcode : 9.0 (9A235)
         Git : git version 2.13.5 (Apple Git-94)
Ruby lib dir : /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib
Repositories : master - https://github.com/CocoaPods/Specs.git @ ae7c17ebfbc1d62bdd7f9b52982ea9384e28f173

Installation Source

Executable Path: /usr/local/bin/pod

Plugins

cocoapods-deintegrate : 1.0.1
cocoapods-plugins     : 1.0.0
cocoapods-search      : 1.0.0
cocoapods-stats       : 1.0.0
cocoapods-trunk       : 1.2.0
cocoapods-try         : 1.1.0

Podfile

# Uncomment this line to define a global platform for your project
platform :ios, '9.0'

target 'nyx-public' do
  # Comment this line if you're not using Swift and don't want to use dynamic frameworks
  use_frameworks!

  # Pods for nyx-public
  pod 'PullToRefresher'   
  pod 'Firebase/Messaging'
  pod 'Firebase/Core'
  pod 'SwiftSpinner'
  pod 'FontAwesome.swift'  
  pod 'FacebookCore'
  pod 'FacebookLogin'
  pod 'KeychainAccess'
  pod 'Alamofire'
  pod 'QRCode'
  pod 'SwiftyJSON'
  pod 'AFNetworking'
  pod 'RoundedSwitch'

  pod 'Spring', :git => 'https://github.com/MengTo/Spring.git', :branch => 'swift3'
  target 'nyx-publicTests' do
    inherit! :search_paths
    # Pods for testing
  end

end

Project that demonstrates the bug

Please add a link to a project we can download that reproduces the bug.

I can't. I don't have anything that's open source.

@nickdnk
Copy link
Author

nickdnk commented Sep 25, 2017

Okay. I did some testing.

I made it work by first of all implementing #75.

Then I created a var to hold the height of the navigation and status bar:

open var navigationControllerHeight: CGFloat = 0

Initially 0 to make it possible to just enable my "fix" by setting:

awesomeRefresher.navigationControllerHeight = (self.navigationController?.navigationBar.frame.size.height)! + UIApplication.shared.statusBarFrame.height

This way it works on both iPhone X and all the other ones, since the new iPhone has a different status bar.

I then use this variable here, adding it to the value of self.scrollViewDefaultInsets (which are 0):

func animateFinishedState() {
        removeScrollViewObserving()
        UIView.animate(
            withDuration: animationDuration,
            delay: hideDelay,
            usingSpringWithDamping: springDamping,
            initialSpringVelocity: initialSpringVelocity,
            options: animationOptions,
            animations: {
                self.scrollView?.contentInset = self.scrollViewDefaultInsets
                if case .top = self.position {
                    self.scrollView?.contentOffset.y = -(self.scrollViewDefaultInsets.top + self.navigationControllerHeight)
                }
        },
            completion: { _ in
                self.addScrollViewObserving()
                self.state = .initial
        }
        )
    }

And here, the same logic:

override open func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
        if (context == &PullToRefresh.KVOContext && keyPath == contentOffsetKeyPath && object as? UIScrollView == scrollView) {
            var offset: CGFloat
            switch position {
            case .top:
                offset = previousScrollViewOffset.y + scrollViewDefaultInsets.top + self.navigationControllerHeight ....

Because it seems the ContentInsets are always 0 when used inside a UITableViewController. Note that I still do:

self.automaticallyAdjustsScrollViewInsets = false
self.tableView.contentInset = UIEdgeInsetsMake(0, 0, 0, 0)
self.tableView.scrollIndicatorInsets = UIEdgeInsetsMake(0, 0, 0, 0)

To make it work.

If anyone has any idea how to properly implement this solution feel free to pitch in! I created a pull request that implements these changes and does so without breaking anything else: #77 It works on both iOS 11 and 10.

@srj0x0
Copy link

srj0x0 commented Nov 6, 2017

@nickdnk Hi! Thank for your report. Starting from iOS 11automaticallyAdjustsScrollViewInsets is deprecated. Although, you have a something new in UIScrollView: contentInsetAdjustmentBehavior, adjustedContentInset and adjustedContentInsetDidChange() method. So you must to set contentInsetAdjustmentBehavior = .never instead of automaticallyAdjustsScrollViewInsets = false and manually manage for your table view insets.

@nickdnk
Copy link
Author

nickdnk commented Mar 26, 2018

Hey @SerjOxo
Sorry I didn't get back on this. I just tried updating to 3.x and it's still very broken inside a UITableViewController. My own forked version still works fine for my use case so I'm using that for now.

I'll have a look at those news properties you mentioned and see if I can get it to work.

@nickdnk
Copy link
Author

nickdnk commented Apr 1, 2018

I cannot get 3.0 to work at all. Did you try to use it with a UITableViewController?

@nickdnk
Copy link
Author

nickdnk commented Oct 17, 2018

Hello

Still doesn't work inside UITableViewController on 3.0.2. Any news on this problem? I don't know what information you need from me to fix it (if you want to or can at all).

Let me know.

@sergey-prikhodko
Copy link
Contributor

Hi @nickdnk!

Does it work correctly on the recent version?

@nickdnk
Copy link
Author

nickdnk commented Sep 3, 2020

Hello
Sorry for the "delay". Our focus has been elsewhere but I now need to update our app, which I cannot as the custom fork of the project I used only works in Swift 3 which is no longer supported.

It still does not work as expected. I now suffer from #124. Again on a UITableViewController subclass.

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

No branches or pull requests

3 participants