diff --git a/.planning/plans/active/018-supported-sizes-and-the-queue-model.md b/.planning/plans/completed/018-supported-sizes-and-the-queue-model.md similarity index 89% rename from .planning/plans/active/018-supported-sizes-and-the-queue-model.md rename to .planning/plans/completed/018-supported-sizes-and-the-queue-model.md index 7a134ef..ed69908 100644 --- a/.planning/plans/active/018-supported-sizes-and-the-queue-model.md +++ b/.planning/plans/completed/018-supported-sizes-and-the-queue-model.md @@ -3,7 +3,8 @@ **Issue:** #24 (`Area/Shell-Nav`, `Priority/High`, `Reviewed/Confirmed`) **Unblocks:** #55 (queue as a screen) — a real Gitea dependency **Relates:** #69 (page-header overflow), #12 (mini-player), #51 (small-screen umbrella) -**Status:** in flight +**Status:** complete — #24 shipped as PR #132, and the matrix's last +unkept promise closed with #69. #73 puts this first in Phase 2 and hangs the rest of the phase off it, so the decision has to be written down and arguable before any CSS @@ -269,6 +270,38 @@ leaving the navigation live means the scrim reads as "this is over the content" (which is what #24 asked for) without pretending the rest of the app is unavailable. +## What #69 did with the promise, and one thing this plan got wrong + +#69 landed on its own branch as decision 3 said it would, and the +matrix's *no action is ever unreachable at any supported size* is now +kept rather than promised. Measured on Playlists, actions clipped: + +| viewport | before #24 | after #24 | after #69 | +|---|---|---|---| +| 900×600, queue open | all three | one (114/162px) | none | +| 900×600, queue closed | one | one | none | +| 800×600, queue closed | one (158/162px) | one | none | +| 390×780 | all three | all three | none | +| 320×600 | all three | all three | none | + +The shape was the one decision 3 predicted — an actions API first, an +overflow rule second — and all three hosts that slot actions migrated. + +**What this document got wrong is smaller and worth keeping.** Decision +1 says the header's minimum is a *comfort* floor and that only the +queue and the actions compete for the header's width. They are not the +only two: every child of that flex row was `flex-shrink: 0`, so +whatever came last lost, and the actions come last. At 320px the sort +control alone is 172px of the header — so with every action already +collapsed into the menu, the *menu button* was 76px off the right edge. +The promise was still broken with nothing left to collapse. + +That is why #69 also had to decide what gives way: the title (which the +navigation also states) and, below 600px, the word "Sort:" (which the +direction arrow implies). Neither is an action, which is the rule the +matrix actually encodes — **an action is a capability and everything +else on that row is a label.** + ## Verification, and what each tier cannot see - `make ui-test` — the queue panel's mode logic is component-tier diff --git a/CLAUDE.md b/CLAUDE.md index 0f75b48..be86733 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1980,6 +1980,79 @@ that corrects itself a moment later is worse than saying nothing. And the field, the direction and their persistence, so the control cannot disagree with the list. +**And an action is data, on that same rule: the header decides what +fits, the host decides what happens.** Playlists slotted three buttons +totalling 390px into a header that gets 700px at 900×600, so "New Smart +Playlist" rendered **114 of its 162px** with the queue closed — and on a +phone none of them could be reached at all, which is what #69 reported. +A host passes `PageAction[]` (`{id, label, icon, onSelect, priority, +drop?}`) and `page-header` renders each one as a button or as an item in +one "More actions" menu. + +**It could not have been a rule added in one place**, and that is a fact +about the API rather than an effort estimate: actions used to arrive +through `` as arbitrary light-DOM markup, and a +component cannot move another component's light-DOM children into a +dropdown and keep their behaviour — there is nothing generic in markup +to render as a menu item. The slot survives for markup a data list +cannot express, at the stated cost that **a slotted action does not +collapse** and must therefore fit at 800×600. + +Six things about it are load-bearing: + +- **The fit is measured, never breakpointed.** A ResizeObserver drives + it, and each pass starts from *all visible* and hides the + lowest-priority action until it fits — so the collapsed set is a pure + function of the current width rather than of how the window got + there. A rule that only ever added to the set would never give a + button back, and one that adjusted by a step would need a hysteresis + band to stop it oscillating on the pixel where a button exactly fits. +- **"Fits" means nothing is clipped, which is not the same as the + header not overflowing.** The title can ellipsis, and the moment it + can it absorbs the pressure: `scrollWidth` reports a header that fits + perfectly while the heading reads "Playlis…". That is this bug moved + from the button to the title, invisible to the same measurement that + missed it the first time — so the heading's own truncation counts as + not fitting, and an action is collapsed before the title gives way. + Below that, at 320px, the title *is* what yields: the navigation also + says which page you are on, and an action has nowhere else to be said. +- **The measurement flips `hidden` on the rendered nodes rather than + re-rendering between steps.** Reading `scrollWidth` forces layout, + which is the point; awaiting a Lit update between steps instead lets + the intermediate all-visible state paint, so the fix would flash the + overflow it exists to prevent. +- **Priority is what a *capability* costs, not what a button is worth.** + New Playlist is highest because it is the **drop target** and a closed + menu cannot be one; that is also why `PageAction.drop` carries the + host's own `dragover`/`dragleave`/`drop` handlers rather than the + header owning a notion of dropping, and why the affordance is simply + absent from the overflow rather than approximated there. +- **`aria-controls` names a panel that is always in the DOM** — + `config-section`'s rule, and `wa-popup` hides it when inactive — and + the keyboard model is `MenuKeyboard`, shared with every other menu in + the app so this is not a second one. +- **It is checked per button, because `layout-overflow.spec.ts` cannot + see this.** That spec asserts the *shell* needs no sideways + scrolling and passed on the broken build; clipping *inside* a + component is invisible to it, which is exactly why the defect + survived a spec named for it. + `e2e/specs/header-action-overflow.spec.ts` measures each button + against its header at 900×600, 800×600, 390×780 and 320×600, and + asserts buttons **plus** menu account for every declared action — + without that half it would pass vacuously on a build that renders no + actions at all. + +One thing it deliberately does **not** grow is a phone mode for the +actions. `PHONE_COLUMN_IDS` is the precedent for "what is drawn and +what can be sorted are different questions", but it exists because the +track list's columns cannot be derived from a width; these can, and a +second declaration of what a phone shows is a second thing to keep in +step. What the header *does* state at phone width is one word: below +600px the sort control's "Sort:" label is visually hidden — 172px of a +320px header for a label the adjacent direction arrow implies — and it +stays in the accessibility tree, because it is the select's accessible +name and hiding it outright is `config-field`'s bug one component over. + **The header search box is view-scoped, and now says so.** It sits in the app header and reads as global; typing `tide` on Playlists answered "No playlists match your search" with three *Tideline* tracks in the diff --git a/e2e/specs/header-action-overflow.spec.ts b/e2e/specs/header-action-overflow.spec.ts new file mode 100644 index 0000000..d37b823 --- /dev/null +++ b/e2e/specs/header-action-overflow.spec.ts @@ -0,0 +1,318 @@ +import { test, expect } from '../support/fixtures.js'; + +/** + * #69: the Playlists header's buttons could not be reached. + * + * Three text buttons — Import (91px), New Playlist (122px), New Smart + * Playlist (162px), 390px in total — inside a header that gets 700px at + * 900×600. "New Smart Playlist" rendered **114 of its 162px**, and at + * phone width the Android report was the plain version of it: you + * cannot scroll to reach them, and scrolling is not how page controls + * should be exposed anyway. + * + * **`layout-overflow.spec.ts` passes on the broken build**, which is why + * this file exists rather than a case being added there. That spec + * asserts the *shell* needs no sideways scrolling; clipping *inside* a + * component is invisible to it. So the measurement here is per-button + * and per-header, against the widths the app promises. + * + * Plan 018's size matrix is the promise being kept: **no action is ever + * unreachable at any supported size.** These are its three bands. + */ + +const VIEWPORTS = [ + // Desktop's worst case, and not the enforced minimum: the sidebar + // collapses to icons *below* 900, so the content area is 843px at 899 + // and 700px at 900. Testing "the minimum" and stopping misses it. + { name: '900×600 (widest sidebar, narrowest content)', width: 900, height: 600 }, + { name: '800×600 (the enforced minimum)', width: 800, height: 600 }, + { name: '390×780 (phone)', width: 390, height: 780 }, + // WCAG 1.4.10's reflow target, which plan 018 promises the app fits. + { name: '320×600 (400% zoom)', width: 320, height: 600 }, +]; + +/** Every action the Playlists header can offer, in declared order. */ +const ACTIONS = ['Import', 'New Playlist', 'New Smart Playlist']; + +/** + * What the header is actually rendering, measured rather than inferred. + * + * A shadow query is the wrong tool for *asserting* — that is what + * `getByRole` below is for — but it is the right one for a measurement, + * because the number this issue is about (a button 48px wider than the + * box holding it) is not in the accessibility tree at all. + */ +const headerFit = (page: import('@playwright/test').Page) => + page.evaluate(() => { + const root = document + .querySelector('[data-testid="main-content"] playlist-view') + ?.shadowRoot?.querySelector('page-header')?.shadowRoot; + + if (!root) return null; + + const header = root.querySelector('.page-header')!; + const box = header.getBoundingClientRect(); + const title = root.querySelector('h1')!; + + const clipped = [ + ...root.querySelectorAll('.action, .more-button'), + ] + .filter((b) => !b.hidden) + .filter((b) => { + const r = b.getBoundingClientRect(); + + return r.right > box.right + 1 || r.left < box.left - 1; + }) + .map((b) => b.dataset['actionId'] ?? 'more'); + + return { + overflow: header.scrollWidth - header.clientWidth, + clipped, + titleTruncated: title.scrollWidth > title.clientWidth + 1, + buttons: [...root.querySelectorAll('.action')] + .filter((b) => !b.hidden) + .map((b) => b.textContent?.trim() ?? ''), + menu: [ + ...root.querySelectorAll('#page-header-overflow wa-dropdown-item'), + ].map((i) => i.textContent?.trim() ?? ''), + }; + }); + +test.describe('the page header never clips an action', () => { + test.beforeEach(async ({ app }) => { + await app.getByTestId('nav-playlists').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'playlists', + ); + }); + + test.afterEach(async ({ app }) => { + await app.setViewportSize({ width: 1280, height: 800 }); + }); + + for (const vp of VIEWPORTS) { + test(`every action is reachable at ${vp.name}`, async ({ app }) => { + await app.setViewportSize({ width: vp.width, height: vp.height }); + + // Polled: the fit is decided by a ResizeObserver, so it settles a + // frame after the resize rather than with it. + await expect + .poll(async () => (await headerFit(app))?.clipped) + .toEqual([]); + + const fit = (await headerFit(app))!; + + expect(fit.overflow).toBeLessThanOrEqual(0); + + // Between them, buttons and menu account for all three. This is + // the assertion the issue asks for: not "it fits" but "nothing + // was dropped to make it fit". + expect([...fit.buttons, ...fit.menu].sort()).toEqual([...ACTIONS].sort()); + }); + } + + /** + * The title gives way before an action does. + * + * Once the heading can ellipsis it absorbs the pressure, and + * `scrollWidth` then reports a header that fits perfectly while the + * heading reads "Playlis…" — this issue's own failure mode moved from + * the button to the title, and invisible to exactly the measurement + * that missed it the first time. At the desktop sizes there is always + * an action to collapse instead. + */ + test('does not truncate the heading to keep a button', async ({ app }) => { + for (const vp of VIEWPORTS.slice(0, 2)) { + await app.setViewportSize({ width: vp.width, height: vp.height }); + + await expect + .poll(async () => (await headerFit(app))?.titleTruncated) + .toBe(false); + } + }); + + /** + * Asserted through the accessibility tree, never a shadow query. An + * overflow menu is exactly the shape that grows a nameless control, + * and this repo has shipped one four times — most recently the + * queue's own close button. + */ + test('the overflow is a named control that opens a named menu', async ({ + app, + }) => { + await app.setViewportSize({ width: 900, height: 600 }); + + const more = app.getByRole('button', { name: 'More actions' }); + + await expect(more).toBeVisible(); + await expect(more).toHaveAttribute('aria-expanded', 'false'); + + await more.click(); + + await expect(more).toHaveAttribute('aria-expanded', 'true'); + + const menu = app.getByRole('menu', { name: 'More actions' }); + + await expect(menu).toBeVisible(); + + // Collapsed at 900×600: Import (lowest priority) and New Smart + // Playlist. New Playlist stays a button because it is the drop + // target, and a closed menu cannot be one. + await expect( + menu.getByRole('menuitem', { name: 'Import' }), + ).toBeVisible(); + await expect( + app.getByRole('button', { name: 'New Playlist', exact: true }), + ).toBeVisible(); + }); + + /** + * The phone case is the original report. Every action is in the menu + * at 390px, and the menu is reachable by name — which is the whole of + * "these need to be reachable in a sensible way". + */ + test('offers every action from the menu on a phone', async ({ app }) => { + await app.setViewportSize({ width: 390, height: 780 }); + + const more = app.getByRole('button', { name: 'More actions' }); + + await expect(more).toBeVisible(); + await more.click(); + + const menu = app.getByRole('menu', { name: 'More actions' }); + + for (const label of ACTIONS) { + await expect(menu.getByRole('menuitem', { name: label })).toBeVisible(); + } + }); + + /** + * Escape closes it and focus goes back to the trigger — `MenuKeyboard` + * is shared with every other menu in the app precisely so this is not + * a second keyboard model, and this is what proves it was wired up + * rather than merely imported. + */ + test('takes the keyboard, and gives it back', async ({ app }) => { + await app.setViewportSize({ width: 900, height: 600 }); + + const more = app.getByRole('button', { name: 'More actions' }); + + await more.click(); + + const menu = app.getByRole('menu', { name: 'More actions' }); + + await expect(menu).toBeVisible(); + + // The first item takes focus on open. `wa-dropdown-item` sets its + // own role in its own first update, so this is polled rather than + // read: a query at the host's updateComplete finds nothing, which + // reads exactly like a menu that refused to take focus. + await expect + .poll(async () => + app.evaluate(() => { + // Stops where `MenuKeyboard`'s own `deepActiveElement` stops: + // on the *host* whose shadow root has no active element. + // Descending unconditionally lands inside the focused + // `wa-dropdown-item`'s own shadow root, where nothing is + // focused — which reads exactly like a menu that refused the + // keyboard, on a build where it did not. + let el = document.activeElement; + + while (el?.shadowRoot?.activeElement) el = el.shadowRoot.activeElement; + + return el?.textContent?.trim() ?? null; + }), + ) + .toBe('Import'); + + await app.keyboard.press('Escape'); + + await expect(more).toHaveAttribute('aria-expanded', 'false'); + await expect(more).toBeFocused(); + }); + + /** + * New Playlist is a drop target, and declaring it as data must not + * take that away — which is why a `PageAction` carries the drop + * handlers rather than the header owning a notion of dropping. + * + * Nothing covered this before, in either tier, and it is the one + * behaviour the migration could plausibly have destroyed silently: + * dragging still *looks* fine against a button that no longer + * accepts anything. + */ + test('New Playlist still accepts a dropped track', async ({ app }) => { + await app.setViewportSize({ width: 1280, height: 800 }); + + const button = app.getByRole('button', { + name: 'New Playlist', + exact: true, + }); + + await expect(button).toBeVisible(); + + const result = await app.evaluate(async () => { + const view = document.querySelector( + '[data-testid="main-content"] playlist-view', + )!; + const target = view.shadowRoot! + .querySelector('page-header')! + .shadowRoot!.querySelector('[data-testid="page-action-new-playlist"]')!; + + const data = new DataTransfer(); + + data.setData( + 'application/x-yj-tracks', + JSON.stringify({ filePaths: ['/tmp/dropped.mp3'] }), + ); + + const fire = (type: string) => + target.dispatchEvent( + new DragEvent(type, { + bubbles: true, + cancelable: true, + dataTransfer: data, + }), + ); + + fire('dragover'); + await new Promise((r) => setTimeout(r, 50)); + + // The affordance is the host's state reaching the header's + // button, which is the half a plain handler call would not prove. + const highlighted = target.classList.contains('drag-over'); + + fire('drop'); + await new Promise((r) => setTimeout(r, 200)); + + return { + highlighted, + opened: view.shadowRoot!.querySelector('.create-form') !== null, + }; + }); + + expect(result).toEqual({ highlighted: true, opened: true }); + + // Leave the view as it was found. + await app.keyboard.press('Escape'); + }); + + /** + * An action given back when the window widens again. The collapsed + * set is a function of the current width and not of how it got there + * — a rule that only ever *added* to it would never widen. + */ + test('gives the buttons back when the window grows', async ({ app }) => { + await app.setViewportSize({ width: 390, height: 780 }); + + await expect.poll(async () => (await headerFit(app))?.buttons).toEqual([]); + + await app.setViewportSize({ width: 1440, height: 900 }); + + await expect + .poll(async () => (await headerFit(app))?.buttons) + .toEqual(ACTIONS); + await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]); + }); +}); diff --git a/frontend/src/assets/icons/fa/solid/ellipsis.svg b/frontend/src/assets/icons/fa/solid/ellipsis.svg new file mode 100644 index 0000000..01d5813 --- /dev/null +++ b/frontend/src/assets/icons/fa/solid/ellipsis.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/frontend/src/components/downloads-view/downloads-view.ts b/frontend/src/components/downloads-view/downloads-view.ts index 43441eb..626e9a0 100644 --- a/frontend/src/components/downloads-view/downloads-view.ts +++ b/frontend/src/components/downloads-view/downloads-view.ts @@ -3,6 +3,7 @@ import { customElement, state } from 'lit/decorators.js'; import '@awesome.me/webawesome/dist/components/icon/icon.js'; import '@awesome.me/webawesome/dist/components/button/button.js'; import '@components/page-header/page-header'; +import type { PageAction } from '@components/page-header/page-header'; import { designTokens } from '../../styles/tokens.css'; import { downloadStore, stateLabel } from '@store/download-store'; import type { Request, RequestSummary, DownloadView as DownloadRecord } from '@store/download-store'; @@ -246,23 +247,23 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) { override render() { return html` - - ${this.tab === 'requests' - ? html` - void this.checkNow()} - > - - ${this.checking ? 'Searching…' : 'Check now'} - - ` - : nothing} - + void this.checkNow(), + }, + ] satisfies PageAction[]) + : []} + >

