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,