From d83d8eed12b0b7e852feffd77d51c9f5a92e6640 Mon Sep 17 00:00:00 2001 From: "Manu Mtz.-Almeida" Date: Mon, 12 Feb 2018 15:00:05 +0100 Subject: [PATCH] fix(menu): setOpen must be coordinated by the controller --- .../menu-controller/menu-controller.ts | 61 +++++++++------- packages/core/src/components/menu/menu.tsx | 72 +++++++++---------- packages/core/src/components/menu/readme.md | 7 +- 3 files changed, 75 insertions(+), 65 deletions(-) diff --git a/packages/core/src/components/menu-controller/menu-controller.ts b/packages/core/src/components/menu-controller/menu-controller.ts index 1fa89923ca..05f214c9ab 100644 --- a/packages/core/src/components/menu-controller/menu-controller.ts +++ b/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(); - @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 { 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 { 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 { + 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 { - 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; } diff --git a/packages/core/src/components/menu/menu.tsx b/packages/core/src/components/menu/menu.tsx index 160d648f94..cf8e327d76 100644 --- a/packages/core/src/components/menu/menu.tsx +++ b/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,8 +133,8 @@ 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; @@ -141,13 +142,10 @@ export class Menu { // 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 { + return this.setOpen(true, animated); + } + + @Method() + close(animated = true): Promise { + return this.setOpen(false, animated); + } + + @Method() + toggle(animated = true): Promise { + return this.setOpen(!this._isOpen, animated); + } + @Method() setOpen(shouldOpen: boolean, animated = true): Promise { + return this.menuCtrl._setOpen(this, shouldOpen, animated); + } + + _setOpen(shouldOpen: boolean, animated = true): Promise { // 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 { - return this.setOpen(true); - } - - @Method() - close(): Promise { - return this.setOpen(false); - } - - @Method() - toggle(): Promise { - return this.setOpen(!this._isOpen); + isActive(): boolean { + return !this.disabled && !this.isPane; } private loadAnimation(): Promise { @@ -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 ([ -
+
this.menuInnerEl = el}>
, - , + this.backdropEl = el}> ,