Skip to content

Commit afee41b

Browse files
Elliott Marquezcopybara-github
Elliott Marquez
authored andcommitted
test(menu): test all positioning values
PiperOrigin-RevId: 578986324
1 parent 5bb4a42 commit afee41b

File tree

8 files changed

+155
-17
lines changed

8 files changed

+155
-17
lines changed

Diff for: docs/components/figures/menu/usage-popover.html

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<div class="figure-wrapper">
2+
<figure
3+
style="justify-content: center"
4+
aria-label="A filled button that says open popover menu. Interact with the button to open a popover menu."
5+
>
6+
<div>
7+
<div style="margin: 16px">
8+
<md-filled-button id="usage-popover-anchor">Open popover menu</md-filled-button>
9+
</div>
10+
<md-menu positioning="popover" id="usage-popover" anchor="usage-popover-anchor">
11+
<md-menu-item>
12+
<div slot="headline">Apple</div>
13+
</md-menu-item>
14+
<md-menu-item>
15+
<div slot="headline">Banana</div>
16+
</md-menu-item>
17+
<md-menu-item>
18+
<div slot="headline">Cucumber</div>
19+
</md-menu-item>
20+
</md-menu>
21+
</div>
22+
<script type="module">
23+
const anchorEl = document.body.querySelector("#usage-popover-anchor");
24+
const menuEl = document.body.querySelector("#usage-popover");
25+
anchorEl.addEventListener("click", () => {
26+
menuEl.open = !menuEl.open;
27+
});
28+
</script>
29+
</figure>
30+
</div>

Diff for: docs/components/images/menu/usage-popover.webp

6.19 KB
Binary file not shown.

Diff for: docs/components/menu.md

