yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

Merge pull request #176: context menus are a bottom sheet on a phone
CI / check (push) Successful in 2m27s
Details
CI / e2e (push) Successful in 9m9s
Details

Browse Source
This commit was merged in pull request #176.
This commit is contained in:
logan
2026-08-21 07:25:55 +00:00
parent 3871d37fdb 4b2eec5703
commit 14e3ab574c
17 changed files with 982 additions and 170 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.planning/NOTES.md
+78
View File
@@ -4707,3 +4707,81 @@ measurement taken was of a screen with `job-band` on it and the art at
route other than its own** (#175) — it was still up, full-screen and
intercepting pointer events, after `AddLibrary` succeeded through the
binding, and was gone after a relaunch. Filed.
## The context menu was clipped on the device, and the fix needed four measurements nothing here could make (measured 2026-08-21, TLP301 / Chrome 113 / 424x439)
#60 had been diagnosed from the Web Awesome source and was right. What
the device added was the numbers, and three things the reading had not
reached.
**The clip, reproduced before any code was written.** Long-press on the
lowest visible track row at 424x439:
| | |
|---|---|
| viewport | 424x439 |
| `.main-panel` | 0 to **318**, computed `contain: content` |
| menu panel | 191 to **401**, 210px tall |
| clipped away | **83px, three of seven items** |
| `wa-popup` computed position | `fixed` |
| `HTMLElement.prototype.hasOwnProperty('popover')` | **false** |
| row height | **29px** (against a 44px floor and a 48px ask) |
A screenshot shows the menu sliced off flush with the mini player's top
edge. Both halves of the diagnosis are therefore measured, not inferred.
**"A dialog escapes containment" was the premise, and it was untested.**
Every dialog in this app is mounted in `index.html`, *outside*
`.main-panel` — so nothing here was evidence about a dialog opened from
inside a view, which is what this change needed. A probe `<dialog>`
appended to `track-list`'s shadow root and `showModal()`n paints to
y=439, over the mini player and the tab bar. A top-layer element's
containing block is the viewport, paint-contained ancestor or not.
Checking that first cost ten minutes and would have cost a rebuild.
**The UA stylesheet is the thing that makes a naive sheet look wrong.**
That same probe came out **354px wide on a 424px screen**, centred,
because a native `<dialog>` carries `max-width: calc(100% - 6px - 2em)`
and `margin: auto`. `max-width: none` and explicit margins are four
declarations that are pure undoing.
**A retry loop cannot win against a steal that happens later.**
`MenuKeyboard` focuses the first item and returns as soon as it lands;
`wa-dialog` then focuses `[autofocus]` or *itself* on the frame after
`showModal()`, and it cannot see our first item to prefer it — the
panel is slotted through `menu-surface`, so the dialog's own
`querySelector` stops at the `<slot>`. Measured: the sheet opened with
`document.activeElement` on the `<dialog>` and every arrow key went
nowhere. Lengthening the retry budget does not help, because the first
attempt *succeeds*. The surface announcing `menu-shown` after
`wa-after-show`, and the keyboard re-asserting, is the fix.
**The submenu was made worse before it was made better, and only a
measurement caught it.** `#playlist-submenu` is a
`placement="right-start"` flyout anchored to its row. Making the menu a
full-width sheet moved that anchor to x=0, so the flip put the playlist
picker at **x −182 to 0 — entirely off-screen**, and "Add to Playlist"
led nowhere at all. Before the change the anchor row started at x≈245
and the same flip landed it on screen. It is a `menu-surface` too now
and stacks as a second sheet. Two lessons: a change that moves an
anchor changes every flip decision downstream of it, and *the scope I
declared on the issue was wrong* — I had said I would measure the
submenu and file it, and the measurement said fix it.
**And the sweep found two call sites the conversion missed.** Twelve
were converted by hand; `menu-surface.test.ts` reads every source file
and fails on a `<wa-popup>` outside a three-file allowlist, which
immediately named `queue-panel`'s add-to-playlist popup (a real menu,
converted) and `now-playing`'s cover preview (a hover affordance in the
bottom bar — allowlisted, since a touch device never opens it and
nothing clips it). A thirteenth menu written as a bare popup would pass
every tier here and be clipped on the device, which is precisely why
the guard is a source sweep rather than a rendered assertion.
**What no tier here can see remains the clip itself.** This runner's
Chromium and CI's WebKit both have the Popover API, so the popup is
top-layered and correct and a "not clipped" assertion passes on the
broken build. The specs assert the *mechanism* — that the surface is a
native `<dialog>` at phone width — which is the same move
`queue-as-a-screen.spec.ts` makes about containment and for the same
reason.
CLAUDE.md
+74 -2
View File
@@ -1367,8 +1367,17 @@ against the real components:
`wa-dropdown-item` sets its `role` in its *own* first update, so a
`[role^="menuitem"]` query at `updateComplete` finds nothing — which
reads exactly like a menu that opened and refused to take focus.
- **`focus()` on a popup that has not positioned itself is a silent
no-op**, so the first focus is retried across a few frames.
- **`focus()` on a surface that has not shown itself is a silent
no-op**, so the first focus is retried on a *time* budget rather
than a frame count — the thing being waited for is another
component's animation. And a retry is not enough on its own for the
sheet below: `wa-dialog` focuses `[autofocus]` or *itself* on the
frame after `showModal()`, and it cannot see the first menu item to
prefer it, because the panel is slotted through `menu-surface` and
the dialog's own `querySelector` stops at the `<slot>`. The first
attempt therefore *succeeds* and is then overwritten, which no
amount of waiting fixes — so the surface announces `menu-shown` when
it has settled and `MenuKeyboard.refocus()` re-asserts.
- **Focus is only taken back if the menu had it.** A click elsewhere
closes the menu too, and pulling focus to the row the user
right-clicked a moment ago is worse than leaving it.
@@ -1376,6 +1385,69 @@ against the real components:
moving focus without setting it leaves the highlight on whichever
item the mouse last touched.
**And a menu is drawn where it fits: a popup on a desktop, a bottom
sheet on a phone** (#60). `components/menu-surface/` is that one
decision. The host renders the panel it always rendered and slots it
into whichever surface is up, so `ContextMenuController` still drives
`.active` and `.anchor` as though it were talking to a `wa-popup`, and
fourteen call sites changed one tag name each and nothing else.
**It is a correctness fix, not a taste one, and the failure was
measured on the device rather than inferred.** Chrome 113 has no
Popover API, so `wa-popup` takes its own documented fallback and
positions with `strategy: "fixed"`; `.main-panel` carries
`contain: layout style paint`, and paint containment *clips* fixed
descendants. On the reference device the main panel spans 0-318 of a
439px viewport while the open menu spanned 191-401 — three of its seven
items cut off, with no way to reach them. `showModal()` is Chrome 37
and uses the real top layer, so a dialog is immune by construction.
Six things about it are load-bearing.
**"Dialogs are fine" needed checking, because every other dialog in
this app is mounted in `index.html`** — outside `.main-panel` — so it
was not evidence about one opened from inside a view. A probe dialog
appended to `track-list`'s shadow root paints to y=439, over the mini
player and the tab bar, with the contained ancestor still in place. A
top-layer element's containing block is the viewport, contained
ancestor or not.
**The sheet has to un-do the UA stylesheet.** A native `<dialog>`
carries `max-width: calc(100% - 6px - 2em)` and `margin: auto`, which
drew a 354px panel floating in the middle of a 424px screen.
`max-width: none` plus explicit margins is what makes it a sheet.
**The row sizing lives in `contextMenuStyles`, not in the component.**
The panel is the *host's* light DOM — it stays in the host's shadow
root, so only the host's stylesheet can reach it. `menu-surface` puts
`data-sheet` on the panel and that shared stylesheet does the rest,
which is how fourteen menus went from 29px rows to 48px ones in one
edit.
**A dismissal has to travel back.** `wa-dialog` closes itself on
Escape, which would leave the controller believing the menu is open —
and the failure mode is not a stuck sheet but the *next* long-press
doing nothing, which reads as the gesture breaking. `menu-dismiss` is
that signal; the three surfaces that do not use `ContextMenuController`
bind it themselves.
**The playlist submenu is a sheet too, and it had to be.** It is a
`placement="right-start"` flyout, and making the menu full-width moved
its anchor — measured at x −182 to 0, entirely off-screen, so "Add to
Playlist" led nowhere. It stacks as a second sheet over the first,
which is also why `menu-shown` does not re-assert focus while the
submenu is open.
**And which call sites exist is swept, not remembered.** A thirteenth
menu written as a bare `<wa-popup>` works perfectly in every tier here
and is clipped on the device, so `menu-surface.test.ts` reads the
source and fails on one outside a three-file allowlist —
`menu-surface` itself, `job-indicator` (in `.top-bar`, which no
ancestor contains — the contrast that proved the diagnosis on #62) and
`now-playing`'s cover preview (a hover affordance, which a touch device
never opens). **The sweep found two of the fourteen**; twelve were
converted by hand.
**And a menu opens from a finger, through the event it already has.**
`utils/long-press.ts` is one document-capture listener installed once
from `index.ts`: a touch that holds still for 500 ms dispatches a
frontend/src/components/artists-view/artists-view.ts
+12 -15
View File
@@ -26,14 +26,15 @@ import {
contextMenuStyles,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import { FavoritesController } from '@store/controllers/favorites-controller';
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
import { RovingGridController } from '@utils/roving-grid';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import '@components/playlist-picker/playlist-picker.js';
import { dict, list } from '@utils/binding';
@@ -131,17 +132,17 @@ export class ArtistsView
private contextMenuArtistId: number | null = null;
@query('#context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup!: WaPopup;
private playlistSubmenuPopup!: MenuSurface;
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup():
| WaPopup
| MenuTarget
| undefined {
return this.playlistSubmenuPopup;
}
@@ -1336,11 +1337,8 @@ export class ArtistsView
private renderContextMenu() {
return html`
<wa-popup
<menu-surface
id="context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu
.contextMenuOpen}
>
@@ -1434,13 +1432,12 @@ export class ArtistsView
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${this.ctxMenu
.playlistSubmenuOpen}
>
@@ -1468,7 +1465,7 @@ export class ArtistsView
</div>
`
: nothing}
</wa-popup>
</menu-surface>
`;
}
frontend/src/components/cover-grid/cover-grid.ts
+12 -15
View File
@@ -23,7 +23,8 @@ import { gridColumnsFor, gridSpacingFor } from '@utils/grid-spacing';
import { queueStore } from '@store/queue-store';
import type { QueueSource } from '@store/queue-store';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@components/playlist-picker/playlist-picker.js';
@@ -51,7 +52,7 @@ import {
ContextMenuController,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import { FavoritesController } from '@store/controllers/favorites-controller';
import { creditLink, exploreLinkStyles } from '../../utils/explore-link';
import { creditStore } from '@store/credit-store';
@@ -415,17 +416,17 @@ export class CoverGrid
splitIndex = 0;
@query('#context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup!: WaPopup;
private playlistSubmenuPopup!: MenuSurface;
// ContextMenuHost interface.
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup(): WaPopup | undefined {
getPlaylistSubmenuPopup(): MenuTarget | undefined {
return this.playlistSubmenuPopup;
}
@@ -2087,11 +2088,8 @@ export class CoverGrid
const { ctxMenu } = this;
return html`
<wa-popup
<menu-surface
id="context-menu"
placement="bottom-start"
flip
shift
.active=${ctxMenu.contextMenuOpen}
>
${ctxMenu.contextMenuOpen
@@ -2204,13 +2202,12 @@ export class CoverGrid
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${ctxMenu.playlistSubmenuOpen}
>
${ctxMenu.playlistSubmenuOpen
@@ -2232,7 +2229,7 @@ export class CoverGrid
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<track-details></track-details>
`;
frontend/src/components/explore-album-details/explore-album-details.ts
+11 -13
View File
@@ -49,9 +49,11 @@ import {
contextMenuStyles,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import { dictByName } from '@utils/binding';
import type { TrackDetails } from '@components/track-details/track-details.js';
@@ -327,7 +329,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
@state() private ctxMenuTrack: MBTrack | null = null;
@query('#track-context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup?: WaPopup;
@@ -337,11 +339,11 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
// -- ContextMenuHost interface --
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup(): WaPopup | undefined {
getPlaylistSubmenuPopup(): MenuTarget | undefined {
return this.playlistSubmenuPopup;
}
@@ -3781,11 +3783,8 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
const track = this.ctxMenuTrack;
return html`
<wa-popup
<menu-surface
id="track-context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu.contextMenuOpen}
>
${this.ctxMenu.contextMenuOpen && track
@@ -3846,13 +3845,12 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${this.ctxMenu.playlistSubmenuOpen}
>
${this.ctxMenu.playlistSubmenuOpen
@@ -3869,7 +3867,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
</div>
`
: nothing}
</wa-popup>
</menu-surface>
`;
}
}
frontend/src/components/explore-artist-details/explore-artist-details.ts
+11 -13
View File
@@ -61,9 +61,11 @@ import {
contextMenuStyles,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import { dict, dictByName } from '@utils/binding';
import type { TrackDetails } from '@components/track-details/track-details.js';
@@ -215,7 +217,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
@state() private ctxMenuTarget: ContextMenuTarget | null = null;
@query('#context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup?: WaPopup;
@@ -233,11 +235,11 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
// -- ContextMenuHost interface --
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup(): WaPopup | undefined {
getPlaylistSubmenuPopup(): MenuTarget | undefined {
return this.playlistSubmenuPopup;
}
@@ -2621,11 +2623,8 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
const target = this.ctxMenuTarget;
return html`
<wa-popup
<menu-surface
id="context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu.contextMenuOpen}
>
${this.ctxMenu.contextMenuOpen && target
@@ -2643,13 +2642,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${this.ctxMenu.playlistSubmenuOpen}
>
${this.ctxMenu.playlistSubmenuOpen
@@ -2666,7 +2664,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
</div>
`
: nothing}
</wa-popup>
</menu-surface>
`;
}
frontend/src/components/explore-view/explore-view.ts
+7 -8
View File
@@ -37,9 +37,11 @@ import {
contextMenuStyles,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import { dict, dictByName } from '@utils/binding';
import { ICON_QUEUE } from '@utils/icon-language';
@@ -206,13 +208,13 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
@state() private ctxMenuTarget: ExploreMenuTarget | null = null;
@litQuery('#explore-context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
// -- ContextMenuHost interface --
// No playlist submenu — same reason as the album/artist detail
// pages: every action here resolves its one file lazily.
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
@@ -1341,11 +1343,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
const owned = Boolean(target?.localId);
return html`
<wa-popup
<menu-surface
id="explore-context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu.contextMenuOpen}
>
${this.ctxMenu.contextMenuOpen && target
@@ -1374,7 +1373,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
</div>
`
: nothing}
</wa-popup>
</menu-surface>
`;
}
frontend/src/components/genres-view/genres-view.ts
+12 -15
View File
@@ -24,14 +24,15 @@ import {
contextMenuStyles,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import { FavoritesController } from '@store/controllers/favorites-controller';
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
import { RovingGridController } from '@utils/roving-grid';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import '@components/playlist-picker/playlist-picker.js';
import { dictByName } from '@utils/binding';
@@ -137,19 +138,19 @@ export class GenresView
private contextMenuGenreName: string | null = null;
@query('#context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup!: WaPopup;
private playlistSubmenuPopup!: MenuSurface;
// ----- ContextMenuHost interface -----
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup():
| WaPopup
| MenuTarget
| undefined {
return this.playlistSubmenuPopup;
}
@@ -1176,11 +1177,8 @@ export class GenresView
private renderContextMenu() {
return html`
<wa-popup
<menu-surface
id="context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu
.contextMenuOpen}
>
@@ -1284,13 +1282,12 @@ export class GenresView
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${this.ctxMenu
.playlistSubmenuOpen}
>
@@ -1318,7 +1315,7 @@ export class GenresView
</div>
`
: nothing}
</wa-popup>
</menu-surface>
`;
}
frontend/src/components/menu-surface/menu-surface.ts
+318
View File
@@ -0,0 +1,318 @@
/**
* Where a context menu is drawn: a popup on a desktop, a bottom sheet
* on a phone (#60).
*
* Every context menu in this app is a `.context-menu-panel` inside a
* `<wa-popup>` anchored to the touch point, driven by
* `ContextMenuController`. On the reference device that is structurally
* broken, and the failure was measured on the hardware rather than
* inferred:
*
* - Chrome 113 has **no Popover API** (`popover` is Chrome 114), so
* `wa-popup` takes its own documented fallback and positions with
* `strategy: "fixed"` instead of the top layer. Measured on the
* device: `HTMLElement.prototype.hasOwnProperty('popover')` is false
* and the popup's computed `position` is `fixed`.
* - `index.css` puts `contain: layout style paint` on `.main-panel`,
* the ancestor of every view. Paint containment **clips** fixed
* descendants. Measured: `.main-panel` computes `contain: content`
* and spans 0-318 of a 439px viewport, while the open menu spans
* 191-401 — so 83px of it, three of its seven items, is cut off.
*
* A `<dialog>` fixes it by construction rather than by styling, because
* `showModal()` is Chrome 37 and uses the real top layer. **That was
* measured too, and it needed to be**: every other dialog in this app
* is mounted in `index.html`, *outside* `.main-panel`, so "dialogs are
* fine" was not evidence about a dialog opened from inside a view. A
* probe dialog appended to `track-list`'s shadow root paints to y=439,
* over the mini player and the tab bar, with the contained ancestor
* still there.
*
* Four things about this component are load-bearing.
*
* **It is one element with two presentations, not two components.**
* The host keeps rendering exactly the panel it rendered before and
* slots it into whichever surface is up, so the twelve call sites
* changed one tag name each and nothing else — no second item model, no
* second keyboard model, and `ContextMenuController` still drives
* `.active` and `.anchor` as if it were talking to a `wa-popup`.
*
* **Which surface exists is `matchMedia`, not a media query.** The
* decision is whether a `<dialog>` is in the tree at all, which is
* `job-band` and `player-controls`' rule: a `display: none` surface is
* still in the shadow root and still something a positional or by-role
* query finds.
*
* **The sheet has to un-do the UA stylesheet to be full-bleed.**
* A native `<dialog>` carries `max-width: calc(100% - 6px - 2em)` and
* `margin: auto`, which on the device produced a 354px panel floating
* in the middle of a 424px screen. `max-width: none` and explicit
* margins are what make it a sheet rather than a small centred box.
* The *positioning* needs no such care: a top-layer dialog's containing
* block is the viewport even with a paint-contained ancestor, which is
* why `bottom: 0` reaches y=439 and not the main panel's 318.
*
* **Dismissal has to travel back.** `wa-dialog` closes itself on
* Escape, which would otherwise leave the controller's
* `contextMenuOpen` true and the menu unopenable until something else
* cleared it. `menu-dismiss` is that signal, and the controller listens
* for it on the document beside the click and contextmenu listeners it
* already has.
*/
import { LitElement, css, html } from 'lit';
import { customElement, property, query, state } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import { PHONE_QUERY } from '@utils/breakpoints';
import { nameDialogsIn } from '@utils/name-dialog';
/** The event a surface dispatches when it closed itself. */
export const MENU_DISMISS_EVENT = 'menu-dismiss';
/**
* The event a surface dispatches once it has finished showing.
*
* Only the sheet sends it, and only because `wa-dialog` moves focus to
* itself on the frame after `showModal()` -- see `MenuKeyboard.refocus`
* for why waiting longer is not the fix.
*/
export const MENU_SHOWN_EVENT = 'menu-shown';
/**
* A `wa-popup` anchor: a real element or a virtual one.
*
* `undefined` rather than `null` for "not set yet", because that is
* what `wa-popup`'s own property accepts — this surface hands the value
* straight through and must not widen it.
*/
type MenuAnchor = WaPopup['anchor'] | undefined;
/** A `wa-dialog`, as much of it as this file needs. */
type DialogEl = HTMLElement & { open: boolean };
@customElement('menu-surface')
export class MenuSurface extends LitElement {
/** Whether the menu is showing. Set by `ContextMenuController`. */
@property({ type: Boolean }) active = false;
/**
* Where the popup hangs from. Ignored in sheet mode, which is
* anchored to the bottom of the screen rather than to the touch
* point — that is the whole point of a sheet.
*/
@property({ attribute: false }) anchor: MenuAnchor = undefined;
/**
* `wa-popup`'s placement, defaulted because all twelve call sites
* passed the same one. Kept as a property so a future menu that
* wants another does not have to reach past this component.
*/
@property() placement = 'bottom-start';
/**
* What to call the sheet, for a surface whose content is not a
* `.context-menu-panel` with an `aria-label` of its own -- the
* playlist submenu, whose content is a `playlist-picker`.
*/
@property() label = '';
@state() private sheet = false;
@query('wa-popup') private popup?: WaPopup;
@query('wa-dialog') private dialog?: DialogEl;
private phoneQuery?: MediaQueryList;
static override styles = css`
:host {
display: contents;
}
wa-popup {
z-index: 200;
}
/* The sheet. A native dialog's UA stylesheet centres it and
caps its width, which on the device drew a 354px box in the
middle of a 424px screen — so all four of these are undoing
that rather than decorating. */
wa-dialog::part(dialog) {
margin: auto auto 0 auto;
max-width: none;
max-height: 85vh;
width: 100%;
border-radius: 12px 12px 0 0;
background: var(--yj-bg-elevated, #343a40);
padding: 0;
}
/* **A long menu scrolls; it does not hang off the bottom.**
Measured on the device at 80vh: seven 48px rows plus the grip
came to 364px against a 351px dialog, so the last row's
bottom was at y=452 on a 439px screen -- the one row a
destructive action is most likely to be. The cap has to stay
(a sheet covering the whole screen is a page, not a sheet),
so the body is what gives. */
wa-dialog::part(body) {
padding: 0;
overflow-y: auto;
}
/* A sheet is dragged at with a thumb, so it says where its top
edge is. Decorative: the panel below it carries the actions. */
.grip {
width: 36px;
height: 4px;
margin: 8px auto 4px;
border-radius: 2px;
background: var(--yj-text-tertiary, #888);
}
`;
override connectedCallback(): void {
super.connectedCallback();
// Looked up here rather than at module load, so a test can
// install its own matchMedia before the element is created.
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
this.sheet = this.phoneQuery?.matches ?? false;
this.phoneQuery?.addEventListener('change', this.onPhoneChange);
}
override disconnectedCallback(): void {
super.disconnectedCallback();
this.phoneQuery?.removeEventListener('change', this.onPhoneChange);
}
private onPhoneChange = (e: MediaQueryListEvent): void => {
this.sheet = e.matches;
};
/**
* Re-run the popup's positioning.
*
* Forwarded rather than dropped because `page-header` calls it when
* it opens the overflow menu: the popup is rendered before the
* button it anchors to has settled. A sheet has nothing to
* reposition -- it is anchored to the bottom of the screen -- so
* there it is deliberately a no-op rather than an error.
*/
reposition(): void {
this.popup?.reposition();
}
/**
* The panel the host slotted in. It is light DOM here and stays in
* the host's shadow root, which is what keeps the host's own
* `contextMenuStyles` applying to it in both presentations.
*/
private get panel(): HTMLElement | null {
return this.querySelector('.context-menu-panel');
}
override updated(): void {
const panel = this.panel;
// The sheet's rows are bigger, and that rule lives in the one
// stylesheet every call site already includes rather than in
// twelve places. The attribute is how it knows.
if (panel) panel.toggleAttribute('data-sheet', this.sheet);
if (this.sheet) {
this.syncSheet(panel);
return;
}
if (this.popup) {
if (this.anchor) this.popup.anchor = this.anchor;
this.popup.active = this.active;
}
}
private syncSheet(panel: HTMLElement | null): void {
const dialog = this.dialog;
if (!dialog) return;
// The dialog is named after the menu it contains, so no call
// site has to say the same thing twice: the panel already
// carries `role="menu"` and an `aria-label` naming what it acts
// on. `without-header` renders no heading, which is
// `name-dialog`'s documented `aria-label` path.
const label = panel?.getAttribute('aria-label') || this.label;
if (label) dialog.setAttribute('label', label);
nameDialogsIn(this.shadowRoot);
if (dialog.open !== this.active) dialog.open = this.active;
}
/**
* `wa-dialog` closed itself — Escape, or its own close button.
* The controller owns `contextMenuOpen`, so it has to hear about
* it or the menu is left open in state and shut on screen.
*/
private onDialogShown = (): void => {
if (!this.active) return;
this.dispatchEvent(
new CustomEvent(MENU_SHOWN_EVENT, {
bubbles: true,
composed: true,
}),
);
};
private onDialogHide = (): void => {
if (!this.active) return;
this.dispatchEvent(
new CustomEvent(MENU_DISMISS_EVENT, {
bubbles: true,
composed: true,
}),
);
};
override render() {
if (this.sheet) {
// **The anchor stays out of the sheet.** One call site --
// `page-header`'s overflow menu -- slots its own trigger
// button as the thing the popup hangs from, and a sheet
// hangs from the bottom of the screen instead. Rendering
// that slot outside the dialog is what keeps the button on
// the page rather than inside the surface it opens.
return html`
<slot name="anchor"></slot>
<wa-dialog
without-header
data-testid="menu-sheet"
@wa-after-show=${this.onDialogShown}
@wa-hide=${this.onDialogHide}
>
<div class="grip"></div>
<slot></slot>
</wa-dialog>
`;
}
return html`
<wa-popup placement=${this.placement} flip shift>
<slot name="anchor" slot="anchor"></slot>
<slot></slot>
</wa-popup>
`;
}
}
declare global {
interface HTMLElementTagNameMap {
'menu-surface': MenuSurface;
}
}
frontend/src/components/page-header/page-header.ts
+16 -9
View File
@@ -1,9 +1,9 @@
import { LitElement, html, css, nothing } from 'lit';
import { customElement, property, query, state } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import { designTokens } from '../../styles/tokens.css';
import {
@@ -183,8 +183,8 @@ export class PageHeader extends LitElement {
@query('#page-header-overflow')
private menuPanel?: HTMLElement;
@query('wa-popup')
private popup?: WaPopup;
@query('menu-surface')
private popup?: MenuSurface;
private menuKeyboard = new MenuKeyboard(() => this.closeMenu());
@@ -418,7 +418,7 @@ export class PageHeader extends LitElement {
outline-offset: -1px;
}
wa-popup {
menu-surface {
z-index: 200;
}
@@ -670,10 +670,17 @@ export class PageHeader extends LitElement {
return html`
<div class="actions">
${this.actions.map((a) => this.renderActionButton(a))}
<wa-popup
<!-- A sheet below 600px, like every other menu in the
app (#60). The clipping that issue is about does
not bite here — this one opens downward from the
top of a full-height view, so it has somewhere to
go even without top-layer promotion — but the touch
targets do: on a phone *every* action of a page
that overflows lives in here, at wa-dropdown-item
defaults. One surface, so there is no second
answer to what a menu looks like. -->
<menu-surface
placement="bottom-end"
flip
shift
.active=${this.menuOpen}
>
<button
@@ -711,7 +718,7 @@ export class PageHeader extends LitElement {
`,
)}
</div>
</wa-popup>
</menu-surface>
</div>
`;
}
frontend/src/components/playlist-details/playlist-details.ts
+12 -15
View File
@@ -7,7 +7,8 @@ import {
} from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import '@lit-labs/virtualizer';
import type { LitVirtualizer } from '@lit-labs/virtualizer';
@@ -35,7 +36,7 @@ import {
contextMenuStyles,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import { focusRovingRow, nextRovingIndex } from '@utils/roving-rows';
import { FavoritesController } from '@store/controllers/favorites-controller';
import { notificationStore } from '@store/notification-store';
@@ -145,10 +146,10 @@ export class PlaylistDetails
private dragImageEl: HTMLElement | null = null;
@query('#context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup!: WaPopup;
private playlistSubmenuPopup!: MenuSurface;
@query('track-details')
private trackDetailsDialog!: TrackDetails;
@@ -163,11 +164,11 @@ export class PlaylistDetails
// ContextMenuHost interface
// =================================================================
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup(): WaPopup | undefined {
getPlaylistSubmenuPopup(): MenuTarget | undefined {
return this.playlistSubmenuPopup;
}
@@ -1617,11 +1618,8 @@ export class PlaylistDetails
private renderContextMenu() {
return html`
<wa-popup
<menu-surface
id="context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu
.contextMenuOpen}
>
@@ -1783,13 +1781,12 @@ export class PlaylistDetails
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${this.ctxMenu
.playlistSubmenuOpen}
>
@@ -1816,7 +1813,7 @@ export class PlaylistDetails
</div>
`
: nothing}
</wa-popup>
</menu-surface>
`;
}
}
frontend/src/components/playlist-view/playlist-view.ts
+9 -9
View File
@@ -2,7 +2,9 @@ import { LitElement, html, css, nothing } from 'lit';
import { customElement, state, query } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import {
@@ -140,7 +142,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
private pendingDropPaths: string[] = [];
@query('#playlist-context-menu')
private playlistContextMenuPopup!: WaPopup;
private playlistContextMenuPopup!: MenuSurface;
@query('duplicate-tracks-dialog')
private duplicateDialog!: DuplicateTracksDialog;
@@ -1059,7 +1061,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
);
}
private closePlaylistContextMenu() {
private closePlaylistContextMenu = () => {
if (!this.playlistContextMenuOpen) return;
this.menuKeyboard.close();
@@ -1072,7 +1074,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
if (popup) {
popup.active = false;
}
}
};
private async onPlaylistContextAction(
action: string,
@@ -1500,13 +1502,11 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
</div>`
: this.renderPlaylistList()}
<wa-popup
<menu-surface
id="playlist-context-menu"
placement="bottom-start"
flip
shift
.active=${this
.playlistContextMenuOpen}
@menu-dismiss=${this.closePlaylistContextMenu}
>
${this.playlistContextMenuOpen
? html`
@@ -1564,7 +1564,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<duplicate-tracks-dialog
@playlist-action-complete=${() =>
frontend/src/components/queue-panel/queue-panel.ts
+19 -20
View File
@@ -9,7 +9,8 @@ import {
} from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import { QueueController } from '@store/controllers/queue-controller';
import { creditStore } from '@store/credit-store';
@@ -34,7 +35,7 @@ import {
contextMenuStyles,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import { focusRovingRow, nextRovingIndex } from '@utils/roving-rows';
import { FavoritesController } from '@store/controllers/favorites-controller';
import {
@@ -140,13 +141,13 @@ export class QueuePanel
private delegationAttached = false;
@query('#add-to-playlist-popup')
private addToPlaylistPopup!: WaPopup;
private addToPlaylistPopup!: MenuSurface;
@query('#context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup!: WaPopup;
private playlistSubmenuPopup!: MenuSurface;
/** Unsubscribes the credit-arrival repaint. */
private creditsUnsub?: () => void;
@@ -305,11 +306,11 @@ export class QueuePanel
// ContextMenuHost interface
// =================================================================
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup(): WaPopup | undefined {
getPlaylistSubmenuPopup(): MenuTarget | undefined {
return this.playlistSubmenuPopup;
}
@@ -1092,7 +1093,7 @@ export class QueuePanel
}
}
private closePlaylistPicker() {
private closePlaylistPicker = () => {
if (!this.playlistPickerOpen) return;
this.playlistPickerOpen = false;
@@ -1102,7 +1103,7 @@ export class QueuePanel
if (popup) {
popup.active = false;
}
}
};
private onPlaylistActionComplete = () => {
this.closePlaylistPicker();
@@ -2042,9 +2043,11 @@ export class QueuePanel
</div>
</div>
<wa-popup
<menu-surface
id="add-to-playlist-popup"
label="Add to playlist"
placement="bottom-end"
@menu-dismiss=${this.closePlaylistPicker}
.active=${this.playlistPickerOpen}
>
${this.playlistPickerOpen
@@ -2060,7 +2063,7 @@ export class QueuePanel
></playlist-picker>
`
: nothing}
</wa-popup>
</menu-surface>
<div
class="list-area"
@@ -2103,11 +2106,8 @@ export class QueuePanel
</div>
</div>
<wa-popup
<menu-surface
id="context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu.contextMenuOpen}
>
${this.ctxMenu.contextMenuOpen
@@ -2195,13 +2195,12 @@ export class QueuePanel
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${this.ctxMenu.playlistSubmenuOpen}
>
${this.ctxMenu.playlistSubmenuOpen &&
@@ -2224,7 +2223,7 @@ export class QueuePanel
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<track-details></track-details>
`;
frontend/src/components/smart-playlist-details/smart-playlist-details.ts
+12 -15
View File
@@ -26,7 +26,7 @@ import {
contextMenuStyles,
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import { focusRovingRow, nextRovingIndex } from '@utils/roving-rows';
import { FavoritesController } from '@store/controllers/favorites-controller';
import {
@@ -40,7 +40,8 @@ import {
} from '@utils/drag-image';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import '@lit-labs/virtualizer';
import type { LitVirtualizer } from '@lit-labs/virtualizer';
@@ -176,10 +177,10 @@ export class SmartPlaylistDetails
private dragImageEl: HTMLElement | null = null;
@query('#context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup!: WaPopup;
private playlistSubmenuPopup!: MenuSurface;
@query('track-details')
private trackDetailsDialog!: TrackDetails;
@@ -188,11 +189,11 @@ export class SmartPlaylistDetails
// ContextMenuHost interface
// =================================================================
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup(): WaPopup | undefined {
getPlaylistSubmenuPopup(): MenuTarget | undefined {
return this.playlistSubmenuPopup;
}
@@ -1465,11 +1466,8 @@ export class SmartPlaylistDetails
private renderContextMenu() {
return html`
<wa-popup
<menu-surface
id="context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu.contextMenuOpen}
>
${this.ctxMenu.contextMenuOpen
@@ -1583,13 +1581,12 @@ export class SmartPlaylistDetails
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${this.ctxMenu
.playlistSubmenuOpen}
>
@@ -1616,7 +1613,7 @@ export class SmartPlaylistDetails
</div>
`
: nothing}
</wa-popup>
</menu-surface>
`;
}
}
frontend/src/components/track-list/track-list.ts
+12 -15
View File
@@ -18,7 +18,7 @@ import {
isContextMenuKey,
} from '@utils/context-menu-controller.js';
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import { PlayerController } from '@store/controllers/player-controller';
import { SearchController } from '@store/controllers/search-controller';
import '@components/page-header/page-header';
@@ -63,7 +63,8 @@ import type {
} from '@lit-labs/virtualizer';
import { flow } from '@lit-labs/virtualizer/layouts/flow.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import { describeError } from '@utils/describe-error';
@@ -231,18 +232,18 @@ export class TrackList
private tracks: library.Track[] = [];
@query('#context-menu')
private contextMenuPopup!: WaPopup;
private contextMenuPopup!: MenuSurface;
@query('#playlist-submenu')
private playlistSubmenuPopup!: WaPopup;
private playlistSubmenuPopup!: MenuSurface;
// -- ContextMenuHost interface --
getContextMenuPopup(): WaPopup | undefined {
getContextMenuPopup(): MenuTarget | undefined {
return this.contextMenuPopup;
}
getPlaylistSubmenuPopup(): WaPopup | undefined {
getPlaylistSubmenuPopup(): MenuTarget | undefined {
return this.playlistSubmenuPopup;
}
@@ -2323,11 +2324,8 @@ export class TrackList
</div>
`}
<wa-popup
<menu-surface
id="context-menu"
placement="bottom-start"
flip
shift
.active=${this.ctxMenu.contextMenuOpen}
>
${this.ctxMenu.contextMenuOpen
@@ -2413,13 +2411,12 @@ export class TrackList
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<wa-popup
<menu-surface
id="playlist-submenu"
label="Add to playlist"
placement="right-start"
flip
shift
.active=${this.ctxMenu.playlistSubmenuOpen}
>
${this.ctxMenu.playlistSubmenuOpen && this.selection.hasSelection
@@ -2437,7 +2434,7 @@ export class TrackList
</div>
`
: nothing}
</wa-popup>
</menu-surface>
<track-details></track-details>
`;
frontend/src/utils/context-menu-controller.ts
+145 -6
View File
@@ -5,7 +5,20 @@ import type {
} from 'lit';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
/**
* What this controller needs of a surface: something it can switch on
* and point at. Both `wa-popup` and `menu-surface` satisfy it.
*/
export type MenuTarget = HTMLElement & {
active: boolean;
anchor?: WaPopup['anchor'];
};
import { registerViewAware } from './view-lifecycle';
import {
MENU_DISMISS_EVENT,
MENU_SHOWN_EVENT,
} from '../components/menu-surface/menu-surface';
/**
* Host interface for components using the ContextMenuController.
@@ -17,10 +30,18 @@ export interface ContextMenuHost
extends ReactiveControllerHost {
updateComplete: Promise<boolean>;
shadowRoot: ShadowRoot | null;
/** Return the main context-menu popup element. */
getContextMenuPopup(): WaPopup | undefined;
/** Return the playlist submenu popup element. */
getPlaylistSubmenuPopup(): WaPopup | undefined;
/**
* Return the main context-menu surface.
*
* `MenuSurface` since #60, which is a `wa-popup` above 600px and a
* bottom sheet below it. The type is the narrow shape this
* controller drives rather than either element, so a host that
* still renders a bare `wa-popup` — the playlist submenu does —
* satisfies it unchanged.
*/
getContextMenuPopup(): MenuTarget | undefined;
/** Return the playlist submenu surface. */
getPlaylistSubmenuPopup(): MenuTarget | undefined;
/**
* Called when the context menu is closed by an
* outside click/contextmenu/mousedown. Components
@@ -33,6 +54,15 @@ export interface ContextMenuHost
/** Submenu close delay in milliseconds. */
const SUBMENU_CLOSE_DELAY = 150;
/**
* How long to keep trying to put focus on a menu's first item.
*
* Long enough to outlast `wa-dialog`'s show animation, which ends by
* focusing the dialog; short enough that a menu which genuinely has no
* items stops rather than spinning for the life of the page.
*/
const FOCUS_RETRY_BUDGET_MS = 500;
/** A menu item, focusable and clickable. Web Awesome sets `role` itself. */
type MenuItem = HTMLElement & { active?: boolean; disabled?: boolean };
@@ -73,6 +103,30 @@ export class MenuKeyboard {
void this.focusFirstItem(panel);
}
/**
* Take focus back, for a surface that finished showing after we
* had already placed it.
*
* `wa-dialog` focuses `[autofocus]` or *itself* on the animation
* frame after `showModal()`, and it cannot see our first menu item
* to prefer it: the panel is slotted through `menu-surface`, so the
* dialog's own `querySelector` stops at the `<slot>`. Retrying on a
* longer budget does not fix this either -- the first attempt
* *succeeds*, and the steal happens afterwards. Measured on the
* device: the sheet opened with focus on the `<dialog>` and every
* arrow key went nowhere.
*
* So the surface says when it has settled and this re-asserts. It
* is a no-op for a menu that is closed or that already has focus.
*/
refocus(): void {
const panel = this.panel;
if (!panel || panel.contains(deepActiveElement())) return;
void this.focusFirstItem(panel);
}
/**
* Focus the first item, once the items are items.
*
@@ -91,11 +145,24 @@ export class MenuKeyboard {
await Promise.all(candidates.map((el) => el.updateComplete ?? null));
// …and once the popup has positioned itself. `wa-popup` places the
// …and once the surface has shown itself. `wa-popup` places the
// panel on an animation frame, and `focus()` on a not-yet-shown
// element is a silent no-op — which looks identical to a menu
// that opened and refused to take focus.
for (let attempt = 0; attempt < 3; attempt++) {
//
// **The budget is time, not frames, because #60 gave this a
// second kind of surface.** Three frames was enough for a
// popup; a `wa-dialog` runs a show *animation* and moves focus
// to the dialog itself when it finishes, which lands after
// those frames and takes the focus back. Measured on the
// device: the sheet opened with `document.activeElement` on the
// `<dialog>`, so every arrow key went nowhere. Retrying to a
// deadline is `roving-grid`'s rule for the same reason — the
// thing being waited for is another component's animation, not
// a fixed number of paints.
const deadline = Date.now() + FOCUS_RETRY_BUDGET_MS;
while (Date.now() < deadline) {
// Bail if the menu closed while we waited.
if (this.panel !== panel) return;
@@ -259,6 +326,20 @@ export class ContextMenuController
/** Bound close handler for document events. */
private closeHandler = () => this.close();
/**
* A surface finished showing; see `MenuKeyboard.refocus`.
*
* **Not while the submenu is up.** Both surfaces send this, and the
* submenu's sheet opens *over* the main one -- so re-asserting
* focus on the main panel's first item would snatch it straight
* back out of the playlist picker the user just opened.
*/
private shownHandler = () => {
if (this.contextMenuOpen && !this.playlistSubmenuOpen) {
this.keyboard.refocus();
}
};
/** Bound mousedown handler for outside-click detection. */
private mousedownCloseHandler = (
e: MouseEvent,
@@ -325,12 +406,28 @@ export class ContextMenuController
'mousedown',
this.mousedownCloseHandler,
);
document.addEventListener(
MENU_DISMISS_EVENT,
this.closeHandler,
);
document.addEventListener(
MENU_SHOWN_EVENT,
this.shownHandler,
);
}
private detach(): void {
if (!this.listening) return;
this.listening = false;
document.removeEventListener(
MENU_DISMISS_EVENT,
this.closeHandler,
);
document.removeEventListener(
MENU_SHOWN_EVENT,
this.shownHandler,
);
document.removeEventListener(
'click',
this.closeHandler,
@@ -550,6 +647,48 @@ export const contextMenuStyles = css`
z-index: 200;
}
/* ---------------------------------------------------------------
The sheet (#60).
menu-surface puts data-sheet on the panel when it is drawn
as a bottom sheet, and these rules are here rather than in that
component because the panel is the *host's* light DOM: it lives
in the host's shadow root, so only the host's stylesheet can
reach it. This file is the one every call site already includes,
which is what makes twelve menus grow thumb-sized rows from one
edit.
Measured on the device before the change: rows were 29px, against
the 44px floor plan 018 promises and the 48px this issue asks
for. --------------------------------------------------------- */
.context-menu-panel[data-sheet] {
border: none;
border-radius: 0;
box-shadow: none;
min-width: 0;
padding: 4px 0 8px;
background-color: transparent;
}
.context-menu-panel[data-sheet] wa-dropdown-item {
font-size: var(--yj-text-md, 0.9375rem);
min-height: 48px;
align-items: center;
}
.context-menu-panel[data-sheet] wa-dropdown-item::part(base) {
min-height: 48px;
align-items: center;
}
/* A submenu arrow means "a flyout opens to the right", which is not
what happens on a phone and is not a thing a thumb can aim at.
The row still works — it is the tap handler that opens the
playlist picker — so what goes is the arrow, not the item. */
.context-menu-panel[data-sheet] .submenu-arrow {
display: none;
}
.context-menu-panel {
background-color: var(
--yj-bg-elevated,
frontend/test/components/menu-surface.test.ts
+222
View File
@@ -0,0 +1,222 @@
/**
* Where a context menu is drawn (#60).
*
* **This file asserts the mechanism, not the symptom, and that is the
* whole point of it.** The defect is that on the reference device's
* Chrome 113 a `wa-popup` has no Popover API to promote it to the top
* layer, so it falls back to `position: fixed` and is then *clipped* by
* `.main-panel`'s `contain: paint`. No tier here can reproduce that:
* this runner's Chromium and CI's WebKit both have the Popover API, so
* the popup is top-layered and looks perfectly correct. A test that
* asserted "the menu is not clipped" would pass on the broken build.
*
* What is checkable everywhere is *which surface exists*. A native
* `<dialog>` uses the real top layer, which Chrome 37 has, so "it is a
* dialog at phone width" is the property that makes the device
* behaviour follow. The measurements that needed the hardware are on
* the PR and in `.planning/NOTES.md`.
*/
import { describe, expect, it, beforeEach, afterEach } from 'vitest';
import '@components/menu-surface/menu-surface';
import { MENU_DISMISS_EVENT } from '@components/menu-surface/menu-surface';
import { fixture } from '@test/support/render';
/** Every source file, as text. */
const SOURCES = import.meta.glob<string>('../../src/**/*.ts', {
eager: true,
query: '?raw',
import: 'default',
});
/**
* The two files allowed to render a raw `wa-popup`.
*
* `menu-surface` *is* the popup, in its desktop presentation.
* `job-indicator` is the documented exception and the contrast that
* proved the diagnosis: it lives in `.top-bar`, no ancestor of which
* has containment, so even the fixed fallback lands correctly on the
* device -- measured on #62, unclipped at every width.
*
* `now-playing`'s cover preview is the third, and it is a different
* reason: it is not a menu. It opens on `mouseenter` over the album
* art, so a touch device never sees it at all, and a bottom sheet for
* a hover preview would be absurd. It is also in the bottom bar rather
* than the main panel, so nothing clips it either.
*
* **Both were found by this sweep, not by the conversion**, which is
* the argument for having it: twelve call sites were converted by hand
* and two more existed.
*/
const MAY_USE_POPUP = [
'menu-surface/menu-surface.ts',
'jobs/job-indicator.ts',
'now-playing/now-playing.ts',
];
/**
* Answer `matchMedia` for the phone query, on `transport-context`'s
* pattern: what is under test is the component's reaction to the
* answer, not whether this runner's window can get below 600px.
*/
const realMatchMedia = window.matchMedia;
function pretendPhone(phone: boolean): void {
window.matchMedia = ((query: string) => ({
matches: phone && query.includes('599'),
media: query,
addEventListener: () => {},
removeEventListener: () => {},
})) as unknown as typeof window.matchMedia;
}
/** A surface with the panel a real call site slots into it. */
async function surfaceWithPanel(): Promise<HTMLElement> {
const el = await fixture('menu-surface');
el.innerHTML =
'<div class="context-menu-panel" role="menu" aria-label="Track actions">' +
'<wa-dropdown-item>Play</wa-dropdown-item>' +
'</div>';
const surface = el as unknown as HTMLElement & {
active: boolean;
updateComplete: Promise<unknown>;
};
surface.active = true;
await surface.updateComplete;
return el;
}
/**
* The sweep, in the spirit of `icon-language.test.ts` and
* `TestNoDirectRuntimeEmits`: the rule is about *every* call site, and
* checking one checks nothing.
*
* Twelve menus were converted by hand. A thirteenth written as a bare
* `<wa-popup>` would work perfectly in every tier here and be clipped
* on the device, which is exactly the failure this whole change is
* about and exactly the one no runtime assertion can see.
*/
describe('every menu goes through the one surface', () => {
it('reads the sources at all', () => {
// A sweep over an empty glob passes, so this is asserted first.
expect(Object.keys(SOURCES).length).toBeGreaterThan(100);
});
it('leaves no raw wa-popup outside the two files allowed one', () => {
const offenders = Object.entries(SOURCES)
.filter(([path]) => !MAY_USE_POPUP.some((ok) => path.endsWith(ok)))
.filter(([, src]) => src.includes('<wa-popup'))
.map(([path]) => path.replace(/^.*\/src\//, 'src/'));
expect(
offenders,
'these render a popup directly; use <menu-surface> so the phone gets a sheet',
).toEqual([]);
});
});
describe('menu-surface', () => {
afterEach(() => {
window.matchMedia = realMatchMedia;
});
describe('above the phone breakpoint', () => {
beforeEach(() => pretendPhone(false));
it('draws a popup, which is what the desktop has always had', async () => {
const el = await surfaceWithPanel();
expect(el.shadowRoot?.querySelector('wa-popup')).not.toBeNull();
expect(el.shadowRoot?.querySelector('wa-dialog')).toBeNull();
});
it('does not mark the panel as a sheet', async () => {
const el = await surfaceWithPanel();
expect(
el.querySelector('.context-menu-panel')?.hasAttribute('data-sheet'),
).toBe(false);
});
});
describe('at phone width', () => {
beforeEach(() => pretendPhone(true));
/**
* The load-bearing one. `wa-dialog` renders a *native* `<dialog>`,
* and it is the native element -- not the wrapper -- that gets the
* top layer and therefore escapes the paint containment that clips
* the popup on the device.
*/
it('draws a native dialog, which is what escapes the clip', async () => {
const el = await surfaceWithPanel();
const wrapper = el.shadowRoot?.querySelector('wa-dialog');
expect(wrapper, 'no wa-dialog at phone width').not.toBeNull();
expect(el.shadowRoot?.querySelector('wa-popup')).toBeNull();
await (wrapper as HTMLElement & { updateComplete: Promise<unknown> })
.updateComplete;
expect(
wrapper?.shadowRoot?.querySelector('dialog'),
'the wrapper is not backed by a native dialog',
).not.toBeNull();
});
/**
* The rows are sized by `contextMenuStyles`, which lives in the
* *host's* shadow root — so the only thing this component can do is
* say which mode it is in. That attribute is the contract between
* the two, and it is what twelve call sites get their thumb-sized
* rows from.
*/
it('marks the panel as a sheet, which is what sizes the rows', async () => {
const el = await surfaceWithPanel();
expect(
el.querySelector('.context-menu-panel')?.hasAttribute('data-sheet'),
).toBe(true);
});
/**
* `wa-dialog` closes itself on Escape. Without this the controller
* would still believe the menu was open, and the *next* long-press
* would do nothing — which is the failure mode that looks like the
* gesture breaking rather than the dialog.
*/
it('reports a dismissal it did not initiate', async () => {
const el = await surfaceWithPanel();
let dismissed = 0;
document.addEventListener(MENU_DISMISS_EVENT, () => {
dismissed += 1;
});
el.shadowRoot
?.querySelector('wa-dialog')
?.dispatchEvent(new CustomEvent('wa-hide', { bubbles: false }));
expect(dismissed, 'no menu-dismiss reached the document').toBe(1);
});
/**
* A dialog with no accessible name is what `utils/name-dialog.ts`
* exists for; here the name is already written on the panel, so no
* call site says it twice.
*/
it('names the sheet after the menu it contains', async () => {
const el = await surfaceWithPanel();
const wrapper = el.shadowRoot?.querySelector('wa-dialog');
expect(wrapper?.getAttribute('label')).toBe('Track actions');
});
});
});