Compare commits

..
Author SHA1 Message Date
logan f967916550 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 2m36s
CI / e2e (pull_request) Successful in 7m17s
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
2026-08-19 15:08:57 -04:00
logan 3fa7c7734b docs: record the page-header actions rule, and complete plan 018
The `page-header` paragraph already stated "the header asks for a sort,
it does not perform one"; actions now follow the same division and it
belongs beside it — the header decides what fits, the host decides what
happens.

Plan 018 moves to completed/ because #69 was the last thing it owed:
its size matrix promised "no action is ever unreachable at any
supported size" and the residual 114/162px clip was that promise
outstanding. Its recap also corrects a claim the plan made — the queue
and the actions were not the only two things competing for the header's
width, since every child of that flex row was flex-shrink: 0 and the
actions come last.
2026-08-19 15:08:57 -04:00
logan cceeb40b16 Merge pull request 'Six quick fixes off the tracker: tooling, a latent index bug, and two touch affordances' (#139) from fix/quick-wins-batch into main
CI / check (push) Successful in 2m34s
CI / e2e (push) Successful in 6m44s
Closes #130
Closes #131
Closes #119
Closes #118
Closes #68
Closes #61
2026-08-19 19:08:46 +00:00
yonlu 2926ecd4b4 docs(notes): record that no test tier can see a hover media query
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m2s
CI / e2e (pull_request) Successful in 6m33s
Both browser tiers are blind to `(hover: hover)` gating, in different
ways and without failing: CDP media emulation does not reach ui-test's
iframe, and e2e's phone specs reach phone width with setViewportSize,
which changes no media feature but width. Written down with what does
work — a device-descriptor context — because the next person to gate an
affordance this way will otherwise re-derive it, and the tempting
conclusion from a green suite is that the gate is covered.
2026-08-19 14:08:38 -04:00
yonlu ff3c4003cb Merge branch 'fix/61-mini-player-plain-text' into fix/quick-wins-batch 2026-08-19 14:08:15 -04:00
yonlu def596a99e Merge branch 'fix/68-hover-affordances-pointer' into fix/quick-wins-batch 2026-08-19 14:08:14 -04:00
yonlu 14f78c0b57 Merge branch 'fix/118-in-library-clear' into fix/quick-wins-batch 2026-08-19 14:08:14 -04:00
yonlu 7cea238e71 Merge branch 'fix/119-dev-headless-port' into fix/quick-wins-batch 2026-08-19 14:08:13 -04:00
yonlu 4f2f1827ab Merge branch 'fix/131-codegen-check-scope' into fix/quick-wins-batch 2026-08-19 14:08:13 -04:00
yonlu e454e4074b Merge branch 'fix/130-issue-claim-user' into fix/quick-wins-batch 2026-08-19 14:08:12 -04:00
yonlu c518ac8c73 feat(now-playing): plain text instead of links in the phone mini player
CI / check (push) Skipped
CI / e2e (push) Skipped
The bottom bar's title, artist and "Playing from X" all navigate. In a
bar sized for a bar they are a few characters of text, which is not a
touch target — and explore-link holds its navigation for one
double-click interval and drops it if a second click arrives, a gesture
that exists so double-clicking a row can play it and that means nothing
on touch.

Below the shell's phone breakpoint the three render as plain text. The
words are unchanged: the source line still says where the queue came
from, because dropping the link is the change and dropping the
information would be a different and worse one. The cover art already
carries the phone-only button that opens the full-screen Now Playing
view, which is where the links live.

This is in JS rather than in the stylesheet because what changes is the
content, not its appearance — no CSS rule takes a click handler off an
element. matchMedia is read in connectedCallback for the reason the
reduce-motion query beside it already is, so a test can answer it first.

Two smaller things. PHONE_QUERY moves out of track-list.ts into
utils/breakpoints.ts: it was a private const when one component needed
it, and a second reader is where a copy starts drifting from index.css.
And `phone` joins geometryKey(), because crossing the breakpoint swaps a
link for a bare string and the marquee travels a distance read from
measuring it — the words being identical either side is not the same as
the box measuring the same.

Closes #61
2026-08-19 14:08:06 -04:00
yonlu 977f624123 fix(home): gate the card play button on the device having hover
CI / check (push) Skipped
CI / e2e (push) Skipped
The play button on a home shelf's cover cards is revealed by :hover, and
a touch long-press synthesises a hover state in the WebView — so on a
phone it flashed into view during the 500ms hold that
utils/long-press.ts is measuring for a context menu. A control appearing
because the user was reaching for a different one.

It is gated on `(hover: hover) and (pointer: fine)` rather than on width,
so it is absent on any touch device and present on a desktop with a small
window. A phone user taps the album and plays from the detail view, so
nothing replaces it.

The default outside the query is display:none, not opacity:0. An
opacity-0 button still takes taps and is still in the accessibility tree,
so leaving the reveal as the only guarded part would keep the hit area
for a control the phone can never show.

The test asserts the parsed stylesheet rather than rendering as a phone,
and says so: CDP's Emulation.setEmulatedMedia does not reach this tier's
iframe, so matchMedia still answers `hover: hover` after it is set. The
regression worth catching is someone hoisting the rule back out of the
query as a tidy-up — a change no desktop assertion can see.

Closes #68
2026-08-19 14:08:06 -04:00
yonlu 8d46c4abb7 fix(scripts): refuse to start dev-headless on a port somebody else holds
CI / check (push) Skipped
CI / e2e (push) Skipped
dev-headless.sh checked the PID in *this* worktree's .dev/app.pid and
nothing else, so an app orphaned by a deleted worktree went on listening
with nothing left to stop it — `make dev-stop` only kills the pid it
wrote. The new app then started, failed to bind, exited, and every
subsequent curl and playwright-cli call went to the other process: the
harness reported facts about an app nobody asked for.

That fails a long way from its cause. It presented as "no such table:
libraries" against a *freshly created* YJ_HOME, which reads exactly like
applySchema or staleshape.go having gone wrong, with a zero-byte app.log
beside it saying nothing.

The startup wait cannot catch this, because its health check is satisfied
by any app on the port — which is precisely the failure — so the check is
before the launch and refuses rather than warns. It names the holder's
pid, cmdline and /proc/<pid>/cwd, which is what identifies the checkout
and says "(deleted)" for the case this exists for. It does not suggest
`make dev-stop`: the PID-file check has already passed, so by
construction dev-stop does not know about this process and would report
success while changing nothing. --port already covers the legitimate
second-app case.

The second, cheaper guard the report asks for goes in after the wait:
"the port answered" is not "the app we started answered", so a dead
APP_PID at that point is now an error with the log tail rather than a
success message about somebody else's process.

Closes #119
2026-08-19 14:08:05 -04:00
yonlu f714fe513d fix(scripts): report only what generation changed, not the worktree
CI / check (push) Skipped
CI / e2e (push) Skipped
The codegen-check hook was `go generate` followed by a bare
`git diff --name-only`, which is the whole unstaged worktree rather than
the generators' output. So a commit whose staged changes were fine failed
whenever anything unrelated sat unstaged — notes, a plan document, the
next commit's files — reporting "Generated code is out of date" and then
a diffstat of files no generator has ever written. `make generate` fixed
nothing, because nothing was stale, so the message sent you looking for a
codegen problem that did not exist. Splitting one piece of work into
several commits is exactly the shape that triggers it.

The tree is snapshotted either side of `go generate` and only what moved
across it is reported. That is deliberately a snapshot rather than the
list of generated paths the issue offers as the other option: a fourth
generator is one //go:generate line away, and a path list is a second
place to remember it.

Two things it has to get right. The comparison is a *symmetric*
difference, because generation can push a file into the unstaged set or
pull it out of one — a hand-edited generated file that the generator puts
back is stale generated code just as much as a source change that
outdates it, and comparing one direction reports it as current. And the
snapshot is content, not names, or a generated file that was already
dirty and is then rewritten further keeps its name on both sides and
slips through.

Closes #131
2026-08-19 14:08:05 -04:00
yonlu 087c69ac8d fix(scripts): let issue.sh claim work on a write:issue-only token
CI / e2e (push) Skipped
CI / check (push) Skipped
`claim` is the one step the workflow requires before the first edit, and
it failed outright on a token scoped to the work it does: `me()` calls
`GET /user` purely to name the assignee, and that endpoint needs
read:user. So the documented process was blocked by its own tooling, and
the fallback was to do the assignment, the label and the comment by hand
— which is the half-made claim `claim` exists to prevent.

GITEA_USER short-circuits the lookup, so least privilege is enough. The
lookup stays as the fallback because it is right when the scope is there
and needs no setup. Failure is now actionable and says both remedies,
and it still happens before any of the three halves are mutated.

Closes #130
2026-08-19 14:08:05 -04:00
24 changed files with 1973 additions and 164 deletions
+32
View File
@@ -3664,3 +3664,35 @@ knowing before someone "fixes" it as broken: sampled from screenshots at
900×600, the main panel's background goes 33,37,41 → 18,20,23 and a
row's text 242 → 133. It covers the content area only — not the sidebar
or the transport — because the queue is not modal.
## No test tier can see a `hover:` media query (measured 2026-08-19)
Gating an affordance on `(hover: hover) and (pointer: fine)` — #68's fix
for the play button that flashed on a long-press — is invisible to both
browser tiers, in *different* ways, and neither of them fails.
- **`make ui-test`**: CDP's `Emulation.setEmulatedMedia` with a `hover`
feature does not reach the tier's iframe. The call succeeds and
`matchMedia('(hover: hover)')` still answers `true` afterwards. So
there is no way to render a component as a phone would and read the
computed style.
- **`make e2e`**: both projects are desktop (`Desktop Chrome`,
`Desktop Safari`), and the phone specs reach phone *width* with
`setViewportSize`, which changes no media feature but `width`. So the
phone specs run with `hover: hover` and the gate is never exercised.
What does work, and what the fix was verified with, is a second browser
context under a device descriptor: `chromium.newContext(devices['Pixel
5'])` reports `hover=false pointer:fine=false` and the button computes
`display: none`, against `flex` at 1440px. That is a one-off script, not
a spec — `isMobile` is Chromium-only, so it cannot become an e2e project
without losing the WebKit half.
`hover-affordance.test.ts` therefore asserts the *parsed stylesheet* —
that the reveal rule sits inside the media query — which catches the
regression that actually threatens it: someone hoisting the rule back out
as a tidy-up, a change nothing on a desktop renders differently.
Related: a width-gated decision **is** testable at both tiers, which is
why #61's phone mini player is a `matchMedia` stub in the component test
and needs nothing special.
@@ -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
+73
View File
@@ -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 `<slot name="actions">` 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
+318
View File
@@ -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<HTMLElement>('.page-header')!;
const box = header.getBoundingClientRect();
const title = root.querySelector<HTMLElement>('h1')!;
const clipped = [
...root.querySelectorAll<HTMLElement>('.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<HTMLElement>('.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([]);
});
});
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M0 256a56 56 0 1 1 112 0 56 56 0 1 1 -112 0zm168 0a56 56 0 1 1 112 0 56 56 0 1 1 -112 0zm224-56a56 56 0 1 1 0 112 56 56 0 1 1 0-112z"/></svg>

