Skip to content

Commit 76f6362

Browse files
authored
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`
1 parent e1f9850 commit 76f6362

File tree

9 files changed

+236
-95
lines changed

9 files changed

+236
-95
lines changed

core/src/components.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -2852,6 +2852,7 @@ export namespace Components {
28522852
* If `true`, the split pane will be hidden.
28532853
*/
28542854
"disabled": boolean;
2855+
"isVisible": () => Promise<boolean>;
28552856
/**
28562857
* When the split-pane should be shown. Can be a CSS media query expression, or a shortcut expression. Can also be a boolean expression.
28572858
*/

core/src/components/menu/menu.scss

+59-3
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,51 @@ ion-backdrop {
179179
// Menu Split Pane
180180
// --------------------------------------------------
181181

182+
/**
183+
* The split pane styles for menu are defined
184+
* in the menu stylesheets instead in the split pane
185+
* stylesheets with ::slotted to allow for menus
186+
* to be wrapped in custom components.
187+
* If we used ::slotted to target the menu
188+
* then menus wrapped in components would never
189+
* receive these styles because they are not
190+
* children of the split pane.
191+
*/
192+
193+
/**
194+
* Do not pass CSS Variables down on larger
195+
* screens as we want them to affect the outer
196+
* `ion-menu` rather than the inner content
197+
*/
182198
:host(.menu-pane-visible) {
183-
width: var(--width);
184-
min-width: var(--min-width);
185-
max-width: var(--max-width);
199+
flex: 0 1 auto;
200+
201+
width: var(--side-width, var(--width));
202+
min-width: var(--side-min-width, var(--min-width));
203+
max-width: var(--side-max-width, var(--max-width));
204+
}
205+
206+
:host(.menu-pane-visible.split-pane-side) {
207+
@include position(0, 0, 0, 0);
208+
209+
position: relative;
210+
211+
box-shadow: none;
212+
z-index: 0;
213+
}
214+
215+
:host(.menu-pane-visible.split-pane-side.menu-enabled) {
216+
display: flex;
217+
218+
flex-shrink: 0;
219+
}
220+
221+
:host(.menu-pane-visible.split-pane-side) {
222+
order: -1;
223+
}
224+
225+
:host(.menu-pane-visible.split-pane-side[side=end]) {
226+
order: 1;
186227
}
187228

188229
:host(.menu-pane-visible) .menu-inner {
@@ -199,3 +240,18 @@ ion-backdrop {
199240
/* stylelint-disable-next-line declaration-no-important */
200241
display: hidden !important;
201242
}
243+
244+
:host(.menu-pane-visible.split-pane-side) {
245+
@include border(0, var(--border), 0, 0);
246+
247+
min-width: var(--side-min-width);
248+
max-width: var(--side-max-width);
249+
}
250+
251+
:host(.menu-pane-visible.split-pane-side[side=end]) {
252+
@include border(0, 0, 0, var(--border));
253+
254+
min-width: var(--side-min-width);
255+
max-width: var(--side-max-width);
256+
}
257+

core/src/components/menu/menu.tsx

+33-17
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { Attributes } from '@utils/helpers';
66
import { inheritAriaAttributes, assert, clamp, isEndSide as isEnd } from '@utils/helpers';
77
import { menuController } from '@utils/menu-controller';
88
import { getPresentedOverlay } from '@utils/overlays';
9+
import { hostContext } from '@utils/theme';
910

1011
import { config } from '../../global/config';
1112
import { getIonMode } from '../../global/ionic-global';
@@ -77,6 +78,14 @@ export class Menu implements ComponentInterface, MenuI {
7778

7879
@Element() el!: HTMLIonMenuElement;
7980

81+
/**
82+
* If true, then the menu should be
83+
* visible within a split pane.
84+
* If false, then the menu is hidden.
85+
* However, the menu-button/menu-toggle
86+
* components can be used to open the
87+
* menu.
88+
*/
8089
@State() isPaneVisible = false;
8190
@State() isEndSide = false;
8291

@@ -225,8 +234,8 @@ export class Menu implements ComponentInterface, MenuI {
225234

226235
// register this menu with the app's menu controller
227236
menuController._register(this);
228-
this.menuChanged();
229237

238+
this.menuChanged();
230239
this.gesture = (await import('../../utils/gesture')).createGesture({
231240
el: document,
232241
gestureName: 'menu-swipe',
@@ -248,6 +257,21 @@ export class Menu implements ComponentInterface, MenuI {
248257

249258
async componentDidLoad() {
250259
this.didLoad = true;
260+
261+
/**
262+
* A menu inside of a split pane is assumed
263+
* to be a side pane.
264+
*
265+
* When the menu is loaded it needs to
266+
* see if it should be considered visible inside
267+
* of the split pane. If the split pane is
268+
* hidden then the menu should be too.
269+
*/
270+
const splitPane = this.el.closest('ion-split-pane');
271+
if (splitPane !== null) {
272+
this.isPaneVisible = await splitPane.isVisible();
273+
}
274+
251275
this.menuChanged();
252276
this.updateState();
253277
}
@@ -288,23 +312,14 @@ export class Menu implements ComponentInterface, MenuI {
288312
}
289313

290314
@Listen('ionSplitPaneVisible', { target: 'body' })
291-
onSplitPaneChanged(ev: CustomEvent) {
292-
const { target } = ev;
293-
const closestSplitPane = this.el.closest('ion-split-pane');
315+
onSplitPaneChanged(ev: CustomEvent<{ visible: boolean }>) {
316+
const closestSplitPane = this.el.closest<HTMLIonSplitPaneElement>('ion-split-pane');
294317

295-
/**
296-
* Menu listens on the body for "ionSplitPaneVisible".
297-
* However, this means the callback will run any time
298-
* a SplitPane changes visibility. As a result, we only want
299-
* Menu's visibility state to update if its parent SplitPane
300-
* changes visibility.
301-
*/
302-
if (target !== closestSplitPane) {
303-
return;
304-
}
318+
if (closestSplitPane !== null && closestSplitPane === ev.target) {
319+
this.isPaneVisible = ev.detail.visible;
305320

306-
this.isPaneVisible = ev.detail.isPane(this.el);
307-
this.updateState();
321+
this.updateState();
322+
}
308323
}
309324

310325
@Listen('click', { capture: true })
@@ -778,7 +793,7 @@ export class Menu implements ComponentInterface, MenuI {
778793
}
779794

780795
render() {
781-
const { type, disabled, isPaneVisible, inheritedAttributes, side } = this;
796+
const { type, disabled, el, isPaneVisible, inheritedAttributes, side } = this;
782797
const mode = getIonMode(this);
783798

784799
return (
@@ -791,6 +806,7 @@ export class Menu implements ComponentInterface, MenuI {
791806
'menu-enabled': !disabled,
792807
[`menu-side-${side}`]: true,
793808
'menu-pane-visible': isPaneVisible,
809+
'split-pane-side': hostContext('ion-split-pane', el),
794810
}}
795811
>
796812
<div class="menu-inner" part="container" ref={(el) => (this.menuInnerEl = el)}>

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

-14
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,3 @@
99
--side-min-width: #{$split-pane-ios-side-min-width};
1010
--side-max-width: #{$split-pane-ios-side-max-width};
1111
}
12-
13-
:host(.split-pane-visible) ::slotted(.split-pane-side) {
14-
@include border(0, var(--border), 0, 0);
15-
16-
min-width: var(--side-min-width);
17-
max-width: var(--side-max-width);
18-
}
19-
20-
:host(.split-pane-visible) ::slotted(.split-pane-side[side=end]) {
21-
@include border(0, 0, 0, var(--border));
22-
23-
min-width: var(--side-min-width);
24-
max-width: var(--side-max-width);
25-
}

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

-14
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,3 @@
99
--side-min-width: #{$split-pane-md-side-min-width};
1010
--side-max-width: #{$split-pane-md-side-max-width};
1111
}
12-
13-
:host(.split-pane-visible) ::slotted(.split-pane-side) {
14-
@include border(0, var(--border), 0, 0);
15-
16-
min-width: var(--side-min-width);
17-
max-width: var(--side-max-width);
18-
}
19-
20-
:host(.split-pane-visible) ::slotted(.split-pane-side[side=end]) {
21-
@include border(0, 0, 0, var(--border));
22-
23-
min-width: var(--side-min-width);
24-
max-width: var(--side-max-width);
25-
}

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

+2-33
Original file line numberDiff line numberDiff line change
@@ -24,48 +24,17 @@
2424
contain: strict;
2525
}
2626

27-
/**
28-
* Do not pass CSS Variables down on larger
29-
* screens as we want them to affect the outer
30-
* `ion-menu` rather than the inner content
31-
*/
32-
::slotted(ion-menu.menu-pane-visible) {
33-
flex: 0 1 auto;
34-
35-
width: var(--side-width);
36-
min-width: var(--side-min-width);
37-
max-width: var(--side-max-width);
38-
}
39-
40-
:host(.split-pane-visible) ::slotted(.split-pane-side),
4127
:host(.split-pane-visible) ::slotted(.split-pane-main) {
4228
@include position(0, 0, 0, 0);
4329

4430
position: relative;
4531

46-
box-shadow: none;
47-
z-index: 0;
48-
}
49-
50-
:host(.split-pane-visible) ::slotted(.split-pane-main) {
5132
flex: 1;
52-
}
5333

54-
:host(.split-pane-visible) ::slotted(.split-pane-side:not(ion-menu)),
55-
:host(.split-pane-visible) ::slotted(ion-menu.split-pane-side.menu-enabled) {
56-
display: flex;
57-
58-
flex-shrink: 0;
34+
box-shadow: none;
35+
z-index: 0;
5936
}
6037

6138
::slotted(.split-pane-side:not(ion-menu)) {
6239
display: none;
6340
}
64-
65-
:host(.split-pane-visible) ::slotted(.split-pane-side) {
66-
order: -1;
67-
}
68-
69-
:host(.split-pane-visible) ::slotted(.split-pane-side[side=end]) {
70-
order: 1;
71-
}

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

+25-14
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ComponentInterface, EventEmitter } from '@stencil/core';
2-
import { Build, Component, Element, Event, Host, Prop, State, Watch, h } from '@stencil/core';
2+
import { Build, Component, Element, Event, Host, Method, Prop, State, Watch, h } from '@stencil/core';
33

44
import { getIonMode } from '../../global/ionic-global';
55

@@ -58,8 +58,15 @@ export class SplitPane implements ComponentInterface {
5858

5959
@Watch('visible')
6060
visibleChanged(visible: boolean) {
61-
const detail = { visible, isPane: this.isPane.bind(this) };
62-
this.ionSplitPaneVisible.emit(detail);
61+
this.ionSplitPaneVisible.emit({ visible });
62+
}
63+
64+
/**
65+
* @internal
66+
*/
67+
@Method()
68+
async isVisible(): Promise<boolean> {
69+
return Promise.resolve(this.visible);
6370
}
6471

6572
async connectedCallback() {
@@ -68,7 +75,7 @@ export class SplitPane implements ComponentInterface {
6875
if (typeof (customElements as any) !== 'undefined' && (customElements as any) != null) {
6976
await customElements.whenDefined('ion-split-pane');
7077
}
71-
this.styleChildren();
78+
this.styleMainElement();
7279
this.updateState();
7380
}
7481

@@ -125,17 +132,20 @@ export class SplitPane implements ComponentInterface {
125132
}
126133
}
127134

128-
private isPane(element: HTMLElement): boolean {
129-
if (!this.visible) {
130-
return false;
131-
}
132-
return element.parentElement === this.el && element.classList.contains(SPLIT_PANE_SIDE);
133-
}
134-
135-
private styleChildren() {
135+
/**
136+
* Attempt to find the main content
137+
* element inside of the split pane.
138+
* If found, set it as the main node.
139+
*
140+
* We assume that the main node
141+
* is available in the DOM on split
142+
* pane load.
143+
*/
144+
private styleMainElement() {
136145
if (!Build.isBrowser) {
137146
return;
138147
}
148+
139149
const contentId = this.contentId;
140150
const children = this.el.children;
141151
const nu = this.el.childElementCount;
@@ -147,10 +157,11 @@ export class SplitPane implements ComponentInterface {
147157
if (foundMain) {
148158
console.warn('split pane cannot have more than one main node');
149159
return;
160+
} else {
161+
setPaneClass(child, isMain);
162+
foundMain = true;
150163
}
151-
foundMain = true;
152164
}
153-
setPaneClass(child, isMain);
154165
}
155166
if (!foundMain) {
156167
console.warn('split pane does not have a specified main node');

0 commit comments

Comments
 (0)