search

telly / TLYShyNavBar

Unlike all those arrogant UINavigationBar, this one is shy and humble! Easily create auto-scrolling navigation bars!
MIT License
3.73k stars 427 forks source link

Navbar expanding #116

Open killev opened 8 years ago

killev commented 8 years ago

Hello,

Thank you for so beautiful library. It helped me a lot, but there are a few bug and behaviour inconsistencies:

  1. [Bug] extensionView is placed incorrectly after popping detalis from navigationController stack as long as you change navigationBar visibility during view's transiotion.

I'm hiding/showing navigation bar via setNavigationBarHidden when pushing/poping details vew. And at that moment line:

[superview convertPoint:maxEdge fromView:self.view] is working incorrectly.

So I've replaced it gently.

  1. Facebok's Navbar shifts it's scrollview on expanding/contracting completion. I've implemented that.
  2. Facebook's Navbar shifts it's scrollview on expanding after view's shown. I've implemented that.

Also there was strange code that created weak pointer without assignation: __weak __typeof(self) weakSelf; I've changed it by: __weak __typeof(self) weakSelf = self;

Maybe I miss something in objective-c?

Hope my changes will help you a bit.

Regards, Peter

Mazyod commented 8 years ago

Wow, thanks for the thorough PR. Your note about the weakSelf is very true, you're not missing anything. That was a bug, thanks for fixing it.

I will need to test this thoroughly as well, since it affects the current behavior for many users, which I need to make sure it doesn't cause any surprises for them.

I'll be testing it right away.

Mazyod commented 8 years ago

In my initial testing, everything seems OK except when you start scrolling down and stop when the navigation bar is half way visible, the content will be scrolled up twice while hiding the navbar.

shynavbar

Also, I am really wondering if we need all these changes to implement the points you mentioned? I feel there are too many changes, and I am worried about changing the behavior too much. Although, I will have to test even more to make sure all the changes are needed. (for example, the change in viewWillDisappear: related to swizzling)

Mazyod commented 8 years ago

Thanks for making the fix, I'll need to review this again