Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(menu): test all positioning values #5171

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions docs/components/figures/menu/usage-popover.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<div class="figure-wrapper">
<figure
style="justify-content: center"
aria-label="A filled button that says open popover menu. Interact with the button to open a popover menu."
>
<div>
<div style="margin: 16px">
<md-filled-button id="usage-popover-anchor">Open popover menu</md-filled-button>
</div>
<md-menu positioning="popover" id="usage-popover" anchor="usage-popover-anchor">
<md-menu-item>
<div slot="headline">Apple</div>
</md-menu-item>
<md-menu-item>
<div slot="headline">Banana</div>
</md-menu-item>
<md-menu-item>
<div slot="headline">Cucumber</div>
</md-menu-item>
</md-menu>
</div>
<script type="module">
const anchorEl = document.body.querySelector("#usage-popover-anchor");
const menuEl = document.body.querySelector("#usage-popover");
anchorEl.addEventListener("click", () => {
menuEl.open = !menuEl.open;
});
</script>
</figure>
</div>
Binary file added docs/components/images/menu/usage-popover.webp
Binary file not shown.
63 changes: 59 additions & 4 deletions docs/components/menu.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,69 @@ Granny Smith, and Red Delicious."](images/menu/usage-submenu.webp)
</script>
```

### Fixed-positioned menus
### Popover-positioned menus

Internally menu uses `position: absolute` by default. Though there are cases
when the anchor and the node cannot share a common ancestor that is `position:
relative`, or sometimes, menu will render below another item due to limitations
with `position: absolute`. In most of these cases, you would want to use the
`positioning="fixed"` attribute to position the menu relative to the window
instead of relative to the element.
with `position: absolute`.

Popover-positioned menus use the native
[Popover API](https://developer.mozilla.org/en-US/docs/Web/API/Popover_API)<!-- {.external} -->
to render above all other content. This may fix most issues where the default
menu positioning (`positioning="absolute"`) is not positioning as expected by
rendering into the
[top layer](google3/third_party/javascript/material/web/g3doc/docs/components/figures/menu/usage-fixed.html)<!-- {.external} -->.

> Warning: Popover API support was added in Chrome 114 and Safari 17. At the
> time of writing, Firefox does not support the Popover API
> ([see latest browser compatiblity](#fixed-positioned-menus)<!-- {.external} -->).
>
> For browsers that do not support the Popover API, `md-menu` will fall back to
> using [fixed-positioned menus](#fixed-positioned-menus).

<!-- no-catalog-start -->

!["A filled button that says open popover menu. There is an open menu anchored
to the bottom of the button with three items, Apple, Banana, and
Cucumber."](images/menu/usage-popover.webp)

<!-- no-catalog-end -->
<!-- catalog-include "figures/menu/usage-fixed.html" -->

```html
<!-- Note the lack of position: relative parent. -->
<div style="margin: 16px;">
<md-filled-button id="usage-popover-anchor">Open popover menu</md-filled-button>
</div>

<!-- popover menus do not require a common ancestor with the anchor. -->
<md-menu positioning="popover" id="usage-popover" anchor="usage-popover-anchor">
<md-menu-item>
<div slot="headline">Apple</div>
</md-menu-item>
<md-menu-item>
<div slot="headline">Banana</div>
</md-menu-item>
<md-menu-item>
<div slot="headline">Cucumber</div>
</md-menu-item>
</md-menu>

<script type="module">
const anchorEl = document.body.querySelector('#usage-popover-anchor');
const menuEl = document.body.querySelector('#usage-popover');

