Simplifying TopHeaderNavigationController

[kyle-fox]

Used Pop to animate the BackgroundView's layer - helped clean up the logic a lot. Let me know how well this works on a device - I don't have the provisioning to run it on mine. Hopefully Pop works as smoothly as your native animations were working before and if not, feel free to close this!

[phillipthelen]

I'm not sure if the merging of hide and show into toggle will cause problems. I remember that i had it that way at first but changed it to separate methods because the app could get stuck in a state where the header display status is inverted (so all views that should have had it didn't have it and all that shouldn't have had it had it.) explicitly calling hide and show prevented that. I couldn't reproduce it in the simulator though, but will test again tomorrow on a device. But switching to POP is good!
About the provisioning profile: since the last wwdc apple allows you to deploy to your own device with a free apple developer account too. That should help that you can also test on your device :)

[kyle-fox]

Ya you were sometimes doing your own checks whether the top bar was hidden and then making explicit calls to hide or show. It looks like that all would be redundant, since then check whether it's showing or not is happening inside of toggle. I didn't run into any problems while testing after making the switch, but I can easily change it back to explicit hide and show calls, if you'd prefer that.

And I have my own account - I just wasn't thinking and immediately assumed I needed to be on this profile. Whoops

[phillipthelen]

Hmm. I'll go through it again. Maybe my solution really was redundant.

Aah. :D no. Changing the team in the app settings should be enough to let you build for a device

[phillipthelen]

Tested it on device. when you use the swipe gesture to go back, but only pull it a little bit and then let go, it toggles, but stays in the same viewcontroller. Causing the display status of the header to be inverted. So explicitly showing and hiding should be the better solution. But you are right. checking if the view actually is hidden is kind of redundant at that point.
Other than that the changes make sense and if you could split up the toggle into two methods, I'll merge it in.

[kyle-fox]

The display status correctly changes even when doing the half swipe as you mentioned. Add NSLog((self.isTopHeaderVisible) ? @"YES" : @"NO"); under line 90 in HRPGTopHeaderNavigationController.m when testing to verify that the correct showing status is being maintained. Looks right to me.

[phillipthelen]

So i was able to reproduce it in the simular too.

It is rather unlikely that it will happen, but the only way it gets fixed again is if the user kills the app. Which isn't ideal.

[kyle-fox]

Good catch, and thanks for the gif! I currently toggle the self.isTopHeaderVisible bool in the animation's completion block, and you're doing the swipeback thing before the animation completes. In this case, toggling the self.isTopHeaderVisible should probably happen immediately, since we use that value to set the value to which the view is animating, and that should solve the issue.

[kyle-fox]

After making this change, I can no longer recreate the issue in the gif above. Let me know what you think!

[phillipthelen]

Issue seems resolved. Nice job!

👍

@crookedneighbor 16fa5d4d-6729-4a7d-af3e-26a25850b5b8

[crookedneighbor]

Good work. Tier awarded!