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

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

Open
lpbas opened this issue Nov 1, 2018 · 8 comments
Open

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

lpbas opened this issue Nov 1, 2018 · 8 comments

Comments

@lpbas
Copy link

lpbas commented Nov 1, 2018

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
Copy link
Owner

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
Copy link
Author

lpbas commented Nov 5, 2018

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
Copy link
Author

lpbas commented Nov 6, 2018

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

lpbas commented Nov 28, 2018

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
Copy link
Owner

andreamazz commented Nov 28, 2018

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
Copy link
Author

lpbas commented Nov 28, 2018

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.

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

No branches or pull requests

2 participants