fix(menu): menu can be encapsulated in a component (#28801)

Issue number: resolves #16304, resolves #20681

---------

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

Split Pane assumes that Menu is a child of the Split Pane. This means
that CSS selectors and JS queries only check for children instead of
descendants. When a Menu is encapsulated in a component that component
can itself show up as an element in the DOM depending on the
environment. For example, both Angular and Web components show up as
elements in the DOM. This causes the Menu to not work because it is no
longer a child of the Split Pane.

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

- Menu can now be used as a descendant of Split Pane. The following
changes make this possible:
1. When the menu is loaded, it queries the ancestor Split Pane. If one
is found, it sets `isPaneVisible` depending on if the split pane is
visible or not.
2. Any changes to the split pane visibility state after the menu is
loaded will be handled through the `ionSplitPaneVisible` event.
3. A menu is now considered to be a pane if it is inside of a split pane
4. Related CSS no longer uses `::slotted` which only targets children.
The CSS was moved into the menu stylesheets. I consider the coupling
here to be valuable as menu and split pane are often used together. We
also already have style coupling between the two components, so this
isn't anything new.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

There are no known changes to the public API or public behavior.
However, there are two risks here:

1. There is an unknown risk into how this change impacts nested split
panes. This isn't an explicitly supported feature, but it technically
works in Ionic now. We don't know if anyone is actually implementing
this pattern. We'd like to evaluate use cases for nested split panes
submitted by the community before we try to account for this
functionality in menu and split pane.
5. There may be some specificity changes due to the new CSS. CSS was
moved from split pane to menu so it could work with encapsulated
components:

`:host(.split-pane-visible) ::slotted(.split-pane-side)` has a
specificity of 0-1-1

`:host(.menu-page-visible.menu-pane-side)` has a specificity of 0-1-0

There shouldn't be any changes needed to developer code since the
selectors are in the Shadow DOM and developer styles are in the Light
DOM. However, we aren't able to anticipate every edge case so we'd like
to place this in a major release just to be safe.

## 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.6.2-dev.11704922151.1fd3369f`

core/src/components.d.ts

@@ -2852,6 +2852,7 @@ export namespace Components {
           * If `true`, the split pane will be hidden.
          */
         "disabled": boolean;
+        "isVisible": () => Promise<boolean>;
         /**
           * When the split-pane should be shown. Can be a CSS media query expression, or a shortcut expression. Can also be a boolean expression.
          */

core/src/components/menu/menu.scss

@@ -179,10 +179,51 @@ ion-backdrop {
 // Menu Split Pane
 // --------------------------------------------------
 
+/**
+ * The split pane styles for menu are defined
+ * in the menu stylesheets instead in the split pane
+ * stylesheets with ::slotted to allow for menus
+ * to be wrapped in custom components.
+ * If we used ::slotted to target the menu
+ * then menus wrapped in components would never
+ * receive these styles because they are not
+ * children of the split pane.
+ */
+
+/**
+ * Do not pass CSS Variables down on larger
+ * screens as we want them to affect the outer
+ * `ion-menu` rather than the inner content
+ */
 :host(.menu-pane-visible) {
-  width: var(--width);
+  flex: 0 1 auto;
-  min-width: var(--min-width);
+
-  max-width: var(--max-width);
+  width: var(--side-width, var(--width));
+  min-width: var(--side-min-width, var(--min-width));
+  max-width: var(--side-max-width, var(--max-width));
+}
+
+:host(.menu-pane-visible.split-pane-side) {
+  @include position(0, 0, 0, 0);
+
+  position: relative;
+
+  box-shadow: none;
+  z-index: 0;
+}
+
+:host(.menu-pane-visible.split-pane-side.menu-enabled) {
+  display: flex;
+
+  flex-shrink: 0;
+}
+
+:host(.menu-pane-visible.split-pane-side) {
+  order: -1;
+}
+
+:host(.menu-pane-visible.split-pane-side[side=end]) {
+  order: 1;
 }
 
 :host(.menu-pane-visible) .menu-inner {
@@ -199,3 +240,18 @@ ion-backdrop {
   /* stylelint-disable-next-line declaration-no-important */
   display: hidden !important;
 }
+
+:host(.menu-pane-visible.split-pane-side) {
+  @include border(0, var(--border), 0, 0);
+
+  min-width: var(--side-min-width);
+  max-width: var(--side-max-width);
+}
+
+:host(.menu-pane-visible.split-pane-side[side=end]) {
+  @include border(0, 0, 0, var(--border));
+
+  min-width: var(--side-min-width);
+  max-width: var(--side-max-width);
+}
+

core/src/components/menu/menu.tsx

@@ -6,6 +6,7 @@ import type { Attributes } from '@utils/helpers';
 import { inheritAriaAttributes, assert, clamp, isEndSide as isEnd } from '@utils/helpers';
 import { menuController } from '@utils/menu-controller';
 import { getPresentedOverlay } from '@utils/overlays';
+import { hostContext } from '@utils/theme';
 
 import { config } from '../../global/config';
 import { getIonMode } from '../../global/ionic-global';
@@ -77,6 +78,14 @@ export class Menu implements ComponentInterface, MenuI {
 
   @Element() el!: HTMLIonMenuElement;
 
+  /**
+   * If true, then the menu should be
+   * visible within a split pane.
+   * If false, then the menu is hidden.
+   * However, the menu-button/menu-toggle
+   * components can be used to open the
+   * menu.
+   */
   @State() isPaneVisible = false;
   @State() isEndSide = false;
 
@@ -225,8 +234,8 @@ export class Menu implements ComponentInterface, MenuI {
 
     // register this menu with the app's menu controller
     menuController._register(this);
-    this.menuChanged();
 
+    this.menuChanged();
     this.gesture = (await import('../../utils/gesture')).createGesture({
       el: document,
       gestureName: 'menu-swipe',
@@ -248,6 +257,21 @@ export class Menu implements ComponentInterface, MenuI {
 
   async componentDidLoad() {
     this.didLoad = true;
+
+    /**
+     * A menu inside of a split pane is assumed
+     * to be a side pane.
+     *
+     * When the menu is loaded it needs to
+     * see if it should be considered visible inside
+     * of the split pane. If the split pane is
+     * hidden then the menu should be too.
+     */
+    const splitPane = this.el.closest('ion-split-pane');
+    if (splitPane !== null) {
+      this.isPaneVisible = await splitPane.isVisible();
+    }
+
     this.menuChanged();
     this.updateState();
   }
@@ -288,23 +312,14 @@ export class Menu implements ComponentInterface, MenuI {
   }
 
   @Listen('ionSplitPaneVisible', { target: 'body' })
-  onSplitPaneChanged(ev: CustomEvent) {
+  onSplitPaneChanged(ev: CustomEvent<{ visible: boolean }>) {
-    const { target } = ev;
+    const closestSplitPane = this.el.closest<HTMLIonSplitPaneElement>('ion-split-pane');
-    const closestSplitPane = this.el.closest('ion-split-pane');
 
-    /**
+    if (closestSplitPane !== null && closestSplitPane === ev.target) {
-     * Menu listens on the body for "ionSplitPaneVisible".
+      this.isPaneVisible = ev.detail.visible;
-     * However, this means the callback will run any time
+
-     * a SplitPane changes visibility. As a result, we only want
+      this.updateState();
-     * 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();
   }
 
   @Listen('click', { capture: true })
@@ -778,7 +793,7 @@ export class Menu implements ComponentInterface, MenuI {
   }
 
   render() {
-    const { type, disabled, isPaneVisible, inheritedAttributes, side } = this;
+    const { type, disabled, el, isPaneVisible, inheritedAttributes, side } = this;
     const mode = getIonMode(this);
 
     return (
@@ -791,6 +806,7 @@ export class Menu implements ComponentInterface, MenuI {
           'menu-enabled': !disabled,
           [`menu-side-${side}`]: true,
           'menu-pane-visible': isPaneVisible,
+          'split-pane-side': hostContext('ion-split-pane', el),
         }}
       >
         <div class="menu-inner" part="container" ref={(el) => (this.menuInnerEl = el)}>

core/src/components/split-pane/split-pane.ios.scss

@@ -9,17 +9,3 @@
   --side-min-width: #{$split-pane-ios-side-min-width};
   --side-max-width: #{$split-pane-ios-side-max-width};
 }
-
-:host(.split-pane-visible) ::slotted(.split-pane-side) {
-  @include border(0, var(--border), 0, 0);
-
-  min-width: var(--side-min-width);
-  max-width: var(--side-max-width);
-}
-
-:host(.split-pane-visible) ::slotted(.split-pane-side[side=end]) {
-  @include border(0, 0, 0, var(--border));
-  
-  min-width: var(--side-min-width);
-  max-width: var(--side-max-width);
-}

core/src/components/split-pane/split-pane.md.scss

@@ -9,17 +9,3 @@
   --side-min-width: #{$split-pane-md-side-min-width};
   --side-max-width: #{$split-pane-md-side-max-width};
 }
-
-:host(.split-pane-visible) ::slotted(.split-pane-side) {
-  @include border(0, var(--border), 0, 0);
-
-  min-width: var(--side-min-width);
-  max-width: var(--side-max-width);
-}
-
-:host(.split-pane-visible) ::slotted(.split-pane-side[side=end]) {
-  @include border(0, 0, 0, var(--border));
-
-  min-width: var(--side-min-width);
-  max-width: var(--side-max-width);
-}

core/src/components/split-pane/split-pane.scss

@@ -24,48 +24,17 @@
   contain: strict;
 }
 
-/**
- * Do not pass CSS Variables down on larger
- * screens as we want them to affect the outer
- * `ion-menu` rather than the inner content
- */
-::slotted(ion-menu.menu-pane-visible) {
-  flex: 0 1 auto;
-
-  width: var(--side-width);
-  min-width: var(--side-min-width);
-  max-width: var(--side-max-width);
-}
-
-:host(.split-pane-visible) ::slotted(.split-pane-side),
 :host(.split-pane-visible) ::slotted(.split-pane-main) {
   @include position(0, 0, 0, 0);
 
   position: relative;
 
+  flex: 1;
+
   box-shadow: none;
   z-index: 0;
 }
 
-:host(.split-pane-visible) ::slotted(.split-pane-main) {
-  flex: 1;
-}
-
-:host(.split-pane-visible) ::slotted(.split-pane-side:not(ion-menu)),
-:host(.split-pane-visible) ::slotted(ion-menu.split-pane-side.menu-enabled) {
-  display: flex;
-
-  flex-shrink: 0;
-}
-
 ::slotted(.split-pane-side:not(ion-menu)) {
   display: none;
 }
-
-:host(.split-pane-visible) ::slotted(.split-pane-side) {
-  order: -1;
-}
-
-:host(.split-pane-visible) ::slotted(.split-pane-side[side=end]) {
-  order: 1;
-}

core/src/components/split-pane/split-pane.tsx

@@ -1,5 +1,5 @@
 import type { ComponentInterface, EventEmitter } from '@stencil/core';
-import { Build, Component, Element, Event, Host, Prop, State, Watch, h } from '@stencil/core';
+import { Build, Component, Element, Event, Host, Method, Prop, State, Watch, h } from '@stencil/core';
 
 import { getIonMode } from '../../global/ionic-global';
 
@@ -58,8 +58,15 @@ export class SplitPane implements ComponentInterface {
 
   @Watch('visible')
   visibleChanged(visible: boolean) {
-    const detail = { visible, isPane: this.isPane.bind(this) };
+    this.ionSplitPaneVisible.emit({ visible });
-    this.ionSplitPaneVisible.emit(detail);
+  }
+
+  /**
+   * @internal
+   */
+  @Method()
+  async isVisible(): Promise<boolean> {
+    return Promise.resolve(this.visible);
   }
 
   async connectedCallback() {
@@ -68,7 +75,7 @@ export class SplitPane implements ComponentInterface {
     if (typeof (customElements as any) !== 'undefined' && (customElements as any) != null) {
       await customElements.whenDefined('ion-split-pane');
     }
-    this.styleChildren();
+    this.styleMainElement();
     this.updateState();
   }
 
@@ -125,17 +132,20 @@ export class SplitPane implements ComponentInterface {
     }
   }
 
-  private isPane(element: HTMLElement): boolean {
+  /**
-    if (!this.visible) {
+   * Attempt to find the main content
-      return false;
+   * element inside of the split pane.
-    }
+   * If found, set it as the main node.
-    return element.parentElement === this.el && element.classList.contains(SPLIT_PANE_SIDE);
+   *
-  }
+   * We assume that the main node
-
+   * is available in the DOM on split
-  private styleChildren() {
+   * pane load.
+   */
+  private styleMainElement() {
     if (!Build.isBrowser) {
       return;
     }
+
     const contentId = this.contentId;
     const children = this.el.children;
     const nu = this.el.childElementCount;
@@ -147,10 +157,11 @@ export class SplitPane implements ComponentInterface {
         if (foundMain) {
           console.warn('split pane cannot have more than one main node');
           return;
+        } else {
+          setPaneClass(child, isMain);
+          foundMain = true;
         }
-        foundMain = true;
       }
-      setPaneClass(child, isMain);
     }
     if (!foundMain) {
       console.warn('split pane does not have a specified main node');

core/src/components/split-pane/test/wrapped-menu/index.html

@@ -0,0 +1,69 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <meta charset="UTF-8" />
+    <title>Split Pane - Wrapped Menu</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 nomodule src="../../../../../dist/ionic/ionic.js"></script>
+    <script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
+    <style>
+      ion-split-pane {
+        --side-width: 200px;
+        --side-min-width: 200px;
+      }
+
+      app-menu {
+        display: flex;
+      }
+    </style>
+  </head>
+  <body>
+    <ion-app>
+      <ion-split-pane id="splitPane" content-id="split-content">
+        <app-menu side="start"></app-menu>
+        <div class="ion-page" id="split-content">
+          <ion-header>
+            <ion-toolbar>
+              <ion-buttons slot="start">
+                <ion-menu-button></ion-menu-button>
+              </ion-buttons>
+
+              <ion-title>Content</ion-title>
+            </ion-toolbar>
+          </ion-header>
+
+          <ion-content class="ion-padding"> Main Content </ion-content>
+        </div>
+      </ion-split-pane>
+    </ion-app>
+
+    <script>
+      class AppMenu extends HTMLElement {
+        constructor() {
+          super();
+        }
+
+        connectedCallback() {
+          this.innerHTML = `
+            <ion-menu side=${this.getAttribute('side')} content-id="split-content">
+              <ion-header>
+                <ion-toolbar color="secondary">
+                  <ion-title>Menu</ion-title>
+                </ion-toolbar>
+              </ion-header>
+              <ion-content class="ion-padding">Menu Content</ion-content>
+            </ion-menu>
+          `;
+        }
+      }
+
+      customElements.define('app-menu', AppMenu);
+    </script>
+  </body>
+</html>

core/src/components/split-pane/test/wrapped-menu/split-pane.e2e.ts

@@ -0,0 +1,47 @@
+import { expect } from '@playwright/test';
+import { configs, test, Viewports } from '@utils/test/playwright';
+
+configs({ directions: ['ltr'], modes: ['ios'] }).forEach(({ title, config }) => {
+  test.describe(title('split-pane: functionality'), () => {
+    test('should be visible on larger viewports', async ({ page }) => {
+      test.info().annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/16304',
+      });
+
+      await page.setViewportSize(Viewports.large);
+      await page.goto(`/src/components/split-pane/test/wrapped-menu`, config);
+
+      const menu = page.locator('ion-menu');
+      await expect(menu).toBeVisible();
+    });
+
+    test('should be visible on larger viewports when added async', async ({ page }) => {
+      test.info().annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/16304',
+      });
+
+      await page.setViewportSize(Viewports.large);
+      await page.goto(`/src/components/split-pane/test/wrapped-menu`, config);
+
+      // Asynchronously add a second menu
+      await page.evaluate(() => {
+        const menu = document.createElement('app-menu');
+        menu.setAttribute('side', 'end');
+        const splitPane = document.querySelector('ion-split-pane')!;
+        splitPane.appendChild(menu);
+      });
+
+      const menus = page.locator('ion-menu');
+      const menuButton = page.locator('ion-menu-button');
+
+      await expect(menus).toHaveCount(2);
+      await expect(menus.nth(0)).toBeVisible();
+      await expect(menus.nth(1)).toBeVisible();
+
+      // Menu button should be hidden because menus are visible
+      await expect(menuButton).toBeHidden();
+    });
+  });
+});