andreamazz / AMScrollingNavbar

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

Sticky Followers Handling #380

Closed alexookah closed 4 years ago

alexookah commented 4 years ago

Added the sticky followers request https://github.com/andreamazz/AMScrollingNavbar/issues/291

See in video how it works https://streamable.com/3dhtj

Made a branch with the example as shown from the video: https://github.com/Alexookah/AMScrollingNavbar/tree/sticky-followers (simply do pod install to get my repo's master branch code)

This is tested using: UINavigationBar.appearance().isTranslucent = false

Currently its pretty simple handling. Each follower is checking the previous followers height. This could be changed into some more customizable setup with some followers to be fully hidden or not.

I think this might be useful for some people like i needed this :)

Hope my implementation is good enough for merge. Feel free to tell me if something needs to be changed.

alexookah commented 4 years ago

@andreamazz any feedback? what do you think?

andreamazz commented 4 years ago

Hi @Alexookah Thanks for the contribution. I'm not sure I fully understand the feature though. Why does the second view only partially hide? Also this doesn't fully work with a translucent navbar, there's a gap between the bar and the followers.

alexookah commented 4 years ago

Hello @andreamazz Well the feature works only for non translucent navbars where the followers hide. (I am not sure how this could be used in translucent navbars)

check this video: https://streamable.com/zjv2g

The first follower is smaller and the second follower is cut when collapsing. This probably works only when the first follower is small enough to hide. Maybe this could be changed somehow.

andreamazz commented 4 years ago

I'm testing against your branch, this config:

        [NavigationBarFollower(view: toolbar, direction: .scrollUp),
         NavigationBarFollower(view: secondView, isSticky: true, direction: .scrollUp),
         NavigationBarFollower(view: thirdView, isSticky: false, direction: .scrollUp)]

and this:

        [NavigationBarFollower(view: toolbar, direction: .scrollUp),
         NavigationBarFollower(view: secondView, isSticky: false, direction: .scrollUp),
         NavigationBarFollower(view: thirdView, isSticky: false, direction: .scrollUp)]

seem to work the same. As you suggested maybe it has to do with the previous view.
What is the intended behaviour though? When sticky is true, the follower never hides, and when false it does?

alexookah commented 4 years ago

try cleaning building folder in my branch https://github.com/Alexookah/AMScrollingNavbar/tree/sticky-followers

Yes that is the intended behaviour

andreamazz commented 4 years ago

Yep, I'm trying a clean build of that branch, I see the same behaviour

alexookah commented 4 years ago

try also pod install from: pod 'AMScrollingNavbar', :git => 'https://github.com/Alexookah/AMScrollingNavbar.git', :branch => 'master' thats where i have the Source changes

andreamazz commented 4 years ago

I am, weird

alexookah commented 4 years ago

Sorry maybe i made a mess using two branches one for test and one for the pull Request. Did you eventually run my implementation?

alexookah commented 4 years ago

now that i am checking both branches have the sticky follower code

andreamazz commented 4 years ago

Yep, the code is there

alexookah commented 4 years ago

Feel free to add or change anything

alexookah commented 4 years ago

Sorry to ask again. Did you run it eventually? or did i do something wrong? What do you think about merging this? or is this too specific?

andreamazz commented 4 years ago

I'm running it, but false or true lead to the same behaviour: https://d.pr/i/k1PMpu

I'd be happy to merge if there was support for translucent and a bit of documentation, as you mentioned it might be a bit too specific, so that option needs to be explained.

alexookah commented 4 years ago

Ok i ll see how this could work for translucent.