fix(page-header): collapse the actions that do not fit into a menu
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 6m53s

Playlists slotted three buttons totalling 390px into a header that gets
700px at 900x600, so "New Smart Playlist" rendered 114 of its 162px
with the queue closed, and 158 of 162 at the 800x600 enforced minimum.
On a phone none of the three could be reached at all, which is what the
Android report said. Plan 018's size matrix promises the opposite: no
action is ever unreachable at any supported size.

The header could not fix that for slotted markup, and that is a fact
about the API rather than an effort estimate — a component 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 passes `PageAction[]` and the header chooses the
rendering; the slot survives for markup a data list cannot express, at
the stated cost that a slotted action does not collapse.

All three hosts that slot actions migrated, which also normalises the
plain-<button>/<wa-button> split between them onto one shape the header
styles — and lets it measure a button that has already upgraded, rather
than a wa-button whose shadow DOM arrives in its own first update.

Four things in it are load-bearing:

- Every measuring pass starts from all-visible, so the collapsed set is
  a pure function of the current width and an action comes back when
  the window grows. It flips `hidden` imperatively rather than
  re-rendering between steps, or the intermediate state paints and the
  fix flashes the overflow it exists to prevent.
- "Fits" means nothing is clipped, not that the header does not
  overflow. Once the title can ellipsis it absorbs the pressure and
  scrollWidth reports a perfect fit while the heading reads "Playlis…"
  — this bug moved from the button to the title, and invisible to the
  same measurement that missed it the first time.
- New Playlist has the highest priority because it is the drop target
  and a closed menu cannot be one. `PageAction.drop` therefore carries
  the host's own handlers; the affordance is absent from the overflow
  rather than approximated there.
- The overflow trigger is a named button with aria-expanded and an
  aria-controls naming a panel that is always in the DOM, and the
  keyboard model is the shared `MenuKeyboard`.

`layout-overflow.spec.ts` passes on the broken build — it asserts the
shell needs no sideways scrolling, and clipping inside a component is
invisible to it, which is why this defect survived a spec named for it.
The new spec measures each button against its own header at four
viewports and asserts buttons plus menu account for every declared
action, without which it would pass vacuously on a build rendering none.