After

Width:  |  Height:  |  Size: 443 B

@@ -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`
<page-header heading="Downloads">
${this.tab === 'requests'
? html`
<wa-button
slot="actions"
size="small"
appearance="outlined"
?disabled=${this.checking}
title="Search every download client for everything on this list right now, instead of waiting for the next scheduled check"
@click=${() => void this.checkNow()}
>
<wa-icon slot="start" name="rotate"></wa-icon>
${this.checking ? 'Searching…' : 'Check now'}
</wa-button>
`
: nothing}
</page-header>
<page-header
heading="Downloads"
.actions=${this.tab === 'requests'
? ([
{
id: 'check-now',
label: this.checking
? 'Searching\u2026'
: 'Check now',
icon: 'rotate',
disabled: this.checking,
title: 'Search every download client for everything on this list right now, instead of waiting for the next scheduled check',
onSelect: () => void this.checkNow(),
},
] satisfies PageAction[])
: []}
></page-header>
<p class="subtitle">
Music you have requested, and the download attempts that
+63 -38
View File
@@ -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';
@@ -160,29 +161,52 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
user-select: none;
}
/*
* The hover play button is a *hover* affordance, so it is
* gated on the device having hover rather than on width. A
* touch long-press synthesises a hover state in the WebView,
* so on a phone it flashed into view during the 500ms hold
* that utils/long-press.ts is measuring for a context menu —
* a control appearing because you were reaching for a
* different one. A phone user taps the album and plays from
* the detail view, so there is nothing to replace it with.
*
* display:none outside the query rather than opacity:0 on
* its own: an opacity-0 button still takes taps and is
* still in the accessibility tree, so the invisible control
* would keep the hit area it was never meant to have on
* touch. Everything else stays inside, so the desktop
* animation is unchanged.
*/
.play {
position: absolute;
right: 8px;
bottom: 8px;
width: 38px;
height: 38px;
border: none;
border-radius: 50%;
background: var(--yj-accent, #ffd43b);
color: var(--yj-accent-fg, #000);
display: flex;
align-items: center;
justify-content: center;
cursor: pointer;
opacity: 0;
transform: translateY(6px);
transition: opacity 0.12s ease, transform 0.12s ease;
display: none;
}
.card:hover .play,
.card:focus-within .play {
opacity: 1;
transform: translateY(0);
@media (hover: hover) and (pointer: fine) {
.play {
position: absolute;
right: 8px;
bottom: 8px;
width: 38px;
height: 38px;
border: none;
border-radius: 50%;
background: var(--yj-accent, #ffd43b);
color: var(--yj-accent-fg, #000);
display: flex;
align-items: center;
justify-content: center;
cursor: pointer;
opacity: 0;
transform: translateY(6px);
transition: opacity 0.12s ease, transform 0.12s ease;
}
.card:hover .play,
.card:focus-within .play {
opacity: 1;
transform: translateY(0);
}
}
.name {
@@ -260,23 +284,24 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
override render() {
return html`
<page-header heading="Home">
<!-- "Shuffle" alone was two different controls with one
name: this one and the transport's shuffle mode.
They were never on screen together until the app
started landing on Home (H-8), and a cached view is
in the accessibility tree either way. -->
<wa-button
slot="actions"
size="small"
appearance="plain"
title="Reshuffle the suggestions"
@click=${() => void this.load()}
>
<wa-icon slot="start" name="shuffle"></wa-icon>
Shuffle suggestions
</wa-button>
</page-header>
<page-header
heading="Home"
.actions=${[
{
// "Shuffle" alone was two different controls
// with one name: this one and the transport's
// shuffle mode. They were never on screen
// together until the app started landing on
// Home (H-8), and a cached view is in the
// accessibility tree either way.
id: 'shuffle-suggestions',
label: 'Shuffle suggestions',
icon: ICON_SHUFFLE,
title: 'Reshuffle the suggestions',
onSelect: () => void this.load(),
},
] satisfies PageAction[]}
></page-header>
<p class="lede">Somewhere to start listening.</p>
${this.renderBody()}
`;
@@ -13,6 +13,7 @@ import {
isQueueSourceNavigable,
navigateToQueueSource,
} from '@utils/queue-source-link';
import { PHONE_QUERY } from '@utils/breakpoints';
import { PlayerController } from '@store/controllers/player-controller';
import { creditStore } from '@store/credit-store';
import { QueueController } from '@store/controllers/queue-controller';
@@ -80,6 +81,19 @@ export class NowPlaying extends LitElement {
private reduceMotionQuery?: MediaQueryList;
/**
* Phone width, from the shell's own breakpoint.
*
* This is in JS rather than in the stylesheet because what changes
* is the *content*, not its appearance: the title, artist and
* source render as plain text instead of as links, and no CSS rule
* can take a click handler off an element.
*/
@state()
private phone = false;
private phoneQuery?: MediaQueryList;
/** Whether each field is actively mid-scroll (class toggle). */
@state()
private titleScrolling = false;
@@ -341,6 +355,12 @@ export class NowPlaying extends LitElement {
this.reduceMotion = this.reduceMotionQuery?.matches ?? false;
this.reduceMotionQuery?.addEventListener('change', this.handleReduceMotionChange);
// Same reasoning as above: looked up here, not at module load,
// so a test can install its own matchMedia first.
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
this.phone = this.phoneQuery?.matches ?? false;
this.phoneQuery?.addEventListener('change', this.handlePhoneChange);
this.resizeObserver = new ResizeObserver(() => {
this.geometryDirty = true;
this.requestUpdate();
@@ -364,6 +384,7 @@ export class NowPlaying extends LitElement {
this.attachDragListeners(false);
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
this.phoneQuery?.removeEventListener('change', this.handlePhoneChange);
this.resizeObserver?.disconnect();
this.stopScrollCycle('title');
this.stopScrollCycle('artist');
@@ -488,7 +509,7 @@ export class NowPlaying extends LitElement {
@mouseleave=${this.handleTitleMouseLeave}
@transitionend=${() => this.onScrollCycleEnd('title')}
>
<span class="scroll-content">${trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
<span class="scroll-content">${this.phone ? track.title : trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
</span>
<span
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
@@ -498,14 +519,15 @@ export class NowPlaying extends LitElement {
@mouseleave=${this.handleArtistMouseLeave}
@transitionend=${() => this.onScrollCycleEnd('artist')}
>
<span class="scroll-content">${creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
<span class="scroll-content">${this.phone ? track.artist || 'Unknown Artist' : creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
</span>
${describeQueueSource(this.queue.source)
? html`
<span
class="track-source ${isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
class="track-source ${!this.phone && isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
data-testid="now-playing-source"
@click=${(e: MouseEvent) => {
if (this.phone) return;
if (!isQueueSourceNavigable(this.queue.source)) return;
navigateToQueueSource(
e.currentTarget as EventTarget,
@@ -571,6 +593,10 @@ export class NowPlaying extends LitElement {
this.reduceMotion = e.matches;
};
private handlePhoneChange = (e: MediaQueryListEvent): void => {
this.phone = e.matches;
};
private shouldScroll(field: 'title' | 'artist'): boolean {
const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
@@ -606,6 +632,12 @@ export class NowPlaying extends LitElement {
track?.artist ?? '',
this.shouldScroll('title') ? '1' : '0',
this.shouldScroll('artist') ? '1' : '0',
// Crossing the breakpoint swaps a link for a bare string,
// and a link is not guaranteed to measure the same as the
// text inside it. The marquee travels a distance read from
// that measurement, so this belongs in the key even though
// the words are identical either side.
this.phone ? '1' : '0',
].join('\u0000');
}
@@ -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 `<slot name="actions">` 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.
*
* `<slot name="actions">` 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<string> = 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()}
<div class="spacer"></div>
${this.renderScope()} ${this.renderSort()}
${this.renderActions()}
<slot name="actions"></slot>
</header>
`;
}
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<string, HTMLElement>();
for (const el of this.renderRoot.querySelectorAll<HTMLElement>(
'[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<string>();
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<string>): 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`
<div class="actions">
${this.actions.map((a) => this.renderActionButton(a))}
<wa-popup
placement="bottom-end"
flip
shift
.active=${this.menuOpen}
>
<button
slot="anchor"
class="more-button"
type="button"
data-testid="page-actions-more"
aria-label="More actions"
aria-haspopup="menu"
aria-expanded=${this.menuOpen ? 'true' : 'false'}
aria-controls="page-header-overflow"
?hidden=${overflowed.length === 0}
@click=${this.onMoreClick}
>
<wa-icon name=${ICON_MORE_ACTIONS}></wa-icon>
</button>
<div
id="page-header-overflow"
class="context-menu-panel"
role="menu"
aria-label="More actions"
>
${overflowed.map(
(a) => html`
<wa-dropdown-item
?disabled=${a.disabled ?? false}
@click=${() => this.onActionSelect(a)}
>
<wa-icon
slot="icon"
name=${a.icon}
></wa-icon>
${a.label}
</wa-dropdown-item>
`,
)}
</div>
</wa-popup>
</div>
`;
}
private renderActionButton(a: PageAction) {
const drop = a.drop;
return html`
<button
class="action ${drop?.active === true ? 'drag-over' : ''}"
type="button"
data-action-id=${a.id}
data-testid=${`page-action-${a.id}`}
title=${a.title ?? nothing}
?disabled=${a.disabled ?? false}
?hidden=${this.collapsed.has(a.id)}
@click=${() => a.onSelect()}
@dragover=${(e: DragEvent) => drop?.onDragOver(e)}
@dragleave=${(e: DragEvent) => drop?.onDragLeave(e)}
@drop=${(e: DragEvent) => drop?.onDrop(e)}
>
<wa-icon name=${a.icon}></wa-icon>
${a.label}
</button>
`;
}
// =================================================================
// 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`
<div class="sort">
<span>Sort: ${this.sortOptions[0]?.label}</span>
<span
><span class="sort-label">Sort: </span
>${this.sortOptions[0]?.label}</span
>
${this.renderDirectionButton(ascending)}
</div>
`;
@@ -267,7 +797,7 @@ export class PageHeader extends LitElement {
return html`
<div class="sort">
<label>
Sort:
<span class="sort-label">Sort:</span>
<select
data-testid="page-sort"
.value=${this.sortField}
@@ -40,7 +40,10 @@ import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/
import {
ICON_NEW,
ICON_PLAYLIST,
ICON_SMART_PLAYLIST,
} from '@utils/icon-language';
import '@components/page-header/page-header';
import type { PageAction } from '@components/page-header/page-header';
const SCROLL_DEBOUNCE_MS = 100;
@@ -194,11 +197,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
contain: layout style;
}
.header-actions {
display: flex;
gap: 8px;
}
.header-spinner {
display: inline-block;
width: 14px;
@@ -209,33 +207,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
animation: spin 0.6s linear infinite;
}
.new-playlist-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: 13px;
cursor: pointer;
display: flex;
align-items: center;
gap: 6px;
font-family: inherit;
}
.new-playlist-button:hover,
.new-playlist-button.drag-over {
border-color: var(--yj-accent, #ffd43b);
color: var(--yj-accent-text, #ffd43b);
}
.new-playlist-button.drag-over {
background-color: var(
--yj-accent-bg-strong,
rgba(255, 212, 59, 0.15)
);
}
.create-form {
display: flex;
align-items: center;
@@ -455,25 +426,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
min-width: 0;
}
.import-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: 13px;
cursor: pointer;
display: flex;
align-items: center;
gap: 6px;
font-family: inherit;
}
.import-button:hover {
border-color: var(--yj-accent, #ffd43b);
color: var(--yj-accent-text, #ffd43b);
}
.import-error {
padding: 0.5em 0.75em;
margin: 0.5em 16px 0;
@@ -1022,10 +974,11 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
) => {
const related =
e.relatedTarget as Node | null;
const btn =
this.shadowRoot?.querySelector(
'.new-playlist-button',
);
// The button the event was bound to, rather than a selector for
// it: `page-header` renders it now, so it is not in this shadow
// root at all and the old `.new-playlist-button` lookup would
// find nothing and leave the highlight stuck on.
const btn = e.currentTarget as Element | null;
if (btn && !btn.contains(related)) {
this.dragOverNewButton = false;
@@ -1470,6 +1423,49 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
this.saveSortPreferences();
};
/**
* The three things this page can do, as data.
*
* The priority order is what #69's Direction asks for and it is
* only interesting for one of them: **New Playlist is highest
* because it is the drop target**. You cannot drag a track onto a
* closed menu, so collapsing it is the one collapse here that
* removes a capability rather than relocating it. Import is lowest
* because it is the rarest, and at 900×600 it is the only one that
* has to go.
*/
private headerActions(): PageAction[] {
return [
{
id: 'import',
label: 'Import',
icon: 'file-import',
priority: 0,
onSelect: () => void this.handleImportPlaylist(),
},
{
id: 'new-playlist',
label: 'New Playlist',
icon: ICON_NEW,
priority: 2,
onSelect: () => this.handleNewPlaylistClick(),
drop: {
active: this.dragOverNewButton,
onDragOver: this.onNewButtonDragOver,
onDragLeave: this.onNewButtonDragLeave,
onDrop: this.onNewButtonDrop,
},
},
{
id: 'new-smart-playlist',
label: 'New Smart Playlist',
icon: ICON_SMART_PLAYLIST,
priority: 1,
onSelect: () => this.handleNewSmartPlaylistClick(),
},
];
}
override render() {
return html`
<page-header
@@ -1484,33 +1480,8 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
search-term=${this.searchCtrl.term}
?busy=${this.refreshing}
@sort-change=${this.onPageHeaderSort}
.actions=${this.headerActions()}
>
<div slot="actions" class="header-actions">
<button
class="import-button"
@click=${this.handleImportPlaylist}
>
<wa-icon name="file-import"></wa-icon>
Import
</button>
<button
class="new-playlist-button ${this.dragOverNewButton ? 'drag-over' : ''}"
@click=${this.handleNewPlaylistClick}
@dragover=${this.onNewButtonDragOver}
@dragleave=${this.onNewButtonDragLeave}
@drop=${this.onNewButtonDrop}
>
<wa-icon name=${ICON_NEW}></wa-icon>
New Playlist
</button>
<button
class="new-playlist-button"
@click=${this.handleNewSmartPlaylistClick}
>
<wa-icon name="filter"></wa-icon>
New Smart Playlist
</button>
</div>
</page-header>
${this.importError
@@ -1765,7 +1736,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
: entry.summary.IsSmart
? html`<wa-icon
class="playlist-icon"
name="filter"
name=${ICON_SMART_PLAYLIST}
></wa-icon>`
: nothing}
${isRenaming
@@ -64,6 +64,7 @@ import { list } from '@utils/binding';
import {
ICON_PLAYLIST,
ICON_QUEUE,
ICON_SMART_PLAYLIST,
} from '@utils/icon-language';
@@ -1214,7 +1215,7 @@ export class SmartPlaylistDetails
<wa-icon name="arrow-left"></wa-icon>
</button>
<div class="playlist-avatar">
<wa-icon name="filter"></wa-icon>
<wa-icon name=${ICON_SMART_PLAYLIST}></wa-icon>
</div>
<div class="playlist-info">
<h1
@@ -11,6 +11,7 @@ import {
import { SelectionController } from '@utils/selection-controller';
import type { SelectionHost } from '@utils/selection-controller';
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
import { PHONE_QUERY } from '@utils/breakpoints';
import {
ContextMenuController,
contextMenuStyles,
@@ -105,9 +106,6 @@ const ROW_CHROME_WIDTH =
const ROW_HEIGHT = 33;
const PHONE_ROW_HEIGHT = 52;
/** The shell's phone breakpoint, as `index.css` and every component
* stylesheet spells it. */
const PHONE_QUERY = '(max-width: 599px)';
// Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM
// overhead (30-50 shadow roots during scroll). Font Awesome 6 paths.
+1
View File
@@ -41,6 +41,7 @@ solid/compact-disc
solid/copy
solid/database
solid/download
solid/ellipsis
solid/file-import
solid/filter
solid/floppy-disk
+23
View File
@@ -0,0 +1,23 @@
/**
* The shell's breakpoints, where JavaScript has to agree with CSS.
*
* A media query inside a shadow root is answered by the viewport, so a
* component normally states what it drops at phone width in its own
* stylesheet and needs nothing from here. This exists for the cases
* where the decision is not a style: `track-list` computes its grid in
* JS from the host width, and `now-playing` renders *different content*
* on a phone a plain string instead of a link which no stylesheet
* can express.
*
* One breakpoint, several expressions of it. It was a private const in
* track-list.ts when there was one; a second reader is where a copy
* would start drifting from index.css.
*/
/**
* Phone width. 600px rather than the sidebar's 900px because 900 is a
* laptop: the answer there is a narrower sidebar, which is still a
* sidebar. Below this the shell drops the sidebar column entirely and
* bottom-nav takes over.
*/
export const PHONE_QUERY = '(max-width: 599px)';
+24
View File
@@ -54,6 +54,18 @@ export const ICON_PLAYLIST = 'list';
*/
export const ICON_NEW = 'plus';
/**
* A smart playlist the rule, and the thing the rule makes.
*
* Governed for the reason `ICON_AUTOTAG` states: it was already at
* three call sites (the Playlists header, the row marker beside a smart
* playlist's name, and `smart-playlist-details`'s avatar), and a name
* stops being a detail of one component the moment there are two. It is
* deliberately *not* `ICON_NEW`, even on the button that makes one:
* an icon names the noun it acts on, and the noun here is the rule.
*/
export const ICON_SMART_PLAYLIST = 'filter';
/**
* The request ("want") toggle, as an outline/solid pair.
*
@@ -100,6 +112,18 @@ export const ICON_AUTOTAG = 'tag';
*/
export const ICON_DOWNLOADING = 'download';
/**
* The rest of what this thing can do.
*
* `page-header` collapses the actions that do not fit into one menu
* behind this, so the glyph has to name *more of the same nouns* rather
* than any one of them which is what an ellipsis is and what `bars`
* (the navigation drawer, one component over in `bottom-nav`) is not.
* It is deliberately the only meaning it carries: an overflow menu that
* shared an icon with a destination would be the `list` problem again.
*/
export const ICON_MORE_ACTIONS = 'ellipsis';
/**
* Take this away.
*
+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);
@@ -0,0 +1,85 @@
/**
* A hover affordance is gated on the device having hover.
*
* The home page's cover cards reveal a play button on :hover. A touch
* long-press synthesises a hover state in the WebView, so on a phone
* that button flashed into view during the 500ms hold that
* utils/long-press.ts is measuring for a context menu a control
* appearing because the user was reaching for a different one.
*
* This is asserted against the *parsed stylesheet* rather than by
* emulating a touch device, and that is a limitation worth stating
* rather than hiding. CDP's Emulation.setEmulatedMedia does not reach
* this tier's iframe matchMedia still answers `hover: hover` after it
* is set so there is no way here to render the component as a phone
* would and read the computed style. What can be checked is the shape
* the browser actually built from the css`` literal: that the reveal
* lives inside a hover media query and that the default is display:none.
*
* Which is the regression worth catching anyway. The failure mode is
* someone hoisting the rule back out of the query for a one-line tidy
* a change nothing renders differently on a desktop, so every other
* assertion in this repo passes and the phone silently regresses.
*/
import { describe, expect, it } from 'vitest';
import '@components/home-view/home-view';
import { fixture } from '@test/support/render';
/** Every rule in the element's own adopted stylesheets, flattened. */
function rulesOf(host: Element): { text: string; condition: string | null }[] {
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
const out: { text: string; condition: string | null }[] = [];
for (const sheet of sheets) {
for (const rule of Array.from(sheet.cssRules)) {
if (rule instanceof CSSMediaRule) {
for (const inner of Array.from(rule.cssRules)) {
out.push({ text: inner.cssText, condition: rule.conditionText });
}
continue;
}
out.push({ text: rule.cssText, condition: null });
}
}
return out;
}
describe('the home card play button', () => {
it('reveals itself only where the device has hover', async () => {
const el = await fixture('home-view', {});
const rules = rulesOf(el);
// The sweep is worth nothing if it read no rules at all — the same
// first assertion icon-language.test.ts makes for the same reason.
expect(rules.length).toBeGreaterThan(0);
const reveals = rules.filter(
(r) => r.text.includes('.play') && /opacity:\s*1/.test(r.text),
);
expect(reveals.length).toBeGreaterThan(0);
for (const rule of reveals) {
expect(rule.condition).toMatch(/hover:\s*hover/);
expect(rule.condition).toMatch(/pointer:\s*fine/);
}
});
it('is display:none rather than transparent where it is absent', async () => {
const el = await fixture('home-view', {});
// opacity:0 alone would leave a button that still takes taps and is
// still in the accessibility tree, so a phone would keep the hit
// area for a control it can never see.
const unconditional = rulesOf(el).filter(
(r) => r.condition === null && r.text.startsWith('.play'),
);
expect(unconditional.length).toBeGreaterThan(0);
expect(unconditional.some((r) => /display:\s*none/.test(r.text))).toBe(true);
});
});
@@ -44,6 +44,8 @@ const GOVERNED = [
'regular/bookmark',
'bars-staggered',
'tag',
'filter',
'ellipsis',
];
/** The one file allowed to say them, plus its own test. */
@@ -0,0 +1,166 @@
/**
* The mini player's links are a desktop affordance.
*
* `utils/explore-link.ts` makes every track and artist name navigate,
* and `utils/queue-source-link.ts` makes "Playing from X" navigate in
* the bottom bar those are a few characters of text at a font size
* chosen for a bar, which is not a touch target. Worse, explore-link
* holds the navigation for one double-click interval and drops it if a
* second click arrives: a gesture that exists so double-clicking a row
* can play it, and which means nothing at all on touch.
*
* So below the shell's phone breakpoint the three render as plain text
* and the whole bar's cover art opens the full-screen Now Playing view,
* which is where the links live.
*
* The breakpoint is stubbed rather than emulated for the reason
* track-list-phone.test.ts states: this tier's viewport is fixed at
* 1280x800 by the runner, and the component reads matchMedia in
* connectedCallback precisely so a test can answer it first.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import '@components/now-playing/now-playing';
import { Events } from '../../src/events';
import { emit, flush } from '@test/support/harness';
import { fixture, shadow, shadowAll, text } from '@test/support/render';
import type { TrackInfo } from '@store/player-store';
import type { QueueTrack } from '@store/queue-store';
const TRACK: TrackInfo = {
fileName: 'ashes.mp3',
filePath: '/music/ashes.mp3',
trackLength: 215,
seekPosition: 0,
state: 'playing',
title: 'Ashes to Ashes',
artist: 'David Bowie',
album: 'Scary Monsters',
coverArt: '',
coverArtSmall: '',
coverArtMedium: '',
coverArtLarge: '',
trackChangeId: 1,
artistMbid: '',
releaseGroupMbid: '',
recordingMbid: '',
};
function queueTrack(n: number, title: string): QueueTrack {
return {
id: n,
audioFileId: n,
filePath: `/music/${n}.mp3`,
position: n,
title,
artist: 'David Bowie',
album: 'Scary Monsters',
coverArtPath: '',
artistMbid: '',
releaseGroupMbid: '',
recordingMbid: '',
};
}
/** Mount the bar with the phone breakpoint answering `matches`. */
async function mountAt(phone: boolean) {
const real = window.matchMedia.bind(window);
window.matchMedia = ((q: string) =>
q.includes('max-width: 599px')
? {
matches: phone,
media: q,
addEventListener() {},
removeEventListener() {},
}
: real(q)) as typeof window.matchMedia;
try {
const el = await fixture('now-playing');
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 });
emit(Events.QueueChanged, {
tracks: [queueTrack(1, 'Ashes to Ashes')],
currentIndex: 0,
source: { type: 'album', id: 7, label: 'Scary Monsters' },
});
await flush();
await el.updateComplete;
return el;
} finally {
window.matchMedia = real;
}
}
describe('the mini player on a phone', () => {
beforeEach(() => {
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 1 });
});
it('renders the title and artist as plain text', async () => {
const el = await mountAt(true);
expect(shadowAll(el, '.explore-link').length).toBe(0);
// The words are unchanged — this is about what they are, not about
// hiding them. A fix that dropped the text would pass an assertion
// about links alone.
expect(text(el, '[data-testid="now-playing-title"]')).toContain(
'Ashes to Ashes',
);
expect(text(el, '[data-testid="now-playing-artist"]')).toContain(
'David Bowie',
);
});
it('does not navigate from the source line', async () => {
const el = await mountAt(true);
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
expect(source?.classList.contains('navigable')).toBe(false);
let navigated = false;
el.addEventListener('navigate', () => {
navigated = true;
});
source?.click();
expect(navigated).toBe(false);
});
it('still says where the queue came from', async () => {
const el = await mountAt(true);
// Dropping the *link* is the change; dropping the information would
// be a different and worse one.
expect(text(el, '[data-testid="now-playing-source"]')).toBe(
'Playing from Scary Monsters',
);
});
it('leaves the desktop bar exactly as it was', async () => {
const el = await mountAt(false);
expect(shadowAll(el, '.explore-link').length).toBeGreaterThan(0);
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
expect(source?.classList.contains('navigable')).toBe(true);
let detail: unknown;
el.addEventListener('navigate', (e) => {
detail = (e as CustomEvent).detail;
});
source?.click();
expect(detail).toEqual({
view: 'explore-album-details',
localAlbumId: 7,
albumName: 'Scary Monsters',
});
});
});
+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.
+6 -11
View File
@@ -20,19 +20,14 @@ pre-commit:
glob: "*.go"
run: go tool golangci-lint run --timeout 5m ./...
# Snapshots the tree either side of the generators and reports only
# what moved across them. This used to be `go generate` plus a bare
# `git diff --name-only`, which is the *whole unstaged worktree* — so
# any unrelated edit sitting there was reported as stale generated
# code, and `make generate` then fixed nothing. See the script.
codegen-check:
glob: "*.{go,sql,templ}"
run: |
go generate ./...
if [ -n "$(git diff --name-only)" ]; then
echo "Generated code is out of date. Run 'make generate' and stage the changes."
# --no-pager, or this blocks forever on `less` waiting for a
# keypress that a hook run without a tty will never get: the
# commit hangs at exactly the moment it is trying to tell you
# why it failed.
git --no-pager diff --stat
exit 1
fi
run: ./scripts/codegen-check.sh
# frontend/bindings is generated by `wails3`, not `go generate`, so
# the check above does not cover it. ~3.5s warm, ~20s on a cold
+81
View File
@@ -0,0 +1,81 @@
#!/usr/bin/env bash
#
# Fails when `go generate ./...` would change something that is not staged.
#
# The obvious spelling of this is `go generate && git diff --name-only`,
# which is what the hook used to be, and it answers the wrong question:
# that diff is the *whole unstaged worktree*, so any unrelated edit — a
# note, a plan document, the next commit's files sitting there while this
# one lands — was reported as
#
# Generated code is out of date. Run 'make generate' and stage the changes.
#
# Running `make generate` then does nothing, because nothing generated is
# stale, and the message sends you looking for a codegen problem that does
# not exist. Splitting one piece of work into several commits is exactly
# the shape that triggers it, so the workaround was a constraint on commit
# order for no real reason.
#
# So the tree is snapshotted either side of the generators and only what
# *moved across them* is reported. That is deliberately not a list of
# generated paths: sqlcgen, `*_templ.go` and `frontend/src/events.ts` are
# today's answer, a fourth generator is one `//go:generate` line away, and
# a path list is a second place to remember it — the same reasoning that
# keeps staleshape.go parsing sql/schemas/ rather than restating it.
#
# Content, not names: a generated file that is *already* dirty and is then
# rewritten further keeps its name in both snapshots and would otherwise
# slip through.
set -euo pipefail
cd "$(dirname "$0")/.."
# name + worktree blob hash for every file that differs from the index.
# A file listed but absent (a deletion) hashes as "gone" rather than
# aborting the pipeline.
snapshot() {
git diff --name-only | while IFS= read -r f; do
if [ -f "$f" ]; then
printf '%s %s\n' "$f" "$(git hash-object -- "$f")"
else
printf '%s gone\n' "$f"
fi
done
}
# A brand-new generated file is not in either diff, because it is not
# tracked at all — the same blind spot bindings-check.sh names. Both
# snapshots are taken before the generators run.
before="$(snapshot)"
before_untracked="$(git ls-files --others --exclude-standard)"
go generate ./...
after="$(snapshot)"
after_untracked="$(git ls-files --others --exclude-standard)"
# Symmetric difference, and the symmetry is the whole point. Generation
# can push a file *into* the unstaged set (it was current, now it is not)
# or *out* of it (someone hand-edited generated output and the generator
# put it back) — and the second is stale generated code just as much as
# the first. Comparing one direction only reports "current" for it,
# which is the failure this script was written to stop.
moved="$(comm -3 <(printf '%s\n' "$before" | sort) <(printf '%s\n' "$after" | sort) |
cut -d' ' -f1 | tr -d '\t' | sort -u | grep -v '^$' || true)"
if [ -n "$moved" ]; then
echo "codegen-check: generated code is out of date." >&2
echo "Run 'make generate' and stage:" >&2
printf ' %s\n' $moved >&2
exit 1
fi
if [ "$after_untracked" != "$before_untracked" ]; then
echo "codegen-check: generation produced new files. Stage them:" >&2
comm -13 <(printf '%s\n' "$before_untracked" | sort) \
<(printf '%s\n' "$after_untracked" | sort) >&2
exit 1
fi
echo "codegen-check: generated code is current"
+63
View File
@@ -97,6 +97,55 @@ if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then
fi
rm -f "$PID_FILE"
# ── Refuse to inherit somebody else's port ───────────────────────────
# The PID check above only knows about *this* worktree: `make dev-stop`
# kills the pid in this .dev/app.pid and nothing else. Several worktrees
# of this repo share the default port, so an app orphaned by a deleted
# worktree goes on listening with nothing left to stop it.
#
# Without this check the new app starts, fails to bind, exits — and every
# curl and playwright-cli call afterwards goes to the *other* process, so
# the harness reports facts about an app nobody asked for. That is not a
# quiet wrongness either: it presented as
# "no such table: libraries" against a freshly created YJ_HOME, which
# reads exactly like applySchema or staleshape.go having gone wrong and
# is a frightening place to start looking.
#
# The startup wait below cannot catch it, because the health check is
# satisfied by *any* app on the port — which is precisely the failure.
# So it is refused here, before anything is launched, rather than warned
# about. --port already exists for the legitimate second-app case.
port_holder() {
command -v ss >/dev/null || return 0
ss -lptn "sport = :$PORT" 2>/dev/null | grep -oP 'pid=\K[0-9]+' | head -n 1
}
if curl -sf -o /dev/null --max-time 2 "http://localhost:$PORT/" ||
[ -n "$(port_holder)" ]; then
holder="$(port_holder)"
echo "dev-headless: :$PORT is already in use; refusing to start" >&2
if [ -n "$holder" ]; then
# /proc/<pid>/cwd names the checkout it belongs to, and says
# "(deleted)" for the orphaned-worktree case that is the whole
# reason this is worth a check.
cwd="$(readlink "/proc/$holder/cwd" 2>/dev/null || echo unknown)"
cmd="$(tr '\0' ' ' <"/proc/$holder/cmdline" 2>/dev/null || echo unknown)"
echo " pid $holder ($cmd)" >&2
echo " cwd $cwd" >&2
# The PID-file check above has already passed, so whatever this
# is, `make dev-stop` does not know about it — saying otherwise
# sends you to a command that will report success and change
# nothing. Never `pkill -f` here either: the pattern would
# match this script's own command line.
echo " 'make dev-stop' will not touch it (it is not in" >&2
echo " ${PID_FILE#"$REPO_ROOT"/}): kill $holder, or pass --port." >&2
else
echo " The holder could not be identified (no ss, or it belongs" >&2
echo " to another user). Try: ss -lptn 'sport = :$PORT'" >&2
fi
exit 1
fi
# ── Choose the YJ_HOME ───────────────────────────────────────────────
# A seed is a YJ_HOME that a previous run of the app produced, tarred
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
@@ -200,6 +249,20 @@ until curl -sf -o /dev/null "http://localhost:$PORT/"; do
sleep 0.25
done
# The loop above exits on the first answer from the port, and "something
# answered" is not "the app we started answered". The pre-launch guard
# makes that unlikely rather than impossible — a race, or a listener
# started in between — and the check is one signal, so it is worth making
# here too. An empty log beside a dead pid is the "it exited immediately
# and nothing said so" case that the original report spent its time on.
if ! kill -0 "$APP_PID" 2>/dev/null; then
echo "dev-headless: :$PORT answered, but the app we started (pid" >&2
echo " $APP_PID) is gone — something else holds the port." >&2
tail -n 30 "$LOG_FILE" >&2
rm -f "$PID_FILE"
exit 1
fi
cat <<EOF
dev-headless: up
url http://localhost:$PORT
+27 -3
View File
@@ -38,8 +38,10 @@
# Where a body is taken and no --body-file is given, it is read from stdin.
#
# Environment:
# GITEA_TOKEN a PAT with write:issue (plus write:repository and read:user,
# which the rest of this repo's tooling reaches for)
# GITEA_TOKEN a PAT with write:issue. `claim` and `mine` additionally
# need to know your username: set GITEA_USER, or give the
# token read:user and it is looked up.
# GITEA_USER your Gitea login. Optional; see above.
# GITEA_URL defaults to https://git.ljones.me
# GITEA_REPO defaults to yonlu/yellowjacket
set -euo pipefail
@@ -81,7 +83,29 @@ read_body() {
if [ "$file" = "-" ]; then cat; else cat "$file"; fi
}
me() { curl -sS -H "Authorization: token $GITEA_TOKEN" "$server/api/v1/user" | python3 "$py" login; }
# The one lookup in this script that needs a scope beyond write:issue.
# `GET /user` requires read:user, and it is reached for exactly two reasons:
# to name the assignee in `claim`, and to filter in `mine`. A token scoped to
# the work this script does — write:issue — therefore failed at `claim`, which
# is the one step the workflow requires before the first edit, so the whole
# documented process was blocked by its own tooling.
#
# GITEA_USER short-circuits it, which is what lets a least-privilege token do
# the job. The lookup stays as the fallback because it is right when the
# scope is there and needs no setup at all.
me() {
if [ -n "${GITEA_USER:-}" ]; then
printf '%s' "$GITEA_USER"
return
fi
curl -sS -H "Authorization: token $GITEA_TOKEN" "$server/api/v1/user" |
python3 "$py" login ||
{
echo "issue.sh: could not resolve your username. Set GITEA_USER, or" >&2
echo "issue.sh: re-issue GITEA_TOKEN with read:user." >&2
exit 1
}
}
label_id() { call GET "/labels?limit=100" | python3 "$py" label-id "$1"; }