Add 'menu-left-open' and 'menu-right-open' classes

[rvanbaalen]

When opening a left side menu or right side menu, toggle a 'menu-right-open' or 'menu-left-open' class on the ion-side-menu-content element.

This allows for custom styling of the content pane (for example; show a dark overlay) when one of the menus is open.

[rvanbaalen]

Not sure what you mean by that @adamdbradley. Could you perhaps explain that with a small snippet?

The reason for the PR originated when I wanted to show a dark overlay over my content pane when one of the menu's is opened. At this point, without the PR, there was no point to do this with pure CSS.

[rvanbaalen]

@adamdbradley I think I understand your question now. If I'm not mistaken, you assume that these dynamic classes are applied to the side menu's itself? That wouldn't make any sense indeed.

Instead, they are applied to the ion-side-menu-content element which slided to the left/right when the menu opened.

[adamdbradley]

Sure if they are applied to the menu content then yes I see its value. Before I merge this in could you write some tests first to make sure they're being added and removed correctly? Thanks.

[rvanbaalen]

@adamdbradley Sure. Since I'm not that familiar with the tests in Ionic, could you point me to the file where I should add the test?
I can't find that many tests; the closest thing I found was this:
https://github.com/rvanbaalen/ionic/blob/patch-2/test/unit/views/sideMenu.unit.js

[ajoslin]

Our test structure right now is super confusing; we're amending that soon. It's here: https://github.com/rvanbaalen/ionic/blob/patch-2/js/ext/angular/test/directive/ionicSideMenu.unit.js

[ajoslin]

Although this isn't actually the right place to be adding a class. You'll be manipulating the DOM every time setTranslateX is called.

I would do it in js/controllers/sideMenuController.js, openPercentage function. Add it to this.content.el - but be sure to do an if (this.content) around it.

[rvanbaalen]

👍 I'm on it.

[rvanbaalen]

@ajoslin There is no this.content.el from what I can see this quick.

[ajoslin]

this.content is an instance of js/views/sideMenuView.js

[rvanbaalen]

I noticed that and I see that this.el is being used within the sideMenuView but it doesn't seem to be a public property. This is what I get when I console.log(this.content, this.content.el, percentage) from within openPercentage():

Notice the undefined and no el property on this.content

[ajoslin]

I see. You're right.

js/views/sideMenuView.js is actually not used! The side menu code is weird because it started without angular and we 'patched' angular on to the code.

The content is set through sideMenuCtrl.setContent() in ionicSideMenu directive.

Umm, you could just add an 'el: $element[0]' field on line 295 of directives/ionicSideMenu.js

[rvanbaalen]

Pretty new to the whole test writing thing. Am I doing it right @ajoslin? At least I did get all the proper code in place to add / remove the classes. Just aren't able to write the tests (yet).

[rvanbaalen]

@ajoslin @adamdbradley Nevermind this pull request. I've found a better way to achieve this goal without un-needed DOM manipulation by adding classes that might never be used.

This is how I archieve my goal now:

<ion-content
    scroll="true"
    class="has-header"
    ng-class="{
        'has-loading': loadingMessage,
        'menu-open': $ionicSideMenuDelegate.isOpenLeft() || $ionicSideMenuDelegate.isOpenRight()
    }">

Perhaps it would be nice to have a $ionicSideMenuDelegate.isOpen() which returns true if any of the side menus are open.

[ajoslin]

@rvanbaalen cool! Thanks for trying. I was going to say, the way you were doing actually wouldn't have worked in all cases now that I thought about it - since when the user is dragging the side menu, openPercentage isn't actually called.

The way to do it (if we were going to do it) would be to setup a $watch in the ionSideMenuContent directive:

$scope.$watch(function() { 
  return sideMenuCtrl._leftShowing; 
}, function(isLeft) {
  $element.toggleClass('menu-left-open', !!isLeft);
});

Then the same for right.

This would be super easy to unit test, too.

Sorry for the runaround! Glad you figured it out.

[rvanbaalen]

Opened two new related pull requests:
#1074
#1075