Music you have requested, and the download attempts that diff --git a/frontend/src/components/home-view/home-view.ts b/frontend/src/components/home-view/home-view.ts index 4d8ca74..73b9c53 100644 --- a/frontend/src/components/home-view/home-view.ts +++ b/frontend/src/components/home-view/home-view.ts @@ -1,8 +1,9 @@ import { LitElement, html, css, nothing } from 'lit'; import { customElement, state } from 'lit/decorators.js'; import '@awesome.me/webawesome/dist/components/icon/icon.js'; -import '@awesome.me/webawesome/dist/components/button/button.js'; import { GetShelves } from '@go/home/service.js'; +import { ICON_SHUFFLE } from '@utils/icon-language'; +import type { PageAction } from '@components/page-header/page-header'; import { GetAlbumTracks } from '@go/library/library.js'; import type * as home from '@go/home/models.js'; import type * as library from '@go/library/models.js'; @@ -283,23 +284,24 @@ export class HomeView extends ViewLifecycleMixin(LitElement) { override render() { return html` - - - void this.load()} - > - - Shuffle suggestions - - + void this.load(), + }, + ] satisfies PageAction[]} + >

Somewhere to start listening.

${this.renderBody()} `; diff --git a/frontend/src/components/page-header/page-header.ts b/frontend/src/components/page-header/page-header.ts index 06eda0e..58a0da0 100644 --- a/frontend/src/components/page-header/page-header.ts +++ b/frontend/src/components/page-header/page-header.ts @@ -1,8 +1,16 @@ import { LitElement, html, css, nothing } from 'lit'; -import { customElement, property } from 'lit/decorators.js'; +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 { designTokens } from '../../styles/tokens.css'; +import { + MenuKeyboard, + contextMenuStyles, +} from '../../utils/context-menu-controller'; +import { ICON_MORE_ACTIONS } from '../../utils/icon-language'; /** * The one arrangement every primary view uses to say what it is. @@ -18,6 +26,19 @@ import { designTokens } from '../../styles/tokens.css'; * Title, count, sort, actions — in that order, in one component, so a * new view gets the shape by using it rather than by copying whichever * neighbour it happened to read. + * + * **Actions are data, and `` is the exception.** + * Playlists' three buttons totalled 390px inside a header that gets + * 700px at 900×600 and clipped "New Smart Playlist" to 114 of its 162 + * (#69) — a live defect at a size the app promises, against plan 018's + * *no action is ever unreachable at any supported size*. The header + * cannot fix that for slotted markup: it cannot move another + * component's light-DOM children into a dropdown and keep their + * behaviour, and arbitrary markup offers nothing generic to render as + * a menu item. So a host declares `PageAction[]` and the header picks + * the rendering. The slot survives for markup a data list genuinely + * cannot express, at the stated cost that **a slotted action does not + * collapse** and must therefore fit at 800×600. */ export interface SortOption { @@ -27,6 +48,50 @@ export interface SortOption { export type SortDirection = 'asc' | 'desc'; +/** + * An action that only makes sense while it is a button. + * + * A drop target is the case: you cannot drag a track onto a closed + * menu, so the affordance is absent from the overflow rather than + * approximated there. The header wires these onto the button it + * renders and owns none of them — the same division the sort control + * already lives by. + */ +export interface PageActionDrop { + /** True while an acceptable payload is over the button. */ + active?: boolean; + onDragOver: (e: DragEvent) => void; + onDragLeave: (e: DragEvent) => void; + onDrop: (e: DragEvent) => void; +} + +/** + * One thing a view can do, as data rather than as markup. + * + * `` cannot be collapsed, and that is a fact about + * the API rather than an effort estimate (#69): a component cannot move + * another component's light-DOM children into a dropdown and keep their + * behaviour, and there is nothing generic in arbitrary markup to render + * as a menu item. Declaring an action instead is what lets the header + * choose between the two renderings. + */ +export interface PageAction { + id: string; + label: string; + /** From `utils/icon-language`, never a literal. */ + icon: string; + onSelect: () => void; + /** + * Higher survives longer. The lowest collapses first, ties broken + * by declaration order from the right, so a host that says nothing + * gets "the last one written goes first". + */ + priority?: number; + disabled?: boolean; + title?: string; + drop?: PageActionDrop; +} + @customElement('page-header') export class PageHeader extends LitElement { /** @@ -80,8 +145,82 @@ export class PageHeader extends LitElement { @property({ type: Boolean }) busy = false; + /** + * What this view can do, in the order it wants them shown. + * + * The header decides what *fits*; the host decides what *happens*. + * That is the rule the sort control already lives by — it asks for + * a sort rather than performing one — and actions follow it, which + * is why an action carries a handler rather than the header + * carrying a verb it would have to interpret. + */ + @property({ attribute: false }) + actions: PageAction[] = []; + + /** Action ids currently in the overflow menu. Derived, never set by a host. */ + @state() + private collapsed: ReadonlySet = new Set(); + + @state() + private menuOpen = false; + + @query('.page-header') + private headerEl?: HTMLElement; + + @query('.more-button') + private moreButton?: HTMLButtonElement; + + @query('#page-header-overflow') + private menuPanel?: HTMLElement; + + @query('wa-popup') + private popup?: WaPopup; + + private menuKeyboard = new MenuKeyboard(() => this.closeMenu()); + + private resizeObserver?: ResizeObserver; + + /** + * Whether the outside-click listener is attached. + * + * A `removeEventListener` with no matching `add` is not harmless + * here: `view-lifecycle.test.ts` counts document listeners across a + * view's life and an unconditional detach on disconnect shows up as + * `held: -1`, which is the same accounting that would hide a real + * leak in the other direction. + */ + private outsideCloseAttached = false; + + /** + * What the last fit was measured against. + * + * `updated()` runs on every pass, so it has to say what it depends + * on or it re-measures — and a measurement here forces synchronous + * layout. Width changes arrive through the ResizeObserver; this key + * covers everything *else* in the flex row that can change how much + * of it the actions are left. + + */ + private lastFitKey = ''; + + override connectedCallback(): void { + super.connectedCallback(); + + this.resizeObserver = new ResizeObserver(() => this.measureFit()); + this.resizeObserver.observe(this); + } + + override disconnectedCallback(): void { + super.disconnectedCallback(); + + this.resizeObserver?.disconnect(); + this.resizeObserver = undefined; + this.detachOutsideClose(); + } + static override styles = [ designTokens, + contextMenuStyles, css` :host { display: block; @@ -96,12 +235,26 @@ export class PageHeader extends LitElement { border-bottom: 1px solid var(--yj-border-subtle, #333); } + /* The title gives way before an action does. + + Everything in this row was flex-shrink: 0, so whatever + came last lost — and the actions come last, which is how + the "More actions" button ended up 76px off the right + edge of a 320px viewport with every action already + collapsed into it. The title is the one thing here the + navigation also says (the sidebar item is selected, the + bottom-nav tab is current), so it is the cheapest thing + to truncate; the count, the sort and the actions are each + the only place they are said. */ h1 { margin: 0; font-size: var(--yj-text-xl, 18px); font-weight: 600; color: var(--yj-text-primary, #fff); white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + min-width: 0; } .count { @@ -191,6 +344,98 @@ export class PageHeader extends LitElement { ::slotted(*) { flex-shrink: 0; } + + .actions { + display: flex; + align-items: center; + gap: 8px; + flex-shrink: 0; + } + + .action, + .more-button { + background: none; + border: 1px solid var(--yj-border-subtle, #555); + border-radius: 4px; + color: var(--yj-text-primary, #fff); + padding: 6px 12px; + font-size: var(--yj-text-md, 13px); + font-family: inherit; + cursor: pointer; + display: flex; + align-items: center; + gap: 6px; + white-space: nowrap; + flex-shrink: 0; + } + + .more-button { + padding: 6px 10px; + } + + /* The display: flex above outranks the UA stylesheet's + rule for [hidden], and hiding is how an action + collapses. (No backticks in here: one ends the css + literal, and what you get is "css(...) is not a + function" a long way from the cause.) */ + .action[hidden], + .more-button[hidden] { + display: none; + } + + .action:hover, + .more-button:hover, + .action.drag-over { + border-color: var(--yj-accent, #ffd43b); + color: var(--yj-accent-text, #ffd43b); + } + + .action.drag-over { + background-color: var( + --yj-accent-bg-strong, + rgba(255, 212, 59, 0.15) + ); + } + + .action:disabled { + opacity: 0.5; + cursor: default; + } + + .action:focus-visible, + .more-button:focus-visible { + outline: 2px solid var(--yj-accent, #ffd43b); + outline-offset: -1px; + } + + wa-popup { + z-index: 200; + } + + /* A component states what it drops at phone width itself, + in its own stylesheet, because a media query inside a + shadow root is answered by the viewport and the shell + cannot reach in. Here that is one word: the sort control + is 172px of a 320px header, and "Sort:" is ~40px of it + for a label the adjacent direction arrow already implies. + It stays in the accessibility tree — it is the select's + accessible name, so hiding it outright would rename the + control to nothing — which is config-field's bug, one + component over. clip-path rather than display: none for + the reason styles/sr-only.css.ts gives. */ + @media (max-width: 599px) { + .sort-label { + position: absolute; + width: 1px; + height: 1px; + margin: -1px; + padding: 0; + overflow: hidden; + clip-path: inset(50%); + white-space: nowrap; + border: 0; + } + } `, ]; @@ -212,11 +457,293 @@ export class PageHeader extends LitElement { ${this.renderCount()}
${this.renderScope()} ${this.renderSort()} + ${this.renderActions()} `; } + protected override updated(): void { + const key = [ + this.heading, + this.count, + this.countNoun, + this.countPlural, + this.searchTerm, + this.sortOptions.length, + this.sortField, + this.sortDirection, + this.busy, + this.actions.map((a) => `${a.id}:${a.label}:${a.disabled ?? false}`).join(','), + ].join('|'); + + if (key === this.lastFitKey) return; + + this.lastFitKey = key; + this.measureFit(); + } + + // ================================================================= + // What fits + // ================================================================= + + /** + * Decide which actions are buttons and which are menu items. + * + * Two things about the shape of this are load-bearing. + * + * **Every pass starts from all-visible**, so the collapsed set is a + * pure function of the current width rather than of the order the + * widths arrived in. A rule that only ever *added* to the set would + * never give an action back when the window grew, and one that + * adjusted by a step would need a hysteresis band to stop it + * oscillating on the pixel where a button exactly fits. + * + * **It flips `hidden` on the rendered nodes rather than re-rendering + * between steps.** Reading `scrollWidth` forces layout, which is the + * point; awaiting a Lit update between steps instead would let the + * intermediate all-visible state paint, so the fix would flash the + * overflow it exists to prevent. The reactive state is set once, at + * the end, and the next render agrees with what was measured. + * + * The budget is the *header's* overflow and not the actions row's, + * because the count and the sort control are `flex-shrink: 0` and + * are therefore competing for the same width — only `.scope` gives + * way, which is what it has an ellipsis for. + */ + private measureFit(): void { + const header = this.headerEl; + + if (!header) return; + + if (this.actions.length === 0) { + this.commitCollapsed(new Set()); + + return; + } + + const buttons = new Map(); + + for (const el of this.renderRoot.querySelectorAll( + '[data-action-id]', + )) { + const id = el.dataset['actionId']; + + if (id !== undefined) buttons.set(id, el); + } + + const more = this.moreButton; + const title = this.renderRoot.querySelector('h1'); + + /** + * Nothing is clipped — which is not the same as the header not + * overflowing, and the difference is a trap worth naming. + * + * Once the title can ellipsis, it absorbs the pressure and + * `scrollWidth` reports a header that fits perfectly while the + * heading reads "Playlis…". That is this issue's own failure + * mode moved from the button to the title, and it is invisible + * to exactly the same measurement that missed it the first time. + * So the title's own truncation counts as not fitting, and + * collapsing an action is tried before the title gives way. + */ + const fits = () => + header.scrollWidth <= header.clientWidth && + (title === null || title.scrollWidth <= title.clientWidth + 1); + + for (const el of buttons.values()) el.hidden = false; + + if (more) more.hidden = true; + + const collapsed = new Set(); + + if (!fits()) { + if (more) more.hidden = false; + + for (const action of this.collapseOrder()) { + collapsed.add(action.id); + + const el = buttons.get(action.id); + + if (el) el.hidden = true; + + if (fits()) break; + } + } + + this.commitCollapsed(collapsed); + } + + /** Lowest priority first; ties broken from the right. */ + private collapseOrder(): PageAction[] { + return this.actions + .map((action, index) => ({ action, index })) + .sort( + (a, b) => + (a.action.priority ?? 0) - (b.action.priority ?? 0) || + b.index - a.index, + ) + .map(({ action }) => action); + } + + private commitCollapsed(next: Set): void { + const same = + next.size === this.collapsed.size && + [...next].every((id) => this.collapsed.has(id)); + + if (same) return; + + this.collapsed = next; + + // Nothing left to show in it. Closing rather than leaving an + // empty menu open is the same rule the shelves follow. + if (next.size === 0 && this.menuOpen) this.closeMenu(); + } + + // ================================================================= + // Rendering + // ================================================================= + + private renderActions() { + if (this.actions.length === 0) return nothing; + + const overflowed = this.actions.filter((a) => this.collapsed.has(a.id)); + + return html` +
+ ${this.actions.map((a) => this.renderActionButton(a))} + + + + +
+ `; + } + + private renderActionButton(a: PageAction) { + const drop = a.drop; + + return html` + + `; + } + + // ================================================================= + // The overflow menu + // ================================================================= + + private onActionSelect(a: PageAction): void { + if (a.disabled === true) return; + + this.closeMenu(); + a.onSelect(); + } + + private onMoreClick = (): void => { + if (this.menuOpen) { + this.closeMenu(); + + return; + } + + this.menuOpen = true; + + void this.updateComplete.then(() => { + if (!this.menuOpen) return; + + this.popup?.reposition(); + this.menuKeyboard.open(this.menuPanel ?? null, this.moreButton); + this.attachOutsideClose(); + }); + }; + + private closeMenu(): void { + if (!this.menuOpen) return; + + this.detachOutsideClose(); + this.menuKeyboard.close(); + this.menuOpen = false; + } + + /** + * A click anywhere else closes it. `composedPath` rather than + * `contains`, because the trigger and the panel are both inside + * this shadow root and a click retargets at the host. + */ + private onOutsideDown = (e: Event): void => { + if (e.composedPath().includes(this.menuPanel as EventTarget)) return; + if (e.composedPath().includes(this.moreButton as EventTarget)) return; + + this.closeMenu(); + }; + + private attachOutsideClose(): void { + if (this.outsideCloseAttached) return; + + this.outsideCloseAttached = true; + document.addEventListener('mousedown', this.onOutsideDown, true); + } + + private detachOutsideClose(): void { + if (!this.outsideCloseAttached) return; + + this.outsideCloseAttached = false; + document.removeEventListener('mousedown', this.onOutsideDown, true); + } + private renderCount() { if (this.count === null) return nothing; @@ -258,7 +785,10 @@ export class PageHeader extends LitElement { if (this.sortOptions.length === 1) { return html`
- Sort: ${this.sortOptions[0]?.label} + Sort: ${this.sortOptions[0]?.label} ${this.renderDirectionButton(ascending)}
`; @@ -267,7 +797,7 @@ export class PageHeader extends LitElement { return html`