mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-08-18 19:21:34 +08:00
fix(menu): emit ionMenuChange when re-mounted (#28049)
Issue number: resolves #28030
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
`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
314055cf7a/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?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- The menu now fires `ionMenuChange` on `connectedCallback` as long as
`componentDidLoad` has already been run.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Dev build: `7.3.2-dev.11692803611.15c1bc87`
---------
Co-authored-by: Sean Perkins <sean@ionic.io>
This commit is contained in:
Liam DeBeasi
64
core/src/components/menu-button/test/async/index.html
Normal file
64
core/src/components/menu-button/test/async/index.html
Normal file
@ -0,0 +1,64 @@
|
|||||||
|
<!DOCTYPE html>
|
||||||
|
<html lang="en" dir="ltr">
|
||||||
|
<head>
|
||||||
|
<meta charset="UTF-8" />
|
||||||
|
<title>Menu - Async</title>
|
||||||
|
<meta
|
||||||
|
name="viewport"
|
||||||
|
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no, viewport-fit=cover"
|
||||||
|
/>
|
||||||
|
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
|
||||||
|
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
|
||||||
|
<script src="../../../../../scripts/testing/scripts.js"></script>
|
||||||
|
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
|
||||||
|
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
|
||||||
|
<script type="module">
|
||||||
|
import { menuController } from '../../../../dist/ionic/index.esm.js';
|
||||||
|
window.menuController = menuController;
|
||||||
|
</script>
|
||||||
|
</head>
|
||||||
|
|
||||||
|
<body>
|
||||||
|
<ion-app>
|
||||||
|
<div id="menu-container">
|
||||||
|
<ion-menu content-id="main">
|
||||||
|
<ion-content class="ion-padding"> Menu Content </ion-content>
|
||||||
|
</ion-menu>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div class="ion-page" id="main">
|
||||||
|
<ion-header>
|
||||||
|
<ion-toolbar>
|
||||||
|
<ion-title>Menu - Async</ion-title>
|
||||||
|
<ion-buttons slot="start" id="buttons-container"></ion-buttons>
|
||||||
|
</ion-toolbar>
|
||||||
|
</ion-header>
|
||||||
|
<ion-content class="ion-padding">
|
||||||
|
Main Content
|
||||||
|
<button onclick="trigger()" id="trigger">Add Menu To DOM</button>
|
||||||
|
</ion-content>
|
||||||
|
</div>
|
||||||
|
</ion-app>
|
||||||
|
|
||||||
|
<script>
|
||||||
|
const buttons = document.querySelector('ion-buttons');
|
||||||
|
const menuButton = document.createElement('ion-menu-button');
|
||||||
|
const menu = document.querySelector('ion-menu');
|
||||||
|
const menuContainer = document.querySelector('#menu-container');
|
||||||
|
|
||||||
|
let firstLoad = true;
|
||||||
|
|
||||||
|
// When the menu loads, immediately remove it from the DOM
|
||||||
|
document.body.addEventListener('ionMenuChange', () => {
|
||||||
|
if (firstLoad) {
|
||||||
|
menuContainer.removeChild(menu);
|
||||||
|
buttons.appendChild(menuButton);
|
||||||
|
firstLoad = false;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
const trigger = () => {
|
||||||
|
menuContainer.append(menu);
|
||||||
|
};
|
||||||
|
</script>
|
||||||
|
</body>
|
||||||
|
</html>
|
||||||
@ -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();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -38,6 +38,7 @@ export class Menu implements ComponentInterface, MenuI {
|
|||||||
private lastOnEnd = 0;
|
private lastOnEnd = 0;
|
||||||
private gesture?: Gesture;
|
private gesture?: Gesture;
|
||||||
private blocker = GESTURE_CONTROLLER.createBlocker({ disableScroll: true });
|
private blocker = GESTURE_CONTROLLER.createBlocker({ disableScroll: true });
|
||||||
|
private didLoad = false;
|
||||||
|
|
||||||
isAnimating = false;
|
isAnimating = false;
|
||||||
width!: number;
|
width!: number;
|
||||||
@ -216,6 +217,7 @@ export class Menu implements ComponentInterface, MenuI {
|
|||||||
|
|
||||||
// register this menu with the app's menu controller
|
// register this menu with the app's menu controller
|
||||||
menuController._register(this);
|
menuController._register(this);
|
||||||
|
this.menuChanged();
|
||||||
|
|
||||||
this.gesture = (await import('../../utils/gesture')).createGesture({
|
this.gesture = (await import('../../utils/gesture')).createGesture({
|
||||||
el: document,
|
el: document,
|
||||||
@ -237,10 +239,22 @@ export class Menu implements ComponentInterface, MenuI {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async componentDidLoad() {
|
async componentDidLoad() {
|
||||||
this.ionMenuChange.emit({ disabled: this.disabled, open: this._isOpen });
|
this.didLoad = true;
|
||||||
|
this.menuChanged();
|
||||||
this.updateState();
|
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() {
|
async disconnectedCallback() {
|
||||||
/**
|
/**
|
||||||
* The menu should be closed when it is
|
* The menu should be closed when it is
|
||||||
|
|||||||
Reference in New Issue
Block a user