search

andreamazz / AMScrollingNavbar

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

Scroll to Top is not exact correct #377

Closed alexookah closed 4 years ago

alexookah commented 4 years ago

A clear and concise description of what the bug is. I tried to tap on the status bar to initiate scroll to top and it seems that the navbar is showing but the scrollview is not going exactly to the top.

check the following video: https://streamable.com/5osha

this line is responsible for the scrolling to top: https://github.com/andreamazz/AMScrollingNavbar/blob/master/Source/ScrollingNavigationController.swift#L263

maybe this has to happen after the navbar has been shown?

similar issue: https://github.com/andreamazz/AMScrollingNavbar/issues/363

andreamazz commented 4 years ago

Hey @Alexookah check out master for a fix. I've added scrollToTop to the showNavbar call, optional, default set to false

alexookah commented 4 years ago

@andreamazz It works perfect when there are no followers. With follower it needs the height of followers. How to calculate that?

alexookah commented 4 years ago

@andreamazz You forgot to handle the scrollToTop inside the function ?

i tried this for one follower which seems ok let size = CGPoint(x: 0, y: -(self.followers.first?.view?.frame.height ?? 0))

maybe create a computer variable that returns for all followers the height?

andreamazz commented 4 years ago

Right, added to master the followers height and fixed the missing check: https://github.com/andreamazz/AMScrollingNavbar/commit/7744693a78f36d1cfb9eb35407b1f5adf7ac98a6

andreamazz commented 4 years ago

Pushed a minor update that filters the followers:

let followersHeight = self.followers.filter { $0.direction == .scrollUp }.compactMap { $0.view?.frame.height }.reduce(0, +)
alexookah commented 4 years ago

nice @andreamazz great job closing the issue

alexookah commented 4 years ago

@andreamazz Sorry for all the fuzz. After checking with my configuration i had to add an additionalScrollToTopOffset made a pull request for this.

https://github.com/andreamazz/AMScrollingNavbar/pull/378 hope its not too late.

alexookah commented 4 years ago

PL merged Thanks so much @andreamazz