andreamazz / AMScrollingNavbar

Scrollable UINavigationBar that follows the scrolling of a UIScrollView
MIT License
6.05k stars 634 forks source link

scroll to top doesn't work correctly #389

Open arashetm opened 4 years ago

arashetm commented 4 years ago

Describe the bug scroll to the top doesn't work correctly.

To Reproduce create a tableView including too many items. scroll to the middle of tableView and then call this:

if let navigationController = self.navigationController as? ScrollingNavigationController {
    navigationController.showNavbar(animated: true, duration: 0.2, scrollToTop: true)
}

It scrolls to top but not actually to top of tableView.

andreamazz commented 4 years ago

See #377 and #378

arashetm commented 4 years ago

Didn't help me! It doesn't work properly. Actually I call the showNavbar method in the action of segmentValueChanged.

andreamazz commented 4 years ago

Hi @arashetm I'm testing against the Demo project, attached the segment action like so:

  @IBAction func segmentChange() {
    if let navigationController = self.navigationController as? ScrollingNavigationController {
      navigationController.showNavbar(animated: true, duration: 0.2, scrollToTop: true)
    }
  }

and it seems to be working fine. Can you share a sample project showing the issue?

arashetm commented 4 years ago

I figured out what's the problem! If I want to scroll to the top without changing segment it works like charm, but if I want to update my table view data source alongside scrolling to the top it doesn't work properly. Here is my code:

@IBAction func segmentValueChanged(_ sender: UISegmentedControl) {
    _ = self.scrollViewShouldScrollToTop(self.archiveTableView)
    self.setBannersFrame()        //setting table header view
    // change table view's data based on the selected segment
    self.reloadArchiveTableView() // reloading table view
}

I found a hacky slution for this::

@IBAction func segmentValueChanged(_ sender: UISegmentedControl) {
    _ = self.scrollViewShouldScrollToTop(self.archiveTableView)
    self.setBannersFrame()        //setting table header view
    // change table view's data based on the selected segment
    DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
        self.reloadArchiveTableView() // reloading table view
    }
}

Maybe you need to add completion handler for showNavbar method.

andreamazz commented 4 years ago

Sounds good, checkout the latest master commit for the completion handler in showNavbar

arashetm commented 4 years ago

It didn't work ...

andreamazz commented 4 years ago

I'm afraid it's still triggered too early, before UIKit rearranges the table

arashetm commented 4 years ago

Do you have any suggestions? Reloading should start exactly right after scroll did end scrolling, could we use scrollViewDidEndScrollingAnimation? I need to achieve this.

andreamazz commented 4 years ago

Checkout the updated demo in master, the tableview now scrolls to top and updates the data source, and it seems to be working. Maybe it helps, let me know https://github.com/andreamazz/AMScrollingNavbar/commit/b280a886d0e51d43868a979c1a9f0866edae9e7d#diff-42fa19733453bd36493bea5901519e2d

andreamazz commented 4 years ago

Any feedback?

uzman commented 2 years ago

you can fix it by calling collectionview's scrolltoItem method in the completion block:

    open func scrollViewShouldScrollToTop(_ scrollView: UIScrollView) -> Bool {
        if let navigationController = self.navigationController as? ScrollingNavigationController {
            navigationController.showNavbar(animated: false, duration: 0.1, scrollToTop: false) {
                DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) {
                    self.collectionView.scrollToItem(at: IndexPath(row: 0, section: 0), at: .top, animated: true)
                }
            }
        }
        return true
    }