andreamazz / AMScrollingNavbar

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

Add option for pre-v5.1.0 custom followers behaviour #353

Open lpbas opened 5 years ago

lpbas commented 5 years ago

Is your feature request related to a problem? Please describe. I am using this great library for my app, and before v5.1.0 (5.0.4 and below), custom followers used to move by the amount of their height. Since 5.1.0, the custom followers now move all the way until they are off the screen. (which is great for most views that are added as followers, with some exceptions)

Describe the solution you'd like I've been using this library to hide my NavBar and TabBar, and above my TabBar there is an advertisement (banner) which has a height of 50. With the previous version of the library, the ad moved down, along with the TabBar and then it stayed at the bottom of the screen, where the TabBar normally is. With the new version, the ad gets hidden, out of the screen bounds, along with the TabBar. This is not the behaviour I desire. Would it be possible to add an option to the followers, to use the "new" or the "old" following method? (like the option we have for the direction of the followers)

Describe alternatives you've considered For now I'm still using v5.0.4 of the library, to keep the functionality I need, but I would like to be able to use the latest features and bug fixes, as well as the Swift 4.2 version of the library.

Thank you very much for your help and your work.

andreamazz commented 5 years ago

Hey @L4grange can't you work around this by adding the banner above the tab bar? If the tab gets out of the way, the banner should remain on screen. Adding a switch to keep the old version or the new is not feasible. I can maybe add an optional offset to the follower, but I'd prefer to make sure that it'actually needed (i.e.: it's not something that can be remedied with autolayout)

lpbas commented 5 years ago

I have my banner constrained to the Safe Area (it does not know that it's VC is inside a TabBarController) and when I don't make the Banner follow the AMScrollingNavBar, it stays where it was, leaving an empty space where the TabBar was, instead of sticking with the TabBar that is moving downwards.

I will try tomorrow again to reset the constraints and make the banner not follow the scrollview and see if auto layout takes care of the movement, with the new version of the library and report back.

I fully understand your concerns about adding this behaviour, and I do agree that the new behaviour is how custom followers should behave, but the moving banner is unfortunately a requirement and I need make this behaviour possible.

I will let you know if I managed to get the desired result with just autolayout or not. Thank you for your consideration.

lpbas commented 5 years ago

I've tested all the possiblities and indeed, the Banner does not move down, along with the TabBar, unless I add it as a follower, meaning that the optional offset you suggested would be a good solution to what I'm trying to achieve. Thank you for your time.

andreamazz commented 5 years ago

Hey @L4grange Ok, sounds good, I'll try to implement it in the next few days. I'm a bit swamped at the moment, so a PR is more than welcome

andreamazz commented 5 years ago

Ok, I'm doing some digging, it looks like the behavior didn't really change between those two versions. The thing that changes is how the safe area is handled, i.e: I use the view's height and add the safe area inset to shove the view out of the way.

lpbas commented 5 years ago

Thank you for taking the time to check this! So how can we go about solving this? Is adding an offset a viable option?

andreamazz commented 5 years ago

Mh, it should, but I'm having issues with the implementation. I think I made a semantical mistake in treating the followers. scrollUp is always visible, scrollDown gets out of the screen. I'm confused by my own API, that's not good. Anyway, while I solve my moral dilemma, checkout the branch follower-offset, there's a crude implementation of the follower offset. I briefly tested it with the tabbar and it works, let me know if that helps in your setup.

lpbas commented 5 years ago

Thank you so much @andreamazz I've tested the branch and using NavigationBarFollower(view: bannerView, direction: .scrollDown, offset: tabBarController?.tabBar.frame.height ?? 0) I get the behaviour I need, which was present in older versions of the library.

I'm sorry about the API, I have not taken a look inside the library. I hope you figure it out soon. Let me know when you have any updates or if there is anything I can help you with.