diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index ec12c98210..350984b5a0 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -40,6 +40,15 @@ export class Menu implements ComponentInterface, MenuI { private blocker = GESTURE_CONTROLLER.createBlocker({ disableScroll: true }); private didLoad = false; + /** + * Flag used to determine if an open/close + * operation was cancelled. For example, if + * an app calls "menu.open" then disables the menu + * part way through the animation, then this would + * be considered a cancelled operation. + */ + private operationCancelled = false; + isAnimating = false; width!: number; _isOpen = false; @@ -432,6 +441,17 @@ export class Menu implements ComponentInterface, MenuI { await this.loadAnimation(); await this.startAnimation(shouldOpen, animated); + + /** + * If the animation was cancelled then + * return false because the operation + * did not succeed. + */ + if (this.operationCancelled) { + this.operationCancelled = false; + return false; + } + this.afterAnimation(shouldOpen); return true; @@ -472,18 +492,24 @@ export class Menu implements ComponentInterface, MenuI { const easingReverse = mode === 'ios' ? iosEasingReverse : mdEasingReverse; const ani = (this.animation as Animation)! .direction(isReversed ? 'reverse' : 'normal') - .easing(isReversed ? easingReverse : easing) - .onFinish(() => { - if (ani.getDirection() === 'reverse') { - ani.direction('normal'); - } - }); + .easing(isReversed ? easingReverse : easing); if (animated) { await ani.play(); } else { ani.play({ sync: true }); } + + /** + * We run this after the play invocation + * instead of using ani.onFinish so that + * multiple onFinish callbacks do not get + * run if an animation is played, stopped, + * and then played again. + */ + if (ani.getDirection() === 'reverse') { + ani.direction('normal'); + } } private _isActive() { @@ -643,8 +669,6 @@ export class Menu implements ComponentInterface, MenuI { } private afterAnimation(isOpen: boolean) { - assert(this.isAnimating, '_before() should be called while animating'); - // keep opening/closing the menu disabled for a touch more yet // only add listeners/css if it's enabled and isOpen // and only remove listeners/css if it's not open @@ -713,10 +737,30 @@ export class Menu implements ComponentInterface, MenuI { this.gesture.enable(isActive && this.swipeGesture); } - // Close menu immediately - if (!isActive && this._isOpen) { - // close if this menu is open, and should not be enabled - this.forceClosing(); + /** + * If the menu is disabled but it is still open + * then we should close the menu immediately. + * Additionally, if the menu is in the process + * of animating {open, close} and the menu is disabled + * then it should still be closed immediately. + */ + if (!isActive) { + /** + * It is possible to disable the menu while + * it is mid-animation. When this happens, we + * need to set the operationCancelled flag + * so that this._setOpen knows to return false + * and not run the "afterAnimation" callback. + */ + if (this.isAnimating) { + this.operationCancelled = true; + } + + /** + * If the menu is disabled then we should + * forcibly close the menu even if it is open. + */ + this.afterAnimation(false); } if (doc?.contains(this.el)) { @@ -730,19 +774,6 @@ export class Menu implements ComponentInterface, MenuI { menuController._setActiveMenu(this); } } - - assert(!this.isAnimating, 'can not be animating'); - } - - private forceClosing() { - assert(this._isOpen, 'menu cannot be closed'); - - this.isAnimating = true; - - const ani = (this.animation as Animation)!.direction('reverse'); - ani.play({ sync: true }); - - this.afterAnimation(false); } render() { diff --git a/core/src/components/menu/test/disable/index.html b/core/src/components/menu/test/disable/index.html new file mode 100644 index 0000000000..fd561ce69d --- /dev/null +++ b/core/src/components/menu/test/disable/index.html @@ -0,0 +1,41 @@ + + + + + Menu - Disable + + + + + + + + + + + + + + Menu + + + Menu Content + + +
+ + + + + + Menu - Disable + + + Content +
+
+ + diff --git a/core/src/components/menu/test/disable/menu.e2e.ts b/core/src/components/menu/test/disable/menu.e2e.ts new file mode 100644 index 0000000000..c4a352694a --- /dev/null +++ b/core/src/components/menu/test/disable/menu.e2e.ts @@ -0,0 +1,66 @@ +import { expect } from '@playwright/test'; +import { configs, test } from '@utils/test/playwright'; + +/** + * This behavior does not vary across modes/directions + */ +configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('menu: disable'), () => { + test.beforeEach(async ({ page }) => { + await page.goto(`/src/components/menu/test/disable`, config); + }); + + test('should disable when menu is fully open', async ({ page }) => { + const logs: string[] = []; + + page.on('console', (msg) => { + if (msg.type() === 'error') { + logs.push(msg.text()); + } + }); + + const menu = page.locator('ion-menu'); + + // Should be visible on initial presentation + await menu.evaluate((el: HTMLIonMenuElement) => el.open()); + await expect(menu).toBeVisible(); + + // Disabling menu should hide it + await menu.evaluate((el: HTMLIonMenuElement) => (el.disabled = true)); + await expect(menu).toBeHidden(); + + // Re-enabling menu and opening it show make it visible + await menu.evaluate((el: HTMLIonMenuElement) => (el.disabled = false)); + await menu.evaluate((el: HTMLIonMenuElement) => el.open()); + await expect(menu).toBeVisible(); + + expect(logs.length).toBe(0); + }); + + test('should disable when menu is animating', async ({ page }) => { + const logs: string[] = []; + + page.on('console', (msg) => { + if (msg.type() === 'error') { + logs.push(msg.text()); + } + }); + + const menu = page.locator('ion-menu'); + + // Opening and quickly disabling menu should hide it + menu.evaluate((el: HTMLIonMenuElement) => { + el.open(); + setTimeout(() => (el.disabled = true), 0); + }); + await expect(menu).toBeHidden(); + + // Re-enabling menu and opening it show make it visible + await menu.evaluate((el: HTMLIonMenuElement) => (el.disabled = false)); + await menu.evaluate((el: HTMLIonMenuElement) => el.open()); + await expect(menu).toBeVisible(); + + expect(logs.length).toBe(0); + }); + }); +});