+59-4
Original file line numberDiff line numberDiff line change
@@ -215,14 +215,69 @@ Granny Smith, and Red Delicious."](images/menu/usage-submenu.webp)
215215
</script>
216216
```
217217

218-
### Fixed-positioned menus
218+
### Popover-positioned menus
219219

220220
Internally menu uses `position: absolute` by default. Though there are cases
221221
when the anchor and the node cannot share a common ancestor that is `position:
222222
relative`, or sometimes, menu will render below another item due to limitations
223-
with `position: absolute`. In most of these cases, you would want to use the
224-
`positioning="fixed"` attribute to position the menu relative to the window
225-
instead of relative to the element.
223+
with `position: absolute`.
224+
225+
Popover-positioned menus use the native
226+
[Popover API](https://developer.mozilla.org/en-US/docs/Web/API/Popover_API)<!-- {.external} -->
227+
to render above all other content. This may fix most issues where the default
228+
menu positioning (`positioning="absolute"`) is not positioning as expected by
229+
rendering into the
230+
[top layer](google3/third_party/javascript/material/web/g3doc/docs/components/figures/menu/usage-fixed.html)<!-- {.external} -->.
231+
232+
> Warning: Popover API support was added in Chrome 114 and Safari 17. At the
233+
> time of writing, Firefox does not support the Popover API
234+
> ([see latest browser compatiblity](#fixed-positioned-menus)<!-- {.external} -->).
235+
>
236+
> For browsers that do not support the Popover API, `md-menu` will fall back to
237+
> using [fixed-positioned menus](#fixed-positioned-menus).
238+
239+
<!-- no-catalog-start -->
240+
241+
!["A filled button that says open popover menu. There is an open menu anchored
242+
to the bottom of the button with three items, Apple, Banana, and
243+
Cucumber."](images/menu/usage-popover.webp)
244+
245+
<!-- no-catalog-end -->
246+
<!-- catalog-include "figures/menu/usage-fixed.html" -->
247+
248+
```html
249+
<!-- Note the lack of position: relative parent. -->
250+
<div style="margin: 16px;">
251+
<md-filled-button id="usage-popover-anchor">Open popover menu</md-filled-button>
252+
</div>
253+
254+
<!-- popover menus do not require a common ancestor with the anchor. -->
255+
<md-menu positioning="popover" id="usage-popover" anchor="usage-popover-anchor">
256+
<md-menu-item>
257+
<div slot="headline">Apple</div>
258+
</md-menu-item>
259+
<md-menu-item>
260+
<div slot="headline">Banana</div>
261+
</md-menu-item>
262+
<md-menu-item>
263+
<div slot="headline">Cucumber</div>
264+
</md-menu-item>
265+
</md-menu>
266+
267+
<script type="module">
268+
const anchorEl = document.body.querySelector('#usage-popover-anchor');
269+
const menuEl = document.body.querySelector('#usage-popover');
270+
271+
anchorEl.addEventListener('click', () => { menuEl.open = !menuEl.open; });
272+
</script>
273+
```
274+
275+
### Fixed-positioned menus
276+
277+
This is the fallback implementation of
278+
[popover-positioned menus](#popover-positioned-menus) and uses `position: fixed`
279+
rather than the default `position: absolute` which calculates its position
280+
relative to the window rather than the element.
226281

227282
> Note: Fixed menu positions are positioned relative to the window and not the
228283
> document. This means that the menu will not scroll with the anchor as the page

Diff for: menu/demo/demo.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,12 @@ const collection = new MaterialCollection<KnobTypesToKnobs<StoryKnobs>>(
6464
}),
6565
new Knob('positioning', {
6666
defaultValue: 'absolute' as const,
67-
ui: selectDropdown<'absolute' | 'fixed' | 'document'>({
67+
ui: selectDropdown<'absolute' | 'fixed' | 'document' | 'popover'>({
6868
options: [
6969
{label: 'absolute', value: 'absolute'},
7070
{label: 'fixed', value: 'fixed'},
7171
{label: 'document', value: 'document'},
72+
{label: 'popover', value: 'popover'},
7273
],
7374
}),
7475
}),

Diff for: menu/demo/stories.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export interface StoryKnobs {
2222
anchorCorner: Corner | undefined;
2323
menuCorner: Corner | undefined;
2424
defaultFocus: FocusState | undefined;
25-
positioning: 'absolute' | 'fixed' | 'document' | undefined;
25+
positioning: 'absolute' | 'fixed' | 'document' | 'popover' | undefined;
2626
open: boolean;
2727
quick: boolean;
2828
hasOverflow: boolean;

Diff for: menu/internal/_menu.scss

+12-1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@
6060
.menu {
6161
border-radius: map.get($tokens, 'container-shape');
6262
display: none;
63+
inset: auto;
64+
border: none;
65+
padding: 0px;
66+
overflow: visible;
67+
// [popover] adds a canvas background
68+
background-color: transparent;
6369
opacity: 0;
6470
z-index: 20;
6571
position: absolute;
@@ -70,6 +76,10 @@
7076
max-width: inherit;
7177
}
7278

79+
.menu::backdrop {
80+
display: none;
81+
}
82+
7383
.fixed {
7484
position: fixed;
7585
}
@@ -93,10 +103,11 @@
93103
padding-block: 8px;
94104
}
95105

96-
.has-overflow .items {
106+
.has-overflow:not([popover]) .items {
97107
overflow: visible;
98108
}
99109

110+
.has-overflow.animating .items,
100111
.animating .items {
101112
overflow: hidden;
102113
}

Diff for: menu/internal/controllers/surfacePositionController.ts

+15
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,14 @@ export class SurfacePositionController implements ReactiveController {
196196
this.host.requestUpdate();
197197
await this.host.updateComplete;
198198

199+
// Safari has a bug that makes popovers render incorrectly if the node is
200+
// made visible + Animation Frame before calling showPopover().
201+
// https://bugs.webkit.org/show_bug.cgi?id=264069
202+
// also the cast is required due to differing TS types in Google and OSS.
203+
if ((surfaceEl as unknown as {popover: string}).popover) {
204+
(surfaceEl as unknown as {showPopover: () => void}).showPopover();
205+
}
206+
199207
const surfaceRect = surfaceEl.getSurfacePositionClientRect
200208
? surfaceEl.getSurfacePositionClientRect()
201209
: surfaceEl.getBoundingClientRect();
@@ -600,5 +608,12 @@ export class SurfacePositionController implements ReactiveController {
600608
'display': 'none',
601609
};
602610
this.host.requestUpdate();
611+
const surfaceEl = this.getProperties().surfaceEl;
612+
613+
// The following type casts are required due to differing TS types in Google
614+
// and open source.
615+
if ((surfaceEl as unknown as {popover?: string})?.popover) {
616+
(surfaceEl as unknown as {hidePopover: () => void}).hidePopover();
617+
}
603618
}
604619
}

Diff for: menu/internal/menu.ts

+36-10
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import '../../elevation/elevation.js';
88
import '../../focus/md-focus-ring.js';
99

10-
import {html, isServer, LitElement, PropertyValues} from 'lit';
10+
import {LitElement, PropertyValues, html, isServer, nothing} from 'lit';
1111
import {property, query, queryAssignedElements, state} from 'lit/decorators.js';
1212
import {ClassInfo, classMap} from 'lit/directives/class-map.js';
1313
import {styleMap} from 'lit/directives/style-map.js';
@@ -16,7 +16,7 @@ import {
1616
polyfillARIAMixin,
1717
polyfillElementInternalsAria,
1818
} from '../../internal/aria/aria.js';
19-
import {createAnimationSignal, EASING} from '../../internal/motion/animation.js';
19+
import {EASING, createAnimationSignal} from '../../internal/motion/animation.js';
2020
import {
2121
ListController,
2222
NavigableKeys,
@@ -107,9 +107,12 @@ export abstract class Menu extends LitElement {
107107
@property() anchor = '';
108108
/**
109109
* Whether the positioning algorithim should calculate relative to the parent
110-
* of the anchor element (absolute) or relative to the window (fixed).
110+
* of the anchor element (`absolute`), relative to the window (`fixed`), or
111+
* relative to the document (`document`). `popover` will use the popover API
112+
* to render the menu in the top-layer. If your browser does not support the
113+
* popover API, it will revert to `fixed`.
111114
*
112-
* Examples for `position = 'fixed'`:
115+
* __Examples for `position = 'fixed'`:__
113116
*
114117
* - If there is no `position:relative` in the given parent tree and the
115118
* surface is `position:absolute`
@@ -118,7 +121,7 @@ export abstract class Menu extends LitElement {
118121
* - The anchor and the surface do not share a common `position:relative`
119122
* ancestor
120123
*
121-
* When using positioning = fixed, in most cases, the menu should position
124+
* When using `positioning=fixed`, in most cases, the menu should position
122125
* itself above most other `position:absolute` or `position:fixed` elements
123126
* when placed inside of them. e.g. using a menu inside of an `md-dialog`.
124127
*
@@ -134,8 +137,14 @@ export abstract class Menu extends LitElement {
134137
* end of the `<body>` to render over everything or in a top-layer.
135138
* - You are reusing a single `md-menu` element that dynamically renders
136139
* content.
140+
*
141+
* __Examples for `position = 'popover'`:__
142+
*
143+
* - Your browser supports `popover`.
144+
* - Most cases. Once popover is in browsers, this will become the default.
137145
*/
138-
@property() positioning: 'absolute' | 'fixed' | 'document' = 'absolute';
146+
@property() positioning: 'absolute' | 'fixed' | 'document' | 'popover' =
147+
'absolute';
139148
/**
140149
* Skips the opening and closing animations.
141150
*/
@@ -362,7 +371,8 @@ export abstract class Menu extends LitElement {
362371
surfaceCorner: this.menuCorner,
363372
surfaceEl: this.surfaceEl,
364373
anchorEl: this.anchorElement,
365-
positioning: this.positioning,
374+
positioning:
375+
this.positioning === 'popover' ? 'document' : this.positioning,
366376
isOpen: this.open,
367377
xOffset: this.xOffset,
368378
yOffset: this.yOffset,
@@ -372,7 +382,10 @@ export abstract class Menu extends LitElement {
372382
// We can't resize components that have overflow like menus with
373383
// submenus because the overflow-y will show menu items / content
374384
// outside the bounds of the menu. (to be fixed w/ popover API)
375-
repositionStrategy: this.hasOverflow ? 'move' : 'resize',
385+
repositionStrategy:
386+
this.hasOverflow && this.positioning !== 'popover'
387+
? 'move'
388+
: 'resize',
376389
};
377390
},
378391
);
@@ -407,13 +420,25 @@ export abstract class Menu extends LitElement {
407420
}
408421
}
409422

423+
// Firefox does not support popover. Fall-back to using fixed.
424+
if (
425+
changed.has('positioning') &&
426+
this.positioning === 'popover' &&
427+
// type required for Google JS conformance
428+
!(this as unknown as {showPopover?: () => void}).showPopover
429+
) {
430+
this.positioning = 'fixed';
431+
}
432+
410433
super.update(changed);
411434
}
412435

413436
private readonly onWindowResize = () => {
414437
if (
415438
this.isRepositioning ||
416-
(this.positioning !== 'document' && this.positioning !== 'fixed')
439+
(this.positioning !== 'document' &&
440+
this.positioning !== 'fixed' &&
441+
this.positioning !== 'popover')
417442
) {
418443
return;
419444
}
@@ -445,7 +470,8 @@ export abstract class Menu extends LitElement {
445470
return html`
446471
<div
447472
class="menu ${classMap(this.getSurfaceClasses())}"
448-
style=${styleMap(this.menuPositionController.surfaceStyles)}>
473+
style=${styleMap(this.menuPositionController.surfaceStyles)}
474+
popover=${this.positioning === 'popover' ? 'manual' : nothing}>
449475
${this.renderElevation()}
450476
<div class="items">
451477
<div class="item-padding"> ${this.renderMenuItems()} </div>

0 commit comments

Comments
 (0)