fix(menu): setOpen must be coordinated by the controller

Author: manucorporat

packages/core/src/components/menu-controller/menu-controller.ts

@@ -1,5 +1,5 @@
-import { Animation, AnimationBuilder, AnimationController, Menu } from '../../index';
 import { Component, Method, Prop } from '@stencil/core';
+import { Animation, AnimationBuilder, Menu } from '../../index';
 
 import MenuOverlayAnimation from './animations/overlay';
 import MenuPushAnimation from './animations/push';
@@ -11,9 +11,9 @@ import MenuRevealAnimation from './animations/reveal';
 export class MenuController {
 
   private menus: Menu[] = [];
-  private menuAnimations: { [name: string]: AnimationBuilder } = {};
+  private menuAnimations = new Map<string, AnimationBuilder>();
 
-  @Prop({ connect: 'ion-animation-controller' }) animationCtrl: AnimationController;
+  @Prop({ connect: 'ion-animation-controller' }) animationCtrl: HTMLIonAnimationControllerElement;
 
   constructor() {
     this.registerAnimation('reveal', MenuRevealAnimation);
@@ -29,11 +29,7 @@ export class MenuController {
   @Method()
   open(menuId?: string): Promise<boolean> {
     const menu = this.get(menuId);
-    if (menu && !this.isAnimating()) {
-      const openedMenu = this.getOpen();
-      if (openedMenu && menu !== openedMenu) {
-        openedMenu.setOpen(false, false);
-      }
+    if (menu) {
       return menu.open();
     }
     return Promise.resolve(false);
@@ -58,7 +54,6 @@ export class MenuController {
     return Promise.resolve(false);
   }
 
-
   /**
    * Toggle the menu. If it's closed, it will open, and if opened, it
    * will close.
@@ -68,11 +63,7 @@ export class MenuController {
   @Method()
   toggle(menuId?: string): Promise<boolean> {
     const menu = this.get(menuId);
-    if (menu && !this.isAnimating()) {
-      const openedMenu = this.getOpen();
-      if (openedMenu && menu !== openedMenu) {
-        openedMenu.setOpen(false, false);
-      }
+    if (menu) {
       return menu.toggle();
     }
     return Promise.resolve(false);
@@ -148,14 +139,12 @@ export class MenuController {
    */
   @Method()
   get(menuId?: string): HTMLIonMenuElement {
-    let menu: Menu;
-
     if (menuId === 'left' || menuId === 'right') {
       // there could be more than one menu on the same side
       // so first try to get the enabled one
-      menu = this.menus.find(m => m.side === menuId && !m.disabled);
+      const menu = this.find(m => m.side === menuId && !m.disabled);
       if (menu) {
-        return menu.getElement();
+        return menu;
       }
 
       // didn't find a menu side that is enabled
@@ -169,13 +158,13 @@ export class MenuController {
     }
 
     // return the first enabled menu
-    menu = this.menus.find(m => !m.disabled);
+    const menu = this.find(m => !m.disabled);
     if (menu) {
-      return menu.getElement();
+      return menu;
     }
 
     // get the first menu in the array, if one exists
-    return (this.menus.length > 0 ? this.menus[0].getElement() : null);
+    return (this.menus.length > 0 ? this.menus[0].el : null);
   }
 
   /**
@@ -191,7 +180,7 @@ export class MenuController {
    */
   @Method()
   getMenus(): HTMLIonMenuElement[] {
-    return this.menus.map(menu => menu.getElement());
+    return this.menus.map(menu => menu.el);
   }
 
   /**
@@ -238,23 +227,43 @@ export class MenuController {
       .map(m => m.disabled = true);
   }
 
+  /**
+   * @hidden
+   */
+  @Method()
+  _setOpen(menu: Menu, shouldOpen: boolean, animated: boolean): Promise<boolean> {
+    if (this.isAnimating()) {
+      return Promise.resolve(false);
+    }
+    if (shouldOpen) {
+      const openedMenu = this.getOpen();
+      if (openedMenu && menu !== openedMenu) {
+        openedMenu.setOpen(false, false);
+      }
+    }
+    return menu._setOpen(shouldOpen, animated);
+  }
+
   /**
    * @hidden
    */
   @Method()
   createAnimation(type: string, menuCmp: Menu): Promise<Animation> {
-    const animationBuilder = this.menuAnimations[type];
+    const animationBuilder = this.menuAnimations.get(type);
+    if (!animationBuilder) {
+      return Promise.reject('animation not registered');
+    }
     return this.animationCtrl.create(animationBuilder, null, menuCmp);
   }
 
-  private registerAnimation(name: string, cls: AnimationBuilder) {
-    this.menuAnimations[name] = cls;
+  private registerAnimation(name: string, animation: AnimationBuilder) {
+    this.menuAnimations.set(name, animation);
   }
 
   private find(predicate: (menu: Menu) => boolean): HTMLIonMenuElement {
     const instance = this.menus.find(predicate);
     if (instance) {
-      return instance.getElement();
+      return instance.el;
     }
     return null;
   }

packages/core/src/components/menu/menu.tsx

@@ -30,7 +30,7 @@ export class Menu {
   contentEl: HTMLElement;
   menuCtrl: HTMLIonMenuControllerElement;
 
-  @Element() private el: HTMLIonMenuElement;
+  @Element() el: HTMLIonMenuElement;
 
   @State() isRightSide = false;
 
@@ -41,7 +41,7 @@ export class Menu {
   /**
    * The content's id the menu should use.
    */
-  @Prop() content: string;
+  @Prop() contentId: string;
 
   /**
    * An id for the menu.
@@ -56,11 +56,12 @@ export class Menu {
   @Prop({ mutable: true }) type = 'overlay';
 
   @Watch('type')
-  typeChanged(type: string) {
-    if (this.contentEl) {
-      this.contentEl.classList.remove('menu-content-' + this.type);
-      this.contentEl.classList.add('menu-content-' + type);
-      this.contentEl.removeAttribute('style');
+  typeChanged(type: string, oldType: string | null) {
+    const contentEl = this.contentEl;
+    if (contentEl && oldType) {
+      contentEl.classList.remove(`menu-content-${oldType}`);
+      contentEl.classList.add(`menu-content-${type}`);
+      contentEl.removeAttribute('style');
     }
     if (this.menuInnerEl) {
       // Remove effects of previous animations
@@ -132,22 +133,19 @@ export class Menu {
     assert(!!this.menuCtrl, 'menucontroller was not initialized');
 
     const el = this.el;
-    const contentQuery = (this.content)
-      ? '#' + this.content
+    const contentQuery = (this.contentId)
+      ? '#' + this.contentId
       : '[main]';
     const parent = el.parentElement;
     const content = this.contentEl = parent.querySelector(contentQuery) as HTMLElement;
     if (!content || !content.tagName) {
       // requires content element
       return console.error('Menu: must have a "content" element to listen for drag events on.');
     }
-    this.menuInnerEl = el.querySelector('.menu-inner') as HTMLElement;
-    this.backdropEl = el.querySelector('.menu-backdrop') as HTMLElement;
-
     // add menu's content classes
     content.classList.add('menu-content');
 
-    this.typeChanged(this.type);
+    this.typeChanged(this.type, null);
     this.sideChanged();
 
     let isEnabled = !this.disabled;
@@ -188,17 +186,32 @@ export class Menu {
     }
   }
 
-  getElement(): HTMLIonMenuElement {
-    return this.el;
-  }
-
   @Method()
   isOpen(): boolean {
     return this._isOpen;
   }
 
+  @Method()
+  open(animated = true): Promise<boolean> {
+    return this.setOpen(true, animated);
+  }
+
+  @Method()
+  close(animated = true): Promise<boolean> {
+    return this.setOpen(false, animated);
+  }
+
+  @Method()
+  toggle(animated = true): Promise<boolean> {
+    return this.setOpen(!this._isOpen, animated);
+  }
+
   @Method()
   setOpen(shouldOpen: boolean, animated = true): Promise<boolean> {
+    return this.menuCtrl._setOpen(this, shouldOpen, animated);
+  }
+
+  _setOpen(shouldOpen: boolean, animated = true): Promise<boolean> {
     // If the menu is disabled or it is currenly being animated, let's do nothing
     if (!this.isActive() || this.isAnimating || (shouldOpen === this._isOpen)) {
       return Promise.resolve(this._isOpen);
@@ -211,18 +224,8 @@ export class Menu {
   }
 
   @Method()
-  open(): Promise<boolean> {
-    return this.setOpen(true);
-  }
-
-  @Method()
-  close(): Promise<boolean> {
-    return this.setOpen(false);
-  }
-
-  @Method()
-  toggle(): Promise<boolean> {
-    return this.setOpen(!this._isOpen);
+  isActive(): boolean {
+    return !this.disabled && !this.isPane;
   }
 
   private loadAnimation(): Promise<void> {
@@ -259,10 +262,6 @@ export class Menu {
     return promise;
   }
 
-  private isActive(): boolean {
-    return !this.disabled && !this.isPane;
-  }
-
   private canSwipe(): boolean {
     return this.swipeEnabled &&
       !this.isAnimating &&
@@ -428,24 +427,23 @@ export class Menu {
 
   hostData() {
     const isRightSide = this.isRightSide;
-    const typeClass = 'menu-type-' + this.type;
     return {
       role: 'complementary',
       class: {
+        [`menu-type-${this.type}`]: true,
         'menu-enabled': !this.disabled,
         'menu-side-right': isRightSide,
         'menu-side-left': !isRightSide,
-        [typeClass]: true,
       }
     };
   }
 
   render() {
     return ([
-      <div class='menu-inner page-inner'>
+      <div class='menu-inner page-inner' ref={el => this.menuInnerEl = el}>
         <slot></slot>
       </div>,
-      <ion-backdrop class='menu-backdrop'></ion-backdrop> ,
+      <ion-backdrop class='menu-backdrop' ref={el => this.backdropEl = el}></ion-backdrop> ,
       <ion-gesture {...{
         'canStart': this.canStart.bind(this),
         'onWillStart': this.onWillStart.bind(this),

packages/core/src/components/menu/readme.md

@@ -7,7 +7,7 @@
 
 ## Properties
 
-#### content
+#### contentId
 
 string
 
@@ -65,7 +65,7 @@ see the `menuType` in the [config](../../config/Config). Available options:
 
 ## Attributes
 
-#### content
+#### content-id
 
 string
 
@@ -144,6 +144,9 @@ Emitted when the menu is open.
 #### close()
 
 
+#### isActive()
+
+
 #### isOpen()