Closes #69
This commit is contained in:
2026-08-19 13:18:55 -04:00
parent a1ee967323
commit f1c066db6e
12 changed files with 1304 additions and 123 deletions
+12 -1
View File
@@ -7,6 +7,7 @@
* opening it.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/home-view/home-view';
import { stub, calls, lastArgs, stubFailure } from '@test/support/harness';
@@ -166,7 +167,17 @@ describe('home view', () => {
const before = calls('home.Service.GetShelves').length;
shadow<HTMLElement>(el, 'wa-button')!.click();
// The action is declared to `page-header` rather than slotted as
// markup (#69), so it is a button in *that* shadow root now.
const header = shadow<HTMLElement>(el, 'page-header')!;
await (header as LitElement).updateComplete;
header.shadowRoot!
.querySelector<HTMLButtonElement>(
'[data-testid="page-action-shuffle-suggestions"]',
)!
.click();
await el.updateComplete;
expect(calls('home.Service.GetShelves').length).toBe(before + 1);
@@ -44,6 +44,8 @@ const GOVERNED = [
'regular/bookmark',
'bars-staggered',
'tag',
'filter',
'ellipsis',
];
/** The one file allowed to say them, plus its own test. */
+320 -1
View File
@@ -9,7 +9,7 @@
* the thing no assertion can — the header looking wrong.
*/
import { describe, expect, it } from 'vitest';
import type { PageHeader } from '@components/page-header/page-header';
import type { PageAction, PageHeader } from '@components/page-header/page-header';
import '@components/page-header/page-header';
import { fixture, shadow, shadowAll, update, visual } from '@test/support/render';
@@ -19,6 +19,67 @@ const SORTS = [
{ id: 'tracks', label: 'Tracks' },
];
/**
* Three actions of the shape that broke: Playlists' own, whose widths
* (91 + 122 + 162 = 390px) are what a 700px header could not hold.
*/
function playlistActions(seen: string[]): PageAction[] {
return [
{
id: 'import',
label: 'Import',
icon: 'file-import',
priority: 0,
onSelect: () => seen.push('import'),
},
{
id: 'new-playlist',
label: 'New Playlist',
icon: 'plus',
priority: 2,
onSelect: () => seen.push('new-playlist'),
},
{
id: 'new-smart-playlist',
label: 'New Smart Playlist',
icon: 'filter',
priority: 1,
onSelect: () => seen.push('new-smart-playlist'),
},
];
}
/**
* Resize and let the fit settle.
*
* The rule is driven by a ResizeObserver, which delivers before paint
* and therefore after the microtask queue an `updateComplete` drains —
* so this waits on frames rather than on promises, and then on the
* render the measurement asks for.
*/
async function widthOf(el: PageHeader, px: number): Promise<void> {
el.style.width = `${px}px`;
for (let frame = 0; frame < 3; frame += 1) {
await new Promise((r) => requestAnimationFrame(r));
await el.updateComplete;
}
}
/** The labels currently rendered as buttons, in order. */
function buttons(el: PageHeader): string[] {
return shadowAll<HTMLButtonElement>(el, '.action')
.filter((b) => !b.hidden)
.map((b) => b.textContent?.trim() ?? '');
}
/** The labels currently in the overflow menu, in order. */
function menu(el: PageHeader): string[] {
return shadowAll(el, '#page-header-overflow wa-dropdown-item').map(
(i) => i.textContent?.trim() ?? '',
);
}
describe('<page-header>', () => {
it('renders the heading as the page\u2019s only h1', async () => {
const el = await fixture<PageHeader>('page-header', {
@@ -153,6 +214,264 @@ describe('<page-header>', () => {
});
});
/**
* #69: Playlists slotted three buttons totalling 390px into a header
* that gets 700px at 900×600, and "New Smart Playlist" rendered 114 of
* its 162. It survived a spec named `layout-overflow` because that one
* asserts the *shell* needs no sideways scrolling — clipping inside a
* component is invisible to it.
*
* The header can only fix that for actions it renders itself, which is
* why they are data now. These are the assertions about the rule; the
* e2e spec is what checks it against the real widths.
*/
describe('<page-header> actions', () => {
it('renders a declared action, and asks the host to perform it', async () => {
// Same division the sort control already lives by: the header
// decides what fits, the host decides what happens.
const seen: string[] = [];
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
actions: playlistActions(seen),
});
await widthOf(el, 1200);
expect(buttons(el)).toEqual([
'Import',
'New Playlist',
'New Smart Playlist',
]);
shadow<HTMLButtonElement>(el, '[data-testid="page-action-import"]')!.click();
expect(seen).toEqual(['import']);
});
it('hides the overflow trigger while everything fits', async () => {
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
actions: playlistActions([]),
});
await widthOf(el, 1200);
expect(shadow<HTMLButtonElement>(el, '.more-button')!.hidden).toBe(true);
expect(menu(el)).toEqual([]);
});
it('collapses the lowest priority first', async () => {
// Import is lowest because it is rarest; New Playlist is highest
// because it is the drop target, and a closed menu cannot be one.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions([]),
});
// Asserted as the *order* rather than at two chosen widths: which
// pixel drops which button depends on the font and on the shell
// this tier does not have, and pinning those numbers here would be
// a test of the fixture. What the host declares is a sequence.
const states: string[][] = [];
for (let width = 1200; width >= 300; width -= 40) {
await widthOf(el, width);
const now = menu(el);
const last = states[states.length - 1];
if (last === undefined || last.join() !== now.join()) states.push(now);
}
expect(states).toEqual([
[],
['Import'],
['Import', 'New Smart Playlist'],
['Import', 'New Playlist', 'New Smart Playlist'],
]);
// The menu lists them in the host's declared order, not in the
// order they happened to collapse — a menu that reshuffles itself
// as the window narrows is a menu nobody can learn.
expect(buttons(el)).toEqual([]);
});
it('gives an action back when the width returns', async () => {
// Every pass starts from all-visible, so the collapsed set is a
// function of the current width and not of how it got there. A rule
// that only ever added to the set would never widen again.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions([]),
});
await widthOf(el, 420);
expect(buttons(el)).toEqual([]);
await widthOf(el, 1200);
expect(menu(el)).toEqual([]);
expect(buttons(el)).toEqual([
'Import',
'New Playlist',
'New Smart Playlist',
]);
});
it('collapses an action before it truncates the title', async () => {
// The title can ellipsis, which means `scrollWidth` reports a
// header that fits perfectly while the heading reads "Playlis…" —
// this issue's failure mode moved from the button to the title, and
// invisible to the same measurement that missed it the first time.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions([]),
});
await widthOf(el, 700);
const h1 = shadow<HTMLElement>(el, 'h1')!;
expect(h1.scrollWidth).toBeLessThanOrEqual(h1.clientWidth + 1);
expect(menu(el).length).toBeGreaterThan(0);
});
it('names the overflow trigger and says what it controls', async () => {
// An overflow menu is exactly the shape that grows a nameless
// control, and `aria-controls` cannot name an element that is not
// in the DOM — which is why the panel renders unconditionally and
// `wa-popup` hides it, the same rule `config-section` follows.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions([]),
});
await widthOf(el, 480);
const more = shadow<HTMLButtonElement>(el, '.more-button')!;
expect(more.hidden).toBe(false);
expect(more.getAttribute('aria-label')).toBe('More actions');
expect(more.getAttribute('aria-expanded')).toBe('false');
expect(more.getAttribute('aria-haspopup')).toBe('menu');
const panel = shadow<HTMLElement>(el, '#page-header-overflow')!;
expect(more.getAttribute('aria-controls')).toBe(panel.id);
expect(panel.getAttribute('role')).toBe('menu');
more.click();
await el.updateComplete;
expect(
shadow<HTMLButtonElement>(el, '.more-button')!.getAttribute(
'aria-expanded',
),
).toBe('true');
});
it('runs a collapsed action from the menu, and closes it', async () => {
const seen: string[] = [];
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions(seen),
});
await widthOf(el, 700);
shadow<HTMLButtonElement>(el, '.more-button')!.click();
await el.updateComplete;
shadowAll<HTMLElement>(el, '#page-header-overflow wa-dropdown-item')[0]!.click();
await el.updateComplete;
expect(seen).toEqual(['import']);
expect(
shadow<HTMLButtonElement>(el, '.more-button')!.getAttribute(
'aria-expanded',
),
).toBe('false');
});
it('keeps a drop target a drop target, and does not fake one in the menu', async () => {
// You cannot drag a track onto a closed menu, so the affordance is
// absent from the overflow rather than approximated there. The
// header wires the handlers onto the button and owns none of them.
const dropped: string[] = [];
const actions: PageAction[] = [
{
id: 'new-playlist',
label: 'New Playlist',
icon: 'plus',
onSelect: () => undefined,
drop: {
active: true,
onDragOver: () => dropped.push('over'),
onDragLeave: () => dropped.push('leave'),
onDrop: () => dropped.push('drop'),
},
},
];
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
actions,
});
await widthOf(el, 1200);
const button = shadow<HTMLElement>(
el,
'[data-testid="page-action-new-playlist"]',
)!;
expect(button.classList.contains('drag-over')).toBe(true);
button.dispatchEvent(new DragEvent('dragover', { bubbles: true }));
button.dispatchEvent(new DragEvent('drop', { bubbles: true }));
expect(dropped).toEqual(['over', 'drop']);
// …and collapsed, it is a menu item with no drop wiring at all.
await widthOf(el, 120);
expect(menu(el)).toEqual(['New Playlist']);
expect(
shadow<HTMLElement>(el, '[data-testid="page-action-new-playlist"]')
?.hidden,
).toBe(true);
});
it('renders nothing at all for a view with no actions', async () => {
// Two of the three hosts have one action and one has none while its
// other tab is up; an empty actions row is not a mode.
const el = await fixture<PageHeader>('page-header', { heading: 'Albums' });
await widthOf(el, 900);
expect(shadow(el, '.actions')).toBeNull();
});
});
describe('<page-header> as each view wears it', () => {
// One baseline per arrangement rather than per view: the point is
// that eight views produce four shapes, not eight.