Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Navbar expanding #116

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Navbar expanding #116

wants to merge 6 commits into from

Conversation

killev
Copy link

@killev killev commented Jan 29, 2016

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.
2. Facebok's Navbar shifts it's scrollview on expanding/contracting completion. I've implemented that.
3. 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
Copy link
Contributor

Mazyod commented Jan 30, 2016

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

Mazyod commented Jan 30, 2016

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

Mazyod commented Feb 2, 2016

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

@Mazyod Mazyod added the pending label Feb 2, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants