John-Lluch / SWRevealViewController

A UIViewController subclass for presenting side view controllers inspired on the FaceBook and Wunderlist apps, done right !
Other
4.52k stars 989 forks source link

hidesBottomBarWhenPushed does not work with navigationControllers #13

Closed georgemp closed 11 years ago

georgemp commented 11 years ago

Hi, In my project I have a UINavigationController inside a tabBarController. On it's child controllers, I set childController.hidesBottomBarWhenPushed = YES; to hide the tab bar when pushing child controllers on the navigation stack. This works fine when the navigationController is the top controller in the tabBarController. However, this fails to work if the navigationController is set as the frontViewController of the SWRevealViewController and the revealViewController is set as the top controller in the tabBarController. Now, when pushing the child controllers in the navigationController, the tab bar continues to be displayed.

George

John-Lluch commented 11 years ago

The documentation of UINavigationController about hidesBottomBarWhenPushed says the following:

"A view controller added as a child of a navigation controller can display an optional toolbar at the bottom of the screen. The value of this property on the topmost view controller determines whether the toolbar is visible. If the value of this property is YES, the toolbar is hidden. If the value of this property is NO, the bar is visible."

Note that it does not mention tabBars but only toolbars. Note also that this is an api from iOS 2.0, i.e. before containment controllers existed as they are today. My guess is that it only works on tabBars for a mere compatibility reason, possibly only if a UITabBarController is the direct parent of the NavigationController. This is completely a hack, because if you think on it you will see that an operation performed on a child controller (in this case a push on the navigationController) causes a layout of the views of its parent (the tabBarController) by hiding its tabBar view with animation and by enlarging its containment view. Thus this breaks any regular layout of the view hierarchy.

So since this is an Apple internal implementation of something that magically happens we can't do anything about it, and even if we wanted to provide a custom implementation (by calling a method or whatever) we would have to mess with the internal views of a tabBarController, which we do not want because such an implementation could break any time in the future.

That's my view, if you have another idea, please let me know. I also assume that for toolbars contained in a NavigationController this api just works as expected.

georgemp commented 11 years ago

Thanks for looking into this - your comment above seems to make sense (I am still quite new to all of this). I deleted my older comment as there was a bug in my implementation. In case this helps anyone in a similar situation, I ended up subclassing the reveal controller and set the subclass to be a delegate of UINavigationController. On init and setting front view controller in the subclass, I set the reveal controller to be the nav controller delegate

- (id)initWithRearViewController:(UIViewController *)rearViewController frontViewController:(UIViewController *)frontViewController
{
    self = [super initWithRearViewController:rearViewController frontViewController:frontViewController];
    if (self) {
        if ([self.frontViewController isKindOfClass:[UINavigationController class]])
            ((UINavigationController *) self.frontViewController).delegate = self;
    }    

    return self;
}
- (void)setFrontViewController:(UIViewController *)frontViewController animated:(BOOL)animated
{
//Remove reveal controller as delegate of the current front view navigation controller
if ([self.frontViewController isKindOfClass:[UINavigationController class]])
    ((UINavigationController *) self.frontViewController).delegate = nil;

//Set reveal controller as delegate of new front view navigation controller
if ([frontViewController isKindOfClass:[UINavigationController class]])
    ((UINavigationController *) frontViewController).delegate = self;

//Call super implementation to actually set the controller
[super setFrontViewController:frontViewController animated:animated];
}

Then, in the subclasses navController delegate method, I hide and show the tab bar as needed

- (void)navigationController:(UINavigationController *)navigationController willShowViewController:(UIViewController *)viewController animated:(BOOL)animated
{
    UITabBarController *tabBarController = self.tabBarController;
    if (tabBarController != nil && viewController.hidesBottomBarWhenPushed) {
        [tabBarController hideTabBar];
    } else {
        [tabBarController showTabBar];
   }
}

The hide/show tabBar were implemented as category methods similar to the solution at http://stackoverflow.com/questions/5272290/how-to-hide-uitabbarcontroller

This seems to work (altho, the animation to hide/show tabBar seems a bit distracting). If there is a better solution, I would love to learn :)

John-Lluch commented 11 years ago

Well, containment view controllers are not generally meant to be subclassed, including the SWRevealViewController. Also, if I am allowed to give my opinion, the use you make of delegates is a no-no. Delegates are meant to be assigned just after initialization or at some time after it, and never changed again. Changing or setting a delegate of an object by a class that did not create the object, is highly dangerous. Of course, you control and know what you do in this case, but imagine if whatever class would be allowed to set the delegate of whatever object: It would convert code maintenance into a nightmare!.

If fact, I do not think that you need to use any delegates in this case, nor involving a subclass of SWRevealViewController. After all what you want to do is to modify the layout of a tabBarController when a custom controller is pushed on a NavigationController that is a child (or a grand child in this case) of said tabBarController. I can think of several approaches for this:

Approach 1: Subclass the UINavigationController to incorporate a hook to change the tabBarController, no delegate involved.

Approach 2: Subclass the UITabBarController and add a custom method to hide/show the tab bar. Call this method from the willAppear of your custom controller.

Approach 3: Create a helper class object to perform approach 2, but let this object to do it all instead of subclassing anything. Add this object to your custom controller, and invoke it from the willAppear method. I would go for this one because this approach does not involve subclassing or delegation.

Note that in any of these approaches the reveal controller is not involved at all, as it should work ok provided you perform the correct layout on its parent controller (the tabBarController). Still, the whole thing is a hack because you are messing with undocumented internal views and layout of a UITabBarController, so even if you can make it to work now, any implementation can break at any time in the future.

Regards,

georgemp commented 11 years ago

I was concerned about setting the delegate as well (but, went with it as in my case I didn't have a delegate set on the nav controller on initialization anyway). I am not sure of the viewWillAppear on child controller - a regular navigation controller already hides the tabBarController. So, if I put code inside viewWillAppear, I might have to detect when the navigation controller is inside a SWRevealViewController and then hide the tabBar (or hope that the code for hiding tab bar doesn't interfere with when the nav controller itself hides the tab bar - and there are no conflicts). At this point most solutions seem to be hacks of sorts...I will try playing around with this and the other approaches you mention as time permits. Thanks for the input and the time you have put into considering this problem :)

John-Lluch commented 11 years ago

No problem for helping, this is an interesting issue and it does not take a lot of time for me. Ultimately I will also learn form your feedback.

You can always check whether the immediate parent of your navigation controller is a tab bar controller. I presume this is the single case where Apple implements its magic with these two. Otherwise you can invoke your layout code.

I would avoid detecting the reveal controller in particular or checking for it because, as I understand it, it has nothing to do with the problem since after all any other containment controller that you would put in the middle would presumably cause the same problem.