anchorEl.addEventListener('click', () => { menuEl.open = !menuEl.open; });
</script>
```

### Fixed-positioned menus

This is the fallback implementation of
[popover-positioned menus](#popover-positioned-menus) and uses `position: fixed`
rather than the default `position: absolute` which calculates its position
relative to the window rather than the element.

> Note: Fixed menu positions are positioned relative to the window and not the
> document. This means that the menu will not scroll with the anchor as the page
Expand Down
3 changes: 2 additions & 1 deletion menu/demo/demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,12 @@ const collection = new MaterialCollection<KnobTypesToKnobs<StoryKnobs>>(
}),
new Knob('positioning', {
defaultValue: 'absolute' as const,
ui: selectDropdown<'absolute' | 'fixed' | 'document'>({
ui: selectDropdown<'absolute' | 'fixed' | 'document' | 'popover'>({
options: [
{label: 'absolute', value: 'absolute'},
{label: 'fixed', value: 'fixed'},
{label: 'document', value: 'document'},
{label: 'popover', value: 'popover'},
],
}),
}),
Expand Down
2 changes: 1 addition & 1 deletion menu/demo/stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export interface StoryKnobs {
anchorCorner: Corner | undefined;
menuCorner: Corner | undefined;
defaultFocus: FocusState | undefined;
positioning: 'absolute' | 'fixed' | 'document' | undefined;
positioning: 'absolute' | 'fixed' | 'document' | 'popover' | undefined;
open: boolean;
quick: boolean;
hasOverflow: boolean;
Expand Down
13 changes: 12 additions & 1 deletion menu/internal/_menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@
.menu {
border-radius: map.get($tokens, 'container-shape');
display: none;
inset: auto;
border: none;
padding: 0px;
overflow: visible;
// [popover] adds a canvas background
background-color: transparent;
opacity: 0;
z-index: 20;
position: absolute;
Expand All @@ -70,6 +76,10 @@
max-width: inherit;
}

.menu::backdrop {
display: none;
}

.fixed {
position: fixed;
}
Expand All @@ -93,10 +103,11 @@
padding-block: 8px;
}

.has-overflow .items {
.has-overflow:not([popover]) .items {
overflow: visible;
}

.has-overflow.animating .items,
.animating .items {
overflow: hidden;
}
Expand Down
15 changes: 15 additions & 0 deletions menu/internal/controllers/surfacePositionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,14 @@ export class SurfacePositionController implements ReactiveController {
this.host.requestUpdate();
await this.host.updateComplete;

// Safari has a bug that makes popovers render incorrectly if the node is
// made visible + Animation Frame before calling showPopover().
// https://bugs.webkit.org/show_bug.cgi?id=264069
// also the cast is required due to differing TS types in Google and OSS.
if ((surfaceEl as unknown as {popover: string}).popover) {
(surfaceEl as unknown as {showPopover: () => void}).showPopover();
}

const surfaceRect = surfaceEl.getSurfacePositionClientRect
? surfaceEl.getSurfacePositionClientRect()
: surfaceEl.getBoundingClientRect();
Expand Down Expand Up @@ -600,5 +608,12 @@ export class SurfacePositionController implements ReactiveController {
'display': 'none',
};
this.host.requestUpdate();
const surfaceEl = this.getProperties().surfaceEl;

// The following type casts are required due to differing TS types in Google
// and open source.
if ((surfaceEl as unknown as {popover?: string})?.popover) {
(surfaceEl as unknown as {hidePopover: () => void}).hidePopover();
}
}
}
46 changes: 36 additions & 10 deletions menu/internal/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import '../../elevation/elevation.js';
import '../../focus/md-focus-ring.js';

import {html, isServer, LitElement, PropertyValues} from 'lit';
import {LitElement, PropertyValues, html, isServer, nothing} from 'lit';
import {property, query, queryAssignedElements, state} from 'lit/decorators.js';
import {ClassInfo, classMap} from 'lit/directives/class-map.js';
import {styleMap} from 'lit/directives/style-map.js';
Expand All @@ -16,7 +16,7 @@ import {
polyfillARIAMixin,
polyfillElementInternalsAria,
} from '../../internal/aria/aria.js';
import {createAnimationSignal, EASING} from '../../internal/motion/animation.js';
import {EASING, createAnimationSignal} from '../../internal/motion/animation.js';
import {
ListController,
NavigableKeys,
Expand Down Expand Up @@ -107,9 +107,12 @@ export abstract class Menu extends LitElement {
@property() anchor = '';
/**
* Whether the positioning algorithim should calculate relative to the parent
* of the anchor element (absolute) or relative to the window (fixed).
* of the anchor element (`absolute`), relative to the window (`fixed`), or
* relative to the document (`document`). `popover` will use the popover API
* to render the menu in the top-layer. If your browser does not support the
* popover API, it will revert to `fixed`.
*
* Examples for `position = 'fixed'`:
* __Examples for `position = 'fixed'`:__
*
* - If there is no `position:relative` in the given parent tree and the
* surface is `position:absolute`
Expand All @@ -118,7 +121,7 @@ export abstract class Menu extends LitElement {
* - The anchor and the surface do not share a common `position:relative`
* ancestor
*
* When using positioning = fixed, in most cases, the menu should position
* When using `positioning=fixed`, in most cases, the menu should position
* itself above most other `position:absolute` or `position:fixed` elements
* when placed inside of them. e.g. using a menu inside of an `md-dialog`.
*
Expand All @@ -134,8 +137,14 @@ export abstract class Menu extends LitElement {
* end of the `<body>` to render over everything or in a top-layer.
* - You are reusing a single `md-menu` element that dynamically renders
* content.
*
* __Examples for `position = 'popover'`:__
*
* - Your browser supports `popover`.
* - Most cases. Once popover is in browsers, this will become the default.
*/
@property() positioning: 'absolute' | 'fixed' | 'document' = 'absolute';
@property() positioning: 'absolute' | 'fixed' | 'document' | 'popover' =
'absolute';
/**
* Skips the opening and closing animations.
*/
Expand Down Expand Up @@ -362,7 +371,8 @@ export abstract class Menu extends LitElement {
surfaceCorner: this.menuCorner,
surfaceEl: this.surfaceEl,
anchorEl: this.anchorElement,
positioning: this.positioning,
positioning:
this.positioning === 'popover' ? 'document' : this.positioning,
isOpen: this.open,
xOffset: this.xOffset,
yOffset: this.yOffset,
Expand All @@ -372,7 +382,10 @@ export abstract class Menu extends LitElement {
// We can't resize components that have overflow like menus with
// submenus because the overflow-y will show menu items / content
// outside the bounds of the menu. (to be fixed w/ popover API)
repositionStrategy: this.hasOverflow ? 'move' : 'resize',
repositionStrategy:
this.hasOverflow && this.positioning !== 'popover'
? 'move'
: 'resize',
};
},
);
Expand Down Expand Up @@ -407,13 +420,25 @@ export abstract class Menu extends LitElement {
}
}

// Firefox does not support popover. Fall-back to using fixed.
if (
changed.has('positioning') &&
this.positioning === 'popover' &&
// type required for Google JS conformance
!(this as unknown as {showPopover?: () => void}).showPopover
) {
this.positioning = 'fixed';
}

super.update(changed);
}

private readonly onWindowResize = () => {
if (
this.isRepositioning ||
(this.positioning !== 'document' && this.positioning !== 'fixed')
(this.positioning !== 'document' &&
this.positioning !== 'fixed' &&
this.positioning !== 'popover')
) {
return;
}
Expand Down Expand Up @@ -445,7 +470,8 @@ export abstract class Menu extends LitElement {
return html`
<div
class="menu ${classMap(this.getSurfaceClasses())}"
style=${styleMap(this.menuPositionController.surfaceStyles)}>
style=${styleMap(this.menuPositionController.surfaceStyles)}
popover=${this.positioning === 'popover' ? 'manual' : nothing}>
${this.renderElevation()}
<div class="items">
<div class="item-padding"> ${this.renderMenuItems()} </div>
Expand Down
Loading