From fa4113117d3c00127200c0883bfeed46dc018af7 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Thu, 24 Aug 2023 09:37:49 -0500 Subject: [PATCH] fix(menu): emit ionMenuChange when re-mounted (#28049) Issue number: resolves #28030 --------- ## What is the current behavior? `ion-menu` registers itself with the menu controller when `connectedCallback` fires and then unregisters itself when `disconnectedCallback` fires. When the menu was removed from the DOM, `disconnectedCallback` was not always being fired due to https://github.com/ionic-team/stencil/issues/4070. `ion-menu-button` checks to see if it should be visible by grabbing the current menu in https://github.com/ionic-team/ionic-framework/blob/314055cf7a66a58e4e88c22e6e755fadd494ed70/core/src/components/menu-button/menu-button.tsx#L74. Since `disconnectedCallback` was not being fired, `ion-menu-button` would still find the menu even when it was no longer in the DOM. In this case, the menu was not being unregistered due to `disconnectedCallback` not firing. When the linked Stencil bug was resolved in Stencil 4.0.3, the menu button started to disappear. This is happening because `disconnectedCallback` is now correctly called when the menu is removed from the DOM. Since `disconnectedCallback` is called, the menu is un-registered from the menu controller, and the menu button can no longer find it. However, this revealed a long-standing bug where re-adding the menu would not fire `ionMenuChange` again. As a result, the menu button remained hidden. ## What is the new behavior? - The menu now fires `ionMenuChange` on `connectedCallback` as long as `componentDidLoad` has already been run. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information Dev build: `7.3.2-dev.11692803611.15c1bc87` --------- Co-authored-by: Sean Perkins --- .../menu-button/test/async/index.html | 64 +++++++++++++++++++ .../menu-button/test/async/menu-button.e2e.ts | 25 ++++++++ core/src/components/menu/menu.tsx | 16 ++++- 3 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 core/src/components/menu-button/test/async/index.html create mode 100644 core/src/components/menu-button/test/async/menu-button.e2e.ts diff --git a/core/src/components/menu-button/test/async/index.html b/core/src/components/menu-button/test/async/index.html new file mode 100644 index 0000000000..105b9a3b87 --- /dev/null +++ b/core/src/components/menu-button/test/async/index.html @@ -0,0 +1,64 @@ + + + + + Menu - Async + + + + + + + + + + + + + +
+ + + Menu - Async + + + + + Main Content + + +
+ + + + + diff --git a/core/src/components/menu-button/test/async/menu-button.e2e.ts b/core/src/components/menu-button/test/async/menu-button.e2e.ts new file mode 100644 index 0000000000..9d06238b19 --- /dev/null +++ b/core/src/components/menu-button/test/async/menu-button.e2e.ts @@ -0,0 +1,25 @@ +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 button: async'), () => { + test('menu button should be visible if menu is moved', async ({ page }) => { + await page.goto(`/src/components/menu-button/test/async`, config); + + const menu = page.locator('ion-menu'); + const menuButton = page.locator('ion-menu-button'); + const triggerButton = page.locator('#trigger'); + + await expect(menu).not.toBeAttached(); + await expect(menuButton).toBeHidden(); + + await triggerButton.click(); + + await expect(menu).toBeAttached(); + await expect(menuButton).toBeVisible(); + }); + }); +}); diff --git a/core/src/components/menu/menu.tsx b/core/src/components/menu/menu.tsx index c010e72d2d..5072bb74cd 100644 --- a/core/src/components/menu/menu.tsx +++ b/core/src/components/menu/menu.tsx @@ -38,6 +38,7 @@ export class Menu implements ComponentInterface, MenuI { private lastOnEnd = 0; private gesture?: Gesture; private blocker = GESTURE_CONTROLLER.createBlocker({ disableScroll: true }); + private didLoad = false; isAnimating = false; width!: number; @@ -216,6 +217,7 @@ export class Menu implements ComponentInterface, MenuI { // register this menu with the app's menu controller menuController._register(this); + this.menuChanged(); this.gesture = (await import('../../utils/gesture')).createGesture({ el: document, @@ -237,10 +239,22 @@ export class Menu implements ComponentInterface, MenuI { } async componentDidLoad() { - this.ionMenuChange.emit({ disabled: this.disabled, open: this._isOpen }); + this.didLoad = true; + this.menuChanged(); this.updateState(); } + private menuChanged() { + /** + * Inform dependent components such as ion-menu-button + * that the menu is ready. Note that we only want to do this + * once the menu has been rendered which is why we check for didLoad. + */ + if (this.didLoad) { + this.ionMenuChange.emit({ disabled: this.disabled, open: this._isOpen }); + } + } + async disconnectedCallback() { /** * The menu should be closed when it is