Skip to content

Commit

Permalink
fix(menu): menu no longer disappears with multiple split panes (#28370)
Browse files Browse the repository at this point in the history
Issue number: resolves #18683, resolves #15538, resolves #22341

---------

<!-- 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. -->

Menus in a split pane are hidden when a second split pane is
mounted/made visible. This is because the `onSplitPaneChanged` callback
does not take into account whether the it is a child of the split pane
that emitted `ionSplitPaneVisible`.

When split pane 2 is shown, that causes the menu is split pane 1 to
hide. When split pane 1 is shown, the menu inside of it _is_ shown.
However, since split pane 2 is then hidden that component also emits
`ionSplitPaneVisible`, causing the menu inside of split pane 1 to hide.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Menus are only hidden when its parent split pane changes visibility

## 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.5.1-dev.11697568647.1ac87d08`

---------

Co-authored-by: Amanda Johnston <[email protected]>
  • Loading branch information
liamdebeasi and averyjohnston authored Oct 19, 2023
1 parent 068d003 commit 5a30082
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 0 deletions.
14 changes: 14 additions & 0 deletions core/src/components/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,20 @@ export class Menu implements ComponentInterface, MenuI {

@Listen('ionSplitPaneVisible', { target: 'body' })
onSplitPaneChanged(ev: CustomEvent) {
const { target } = ev;
const closestSplitPane = this.el.closest('ion-split-pane');

/**
* Menu listens on the body for "ionSplitPaneVisible".
* However, this means the callback will run any time
* a SplitPane changes visibility. As a result, we only want
* Menu's visibility state to update if its parent SplitPane
* changes visibility.
*/
if (target !== closestSplitPane) {
return;
}

this.isPaneVisible = ev.detail.isPane(this.el);
this.updateState();
}
Expand Down
69 changes: 69 additions & 0 deletions core/src/components/split-pane/test/multiple/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<!DOCTYPE html>
<html lang="en" dir="ltr">
<head>
<meta charset="UTF-8" />
<title>Split Pane - Multiple</title>
<meta
name="viewport"
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
/>
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
<script src="../../../../../scripts/testing/scripts.js"></script>
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
</head>
<body>
<ion-split-pane id="pane-one" content-id="content-one">
<ion-menu id="pane-one-menu-one" side="start" content-id="content-one">
<div class="ion-padding">Split One Menu One</div>
</ion-menu>

<div class="ion-page" id="content-one">
<ion-content class="ion-padding">
Page Content One
<button id="show-pane-two" onclick="showSecondPane()">Show Second Pane</button>
</ion-content>
</div>

<ion-menu id="pane-one-menu-two" side="end" content-id="content-one">
<div class="ion-padding">Split One Menu Two</div>
</ion-menu>
</ion-split-pane>

<ion-split-pane id="pane-two" content-id="content-two" disabled="true" class="ion-page-hidden">
<ion-menu id="pane-two-menu-one" side="start" content-id="content-two">
<div class="ion-padding">Split Two Menu One</div>
</ion-menu>

<div class="ion-page" id="content-two">
<ion-content class="ion-padding">
Page Content Two
<button id="show-pane-one" onclick="showFirstPane()">Show First Pane</button>
</ion-content>
</div>

<ion-menu id="pane-two-menu-two" side="end" content-id="content-two">
<div class="ion-padding">Split Two Menu Two</div>
</ion-menu>
</ion-split-pane>

<script>
const splitPaneOne = document.querySelector('ion-split-pane#pane-one');
const splitPaneTwo = document.querySelector('ion-split-pane#pane-two');
const showSecondPane = () => {
splitPaneOne.disabled = true;
splitPaneOne.classList.add('ion-page-hidden');

splitPaneTwo.disabled = false;
splitPaneTwo.classList.remove('ion-page-hidden');
};
const showFirstPane = () => {
splitPaneOne.disabled = false;
splitPaneOne.classList.remove('ion-page-hidden');

splitPaneTwo.disabled = true;
splitPaneTwo.classList.add('ion-page-hidden');
};
</script>
</body>
</html>
38 changes: 38 additions & 0 deletions core/src/components/split-pane/test/multiple/split-pane.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';
import { configs, test, Viewports } from '@utils/test/playwright';

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('split-pane: multiple'), () => {
test('using multiple split panes should not hide a menu in another split pane', async ({ page }) => {
test.info().annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/18683',
});

await page.setViewportSize(Viewports.large);
await page.goto(`/src/components/split-pane/test/multiple`, config);

const paneOneMenuOne = page.locator('ion-menu#pane-one-menu-one');
const paneOneMenuTwo = page.locator('ion-menu#pane-one-menu-two');

const paneTwoMenuOne = page.locator('ion-menu#pane-two-menu-one');
const paneTwoMenuTwo = page.locator('ion-menu#pane-two-menu-two');

const showPaneOne = page.locator('button#show-pane-one');
const showPaneTwo = page.locator('button#show-pane-two');

await expect(paneOneMenuOne).toBeVisible();
await expect(paneOneMenuTwo).toBeVisible();

await showPaneTwo.click();

await expect(paneTwoMenuOne).toBeVisible();
await expect(paneTwoMenuTwo).toBeVisible();

await showPaneOne.click();

await expect(paneOneMenuOne).toBeVisible();
await expect(paneOneMenuTwo).toBeVisible();
});
});
});

0 comments on commit 5a30082

Please sign in to comment.