From 9aaa8beb99f7cad362bc17ff57a209aee00143ba Mon Sep 17 00:00:00 2001 From: Logan Date: Fri, 21 Aug 2026 02:23:33 -0400 Subject: [PATCH] feat(shell): draw a context menu where it fits, not where it is anchored On the reference device every context menu in the app is clipped, and the two halves of that are structural rather than incidental. 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. Measured at 424x439 before any of this: the main panel spans 0-318, the open menu spanned 191-401, and three of its seven items were cut off with no way to reach them. Rows were 29px against a 44px floor. menu-surface is one element with two presentations -- a wa-popup above 600px, a wa-dialog bottom sheet below it -- so the host keeps rendering the panel it always rendered and ContextMenuController keeps driving .active and .anchor as though it were talking to a popup. showModal() is Chrome 37 and uses the real top layer, so the sheet is immune by construction rather than by styling. Four things needed measuring on the hardware rather than reading. "A dialog escapes containment" was the premise and was untested here: every other dialog in this app is mounted in index.html, outside .main-panel. A probe dialog appended to track-list's shadow root paints to y=439, over the mini player and the tab bar. A native dialog's UA stylesheet centres it and caps its width, which drew a 354px panel in the middle of a 424px screen -- so four declarations in this component are pure undoing. wa-dialog focuses [autofocus] or itself on the frame after showModal(), and it cannot see our first menu item to prefer it: the panel is slotted, so its own querySelector stops at the . A longer retry budget does not fix that, because the first attempt succeeds and is then overwritten -- hence menu-shown and MenuKeyboard.refocus(). The budget became time-based anyway, since what is being waited for is another component's animation. And a dismissal has to travel back: wa-dialog closes itself on Escape, which would leave the controller believing the menu is open. The failure mode there is not a stuck sheet but the *next* long-press doing nothing, which reads as the gesture breaking. --- .../components/menu-surface/menu-surface.ts | 318 ++++++++++++++++++ frontend/src/utils/context-menu-controller.ts | 151 ++++++++- 2 files changed, 463 insertions(+), 6 deletions(-) create mode 100644 frontend/src/components/menu-surface/menu-surface.ts diff --git a/frontend/src/components/menu-surface/menu-surface.ts b/frontend/src/components/menu-surface/menu-surface.ts new file mode 100644 index 0000000..94652cd --- /dev/null +++ b/frontend/src/components/menu-surface/menu-surface.ts @@ -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 + * `` 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 `` 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 `` 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 `` 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` + + +
+ + + `; + } + + return html` + + + + + `; + } +} + +declare global { + interface HTMLElementTagNameMap { + 'menu-surface': MenuSurface; + } +} diff --git a/frontend/src/utils/context-menu-controller.ts b/frontend/src/utils/context-menu-controller.ts index 824caa4..53d51c5 100644 --- a/frontend/src/utils/context-menu-controller.ts +++ b/frontend/src/utils/context-menu-controller.ts @@ -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; 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 ``. 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 `` 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 + // ``, 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,