Compare commits

..
Author SHA1 Message Date
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 23f3d4b3b0 fix(explore): clear in_library on a row that has no local id
CI / check (push) Skipped
CI / e2e (push) Skipped
`in_library = 1 AND local_*_id IS NULL` was a fixed point.
upsertBatch's conflict clause is `MAX(in_library, excluded.in_library)`,
so it can only ever raise the flag, and pruneStaleLocalCrossReferences —
which its own comment calls the only place a removal from the library is
reflected back into the index — was gated on the id being present. So
nothing in the app could clear such a row, ever: a permanent claim of
ownership with no local row to check it against.

The gate is now the flag *or* the id, for all three entity types. A NULL
id fails the existence test on its own, so this needs no second clause to
say what "not owned" means.

Nothing in the tree writes that shape today — collectLibraryEntities sets
both together — which is why this is worth closing rather than leaving:
the exposure is a database written by a version whose local-id columns
were populated differently, and the next writer that sets the flag
without an id, which nothing structurally prevents and which this shape
made permanent rather than merely wrong until the next scan.

The test seeds the row with raw SQL on purpose. upsertBatch writes a zero
LocalArtistID as literal 0, and 0 satisfies `IS NOT NULL`, so the old
gate already caught that shape — a fixture built through the upsert
cannot reproduce this at all. NULL is what the artifact importer and any
older writer leave behind, the columns being nullable with no default.
Reverted against the old gate, it fails on all three types.

Closes #118
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
logan bb7dde1963 Merge pull request 'A CI-only change is ci:, not fix(ci):' (#112) from docs/ci-commit-type into main
CI / check (push) Successful in 2m26s
CI / e2e (push) Successful in 6m31s
2026-08-19 16:23:20 +00:00
yonlu 446380e3a9 docs: a CI-only change is ci:, not fix(ci):
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / e2e (pull_request) Successful in 6m41s
CI / check (pull_request) Successful in 2m28s
The commit-analyzer reads the type and ignores the scope, so `fix` is a
patch whatever sits in the brackets. Two commits touching nothing but
.gitea/workflows/unclaim.yml were written `fix(ci):` and cut v0.2.1 and
v0.2.2 -- real releases, published to Arch, Homebrew and the APK
registry, containing no user-facing change.

CLAUDE.md already warned that a mistyped feat ships a minor version.
That was not enough, because this was not a mistyped type: `fix` was
chosen deliberately, in the belief that the (ci) scope qualified it.

The version bump is the small half, which is why this gets a paragraph
rather than a clause. A merge to main starts two workflows; if
release.yml then pushes a tag, that tag push starts four more --
arch-package, homebrew-formula, android-apk and desktop-assets -- on a
runner with capacity 1, where the APK build alone is tens of minutes
and publishes a signed artifact to a public registry. So a mistyped
type is six workflow runs, not an odd-looking changelog.

`make release-dry` answers this before the merge instead of after, and
is cheaper than any one of those runs.

The two releases are staying: they are already published, and a version
that vanishes is worse for whoever pulled it than one that turns out to
be empty.

Closes #111
2026-08-19 16:03:38 +00:00
logan e07f248cc8 Merge pull request 'Wait for the scroll range the assertion needs' (#134) from fix/133-album-dropdown-scroll-race into main
CI / e2e (push) Successful in 6m39s
CI / check (push) Successful in 2m33s
2026-08-19 16:03:17 +00:00
logan 90ac6e0825 test(e2e): wait for the scroll range the assertion needs
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m33s
CI / e2e (pull_request) Successful in 6m22s
The guard polled for `scrollHeight > clientHeight + 40` and the next
line asserted the container could be scrolled to 80, so any range in
41-79 satisfied the precondition and could not satisfy the assertion.
The grid passes through exactly that while it settles, because it
recomputes its columns after a viewport change rather than during it,
so the test read a clamped scrollTop and reported 10 against 80.

It failed CI on a pull request that changes one paragraph of CLAUDE.md
and nothing else, while WebKit passed in the same run. Reproduced
locally: 0 failures in 6 runs before #132, 2 in 9 after, 0 in 10 with
this change.

#132 is what made it reachable rather than what broke it. The queue
panel's mode is measured rather than media-queried, so a viewport
change at this width costs one more layout pass, and cover-grid settles
after it instead of before. The settled range is 330 and stable, the
main panel is 700px, and the panel is correctly display:none while
closed — there is no user-visible defect, only a wider window for a
race the spec already had.

A threshold below the value its caller depends on is not a guard, so
the target is one constant that both the guard and the assertion read.

Closes #133
2026-08-19 11:50:46 -04:00
logan 4e3c953acf Merge pull request 'Decide the supported sizes, and stop the queue taking the page's width' (#132) from feat/24-supported-sizes-queue-model into main
CI / check (push) Successful in 2m30s
CI / e2e (push) Successful in 6m20s
2026-08-19 15:22:39 +00:00
logan ede183d026 test(shell): check 900x600, which is narrower than the minimum
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m34s
CI / e2e (pull_request) Successful in 6m42s
The sidebar collapses to icons *below* 900, so the main panel is 843px
at 899 and 700px at 900: the narrowest content area any desktop width
produces is at the top of the Compact band, not at the enforced floor.
A viewport list that stopped at "the minimum" was missing its own worst
case.

MinWidth's comment loses both reasons it used to give, because neither
mechanism can happen any more — the subtitle is display:none from 899
down, and the sidebar host is overflow-y:auto (at 600x460 its
scrollHeight is 434 against a 332px client, and Settings is reachable
after scrolling). The value does not change: 800x600 is where desktop
chrome stops being comfortable, not where the app breaks, and below
600 the phone layout takes over. A floor defended by two expired
mechanisms is a number nobody can argue with, which is worse than
either answer.

Closes #24
2026-08-19 10:54:52 -04:00
logan 481c9dca65 docs: record the size bands and what the queue model cost to find
CLAUDE.md gains the three bands as a promise (Phone <600, Compact
600-899, Desktop >=900, and "no action is ever unreachable at any
supported size"), the computed queue rule and why it cannot be a media
query, and the correction that 900 — not the 800x600 minimum — is the
worst desktop width.

NOTES.md gets the measurements, including two things worth more than
the fix. My first probe for the sidebar's scroller searched
shadowRoot.querySelectorAll('*') and reported "no scroller, items are
unreachable", which reads exactly like a live Settings-unreachable bug;
the scroller is the host, and a host is not inside its own shadow root.
And the plan's first draft claimed the overlay "removes the desktop
half of #69", which the screenshot disproved: open and closed are now
identical at 900x600, so the queue's contribution is gone, but the
header's own overflow remains and is still a live defect.

Refs #24
2026-08-19 10:54:52 -04:00
logan 4025106234 fix(queue): overlay the content instead of taking its width
The panel is flex-shrink: 0 in the flow of .content-area, so an open
queue was paid for by the main panel rather than covering it. Measured
on Playlists: 379px of content left at 900x600 with all three of the
page header's actions clipped, 69px at 390px, and 0px at 320px — where
the content was not degraded but gone.

It goes to an overlay with a scrim when the content cannot spare the
width, and the rule is computed rather than breakpointed:
`available - panelWidth < 480`, where available is .content-area's
width and so already accounts for the sidebar's collapse at 900. A
media query cannot express this, which is the reason for the property:
the panel is drag-resizable between 200 and 500px and persisted, so a
viewport breakpoint silently assumes the default 320 and is wrong by up
to 180px for a user who widened it — in the direction that hurts, since
a wider queue is exactly when the content can least afford it.

480 is a judgement and the comment says so: there is no cliff to derive
it from (the track list rescales continuously, 213px to 124px columns
with no row overflow), so it is anchored to keep the default 1100px
window inline while putting every measured-broken case on the overlay
side.

The overlay is a presentation and not a fork — #55 asks for one
component with two mount points — so the roving tab stop, Alt+Arrow
reorder, drag reorder and selection semantics are untouched. Escape
closes it and returns focus, attached only while the overlay is up: it
is a dismissal rather than a shortcut, which is why it is not a
panel-scoped binding. The scrim covers the content area only, not the
sidebar or the transport, because the queue is not modal.

Refs #24
2026-08-19 10:54:52 -04:00
logan a3134f997f docs(planning): decide the supported sizes and the queue panel's model
#24 asks for a design pass, and #73 hangs the rest of Phase 2 off the
answer, so the decision is written down before any CSS moves.

Measured against the running app, and five things are not in the issue:
the Playlists header clips at 800x600 with the queue *closed* — the
minimum window is the only size this app promises; 900x600 is worse
than 800x600, because the sidebar expands at 900, so the worst desktop
case is not the minimum and every test that stops at the minimum misses
it; at 320px with the queue open the main panel is 0px wide, because
the panel is in the flow rather than over it; only Playlists overflows,
so #69 is one view's action set and not a systemic header failure; and
both reasons in MinWidth's comment describe mechanisms that no longer
exist.

The queue's mode cannot be a media query: its width is drag-resizable
between 200 and 500px and persisted, so a fixed breakpoint assumes the
default 320 and is wrong by 180px in the direction that hurts. It is
computed from the measured widths instead.

#69 stays its own PR on a finding rather than an estimate: page-header
cannot collapse actions that arrive as arbitrary light-DOM markup
through a slot, so the fix needs an actions API across all three hosts.

A very small window becomes the phone layout, which already exists and
is already tested, rather than the mini-player: #12 is a second
always-on-top window, and making it a mode of the main window would
discard navigation state on a resize and put the process-level MPRIS
question on a path a drag can trigger.
2026-08-19 10:54:52 -04:00
logan 3607fe445e Merge pull request 'Fix the player states that report one track's progress against another' (#129) from fix/player-playing-state into main
CI / check (push) Successful in 2m30s
CI / e2e (push) Successful in 6m14s
2026-08-19 14:53:46 +00:00
22 changed files with 1796 additions and 59 deletions
+116
View File
@@ -3580,3 +3580,119 @@ per-card flag can answer at all.
The general point: **two columns that agree today are not one column.**
Which of them a new surface reads should be decided by which one has
something that can un-set it.
## The queue panel was a column that could not afford to be one (measured 2026-08-19)
Plan 018, issue #24. Measured against the running app (`make
dev-headless SEED=default`, Chromium) on Playlists, sweeping the
viewport with the queue open and closed. Main panel width, and how much
of the page header survived:
| viewport | sidebar | main (queue open) | actions clipped |
|---|---|---|---|
| 1280×800 | 200 | 759 | — |
| 1000×700 | 200 | 479 | 2 of 3 |
| **900×600** | 200 | **379** | all three |
| 800×600 | 56 | 423 | all three |
| 390×780 | — | **69** | all three |
| 320×600 | — | **0** | all three |
| 800×600 | 56 | 744 *(closed)* | New Smart Playlist, 158/162px |
Five things came out of it that the issue did not say.
- **The header clips at the enforced minimum with the queue closed.**
800×600 is the only size this app promises, and "New Smart Playlist"
loses 4px of its 162 there. The queue makes it dramatic; it is not
the cause.
- **900×600 is worse than 800×600.** `AUTO_COLLAPSE_VIEWPORT` collapses
the sidebar *below* 900, so the main panel is 843px at 899 and 700px
at 900. **The worst desktop case is the top of the Compact band, not
the enforced floor** — so every viewport list that stopped at "the
minimum" was missing its own worst case. `layout-overflow.spec.ts`
carries 900 now.
- **At 320px the main panel was 0px.** The panel is `flex-shrink: 0` in
the flow of `.content-area`, so an open queue is paid for by the
content rather than covering it. Not degraded — gone. That is the
measurement #55 wanted and did not have.
- **Only Playlists overflows.** All ten primary views swept at 900×600
and 390×780; every other header reports `scrollWidth ==
clientWidth`, and Albums at 390 renders title, count and sort legibly
(checked on a screenshot, not just the number). So #69 is one view's
action set — three text buttons totalling 390px — and not a systemic
header failure.
- **Both reasons in `MinWidth`'s comment had expired.** The subtitle is
`display: none` from 899 down, and the sidebar host is
`overflow-y: auto` (at 600×460, `scrollHeight` 434 against a 332px
client, Settings reachable after scrolling). The floor is right; its
stated defence was two mechanisms that can no longer happen, which is
worse than either answer because nobody can argue with it.
**A correction worth keeping, because it nearly went in the plan.** My
first probe for the sidebar's scroller searched
`shadowRoot.querySelectorAll('*')` and reported "no scroller — items
are unreachable", which reads exactly like a live Settings-unreachable
bug. The scroller is the **host**, and a host is not inside its own
shadow root. CLAUDE.md was right and the probe was wrong.
**And one claim in the plan's first draft was too strong**: that the
overlay "removes the desktop half of #69". After phase 2, at 900×600,
open and closed are now *identical* (main 700, one action clipped)
where open used to be main 379 with all three clipped. The queue's
contribution is gone; the header's own overflow remains and is still a
live defect at a supported size.
### The mode cannot be a media query
The panel is drag-resizable 200500px and persisted, so a viewport
breakpoint assumes the default 320 and is wrong by up to 180px for a
user who widened it — in the direction that hurts, since a wider queue
is exactly when the content can least afford it. It is computed from
`.content-area`'s width instead (which already accounts for the
sidebar's collapse), and the component test that matters widens the
panel at a *fixed* parent width and asserts the flip.
The floor (480) is a judgement, and the measurement is why: there is no
cliff. The track list rescales its columns continuously — 213px down to
124px between main widths of 900 and 544, `rowOverflow=0` at every step
— and the album grid steps 3 columns to 2 somewhere between 564 and 644
without breaking. So 480 is anchored at both ends instead: it keeps the
default 1100px window inline, and puts every measured-broken case on
the overlay side.
The scrim is perceptible but subtle on a dark ramp, which is worth
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.
@@ -0,0 +1,294 @@
# 018 — Supported sizes, and what the queue panel is
**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
#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
moves. This document is the decision. Everything below the matrix is
either a measurement or an argument for one of the four choices #24
asks for.
---
## What is actually wrong, measured
Against the running app (`make dev-headless SEED=default`, Chromium),
Playlists, sweeping the viewport with the queue open and closed. The
number that matters is how much of the page header survives.
| viewport | sidebar | queue | main panel | header needs | actions clipped |
|---|---|---|---|---|---|
| 1280×800 | 200 | open 321 | 759 | 759 | — |
| 1000×700 | 200 | open 321 | 479 | 747 | New Playlist, New Smart Playlist |
| **900×600** | 200 | open 321 | **379** | 747 | **all three** |
| 800×600 | 56 | open 321 | 423 | 747 | all three |
| 700×600 | 56 | open 321 | 323 | 747 | all three |
| 390×780 | — | open 321 | **69** | 747 | all three |
| 320×600 | — | open 321 | **0** | 747 | all three |
| 900×600 | 200 | closed | 700 | 747 | New Smart Playlist |
| **800×600** | 56 | closed | 744 | 747 | **New Smart Playlist (158/162px)** |
| 320×600 | — | closed | 320 | 747 | all three |
Five things in that table are not in the issue.
**The header clips at the supported minimum with the queue closed.**
At 800×600 — the size `backend/config/window.go` enforces and the only
size this app *promises* — "New Smart Playlist" loses 4px of its 162.
#24 reads as a queue-panel bug; the queue makes it dramatic, but the
header overflows on its own at the minimum window.
**900×600 is worse than 800×600, because the sidebar expands at 900.**
`AUTO_COLLAPSE_VIEWPORT` collapses the sidebar to icons *below* 900, so
at 899px the main panel is 843px and at 900px it is 700px. The worst
desktop case is therefore not the minimum window; it is the pixel
immediately above the collapse. Anything that tests "the minimum" and
stops has not tested the worst case, which is what
`layout-overflow.spec.ts` does today.
**At phone widths the queue is not a drawer, it is an amputation.**
`queue-panel`'s host is `flex-shrink: 0; width: 0`, going to
`width: var(--queue-width, 320px)` under `[open]` — it is *in the flow*
of `.content-area`, so it takes its width from the main panel rather
than covering it. At 390px that leaves 69px of the page; at 320px it
leaves **0px**, and the app is not degraded but gone. This is the
measurement #55 needs and did not have.
**Only Playlists overflows.** Sweeping all ten primary views at 900×600
and at 390×780, every other header reports `scrollWidth ==
clientWidth`, and Albums at 390px renders title, count and sort
legibly (checked on a screenshot, not just the number). #69 is
therefore one view's action set — three text buttons totalling 390px —
and not a systemic header failure, though the *rule* still belongs in
`page-header`.
**Both reasons in `MinWidth`'s comment are stale.** It says the floor is
800×600 because "below ~780 the header's subtitle wraps" and "below
~600 tall the eleven sidebar items no longer fit". The subtitle is
`display: none` below 900 (index.css), and the sidebar host is
`overflow-y: auto` — at 600×460 its `scrollHeight` is 434 against a
332px client, and Settings is reachable after scrolling. Neither
mechanism can happen any more. That does not mean the floor should
move; it means its stated reason no longer supports it, which is worse
than either answer.
*(Care needed: my first probe for the sidebar scroller searched
`shadowRoot.querySelectorAll('*')` and reported "items are
unreachable", because the scroller is the **host** and a host is not in
its own shadow root. The claim in CLAUDE.md is correct.)*
---
## Decision 1 — the supported size matrix
Three bands. Two of them already exist and are already argued; what is
new is that they are written down as a *promise*, and that the queue is
part of it.
| band | width | navigation | queue | promise |
|---|---|---|---|---|
| **Phone** | < 600 | `bottom-nav` + drawer | overlay, full width | reflows; nothing needs sideways scrolling; fits 320px |
| **Compact** | 600 899 | icon sidebar | overlay + scrim | nothing is clipped or unreachable at any width in the band |
| **Desktop** | ≥ 900 | labelled sidebar | inline where it fits (see decision 2), else overlay | as Compact |
And one promise across all three: **no action is ever unreachable.**
That is the sentence #69 asks for and it is the one the matrix exists
to make checkable.
**400% zoom** keeps the meaning it already has: WCAG 1.4.10 names 320px
as the reflow target, the phone band covers it, and
`layout-overflow.spec.ts` already asserts a 320px viewport needs no
sideways scrolling. What changes is that the *queue* must be part of
that assertion — it is not today, and with the queue open at 320px the
main panel is 0px wide, which no current test can see.
**The window minimum stays 800×600**, and its comment gets the real
reason. The old mechanisms are gone, but the floor is still where the
Compact band's chrome stops being comfortable, and lowering it would
mean promising the desktop layout at sizes where only the phone layout
works. The interesting consequence is decision 4.
---
## Decision 2 — the queue is an overlay when it cannot afford to be a column
**The rule.** The queue panel renders inline — in the flow, as today —
only while
```
viewport sidebar queueWidth ≥ 480
```
and as an overlay with a scrim otherwise.
**Why it cannot be a media query**, which is the load-bearing half:
the queue's width is *user state*. It is drag-resizable between 200 and
500px and persisted (`--queue-width`, `MIN_WIDTH`/`MAX_WIDTH` in
`queue-panel.ts`). A breakpoint at a fixed viewport width silently
assumes the default 320, and is wrong by 180px for a user who has
dragged the panel wide — in the direction that hurts, since a wider
queue is exactly when the content can least afford it. So the mode is
computed from the measured widths and published as an attribute, the
way `data-active-view` already is, and the CSS keys off that.
**Why 480, honestly.** There is no cliff to derive it from. The track
list rescales its columns continuously — at main widths from 900 down
to 544 its `--grid-cols` shrink from 213px to 124px with
`rowOverflow=0` throughout — and the album grid steps 3 columns to 2
somewhere between 564 and 644 without breaking. So this is a judgement,
anchored on two things: it keeps the *default* window (1100 wide, main
= 580) inline, because the inline queue is a desktop affordance people
choose and turning it into an overlay for the common case would be a
regression in feel; and it puts every case measured as broken —
900×600 at main=379, and every phone width — on the overlay side.
1024×768 lands at main=504 and stays inline.
**The scrim is the other half of the issue's complaint** ("make the
queue obviously an overlay *over* the content so it reads as something
to close"). An overlay queue gets a scrim, closes on scrim click and on
Escape, and returns focus to `#queue-button`.
**What must not change**: #55's Direction is explicit — one component,
two mount points, do not fork it. The overlay is a *presentation* of
the same `queue-panel`, so the roving tab stop, Alt+Arrow reorder, drag
reorder, selection semantics and the `virtualizer.requestUpdate()` on
selection and current-track change all come along untouched. This
decision deliberately stops short of #55's detail-view mount, but it is
the shape that makes it possible, and it unblocks it.
---
## Decision 3 — #69 is its own PR, and here is the finding that decides it
`page-header` **cannot collapse its own actions**, and that is not an
effort estimate but a fact about the API. Actions arrive through
`<slot name="actions">` as arbitrary light-DOM markup — Playlists slots
a `<div class="header-actions">` of three `<button>`s with click
handlers, drag handlers and a conditional class. A component cannot
move another component's light-DOM children into a dropdown and keep
their behaviour; there is nothing generic to render as a menu item.
So the overflow rule needs an *actions API* — hosts declaring
`{icon, label, handler, priority}` data that `page-header` can render
either as buttons or as menu items — which is a change to all three
hosts that slot actions, not a rule added in one place. That is a
different piece of work from this one, it is independently verifiable,
and the desktop half of #69's symptom is removed by decision 2 anyway
(the queue stops eating the header's width).
It therefore stays #69, gets the finding above recorded on it, and
follows immediately after this. What *this* plan owes it is the
promise in the matrix — no action unreachable at any supported size —
and the measurement that the only offender today is Playlists.
**And the promise is not kept yet, which is the honest version of a
claim this document made in its first draft.** "Decision 2 removes the
desktop half of #69's symptom" was too strong. Measured after phase 2,
at 900×600 on Playlists:
| | before | after |
|---|---|---|
| queue open | main 379px, **all three** actions clipped | main 700px, **one** clipped |
| queue closed | main 700px, one clipped | unchanged |
So the queue's *contribution* is gone — open and closed are now
identical, which is the whole of what this decision owed — and the
residual "New Smart Playlist: 114/162px" is the header overflowing on
its own, at a size the queue never touched. #69 is still a live defect
at a supported size, and the matrix's promise is what will close it.
---
## Decision 4 — a very small window becomes the phone layout, not the mini-player
#24 asks whether a very small window should switch to the mini-player
(#12) "or simply refuse to go there". Both options in the question are
worse than the one the codebase already has.
**#12 is a second window, not a mode.** Its findings say so: v3
supports multiple windows, `AlwaysOnTop` is a window *option*, and the
frontend would need an entry branch mounting only the mini-player root
for a second window loading the same bundle. Turning the main window
into a mini-player at some width conflates the two: it would throw away
the user's navigation state on a resize, and it puts the MPRIS question
(#12's own open question — media controls are process-level and must
not be per-window) on a code path that a drag can trigger by accident.
**And "refuses" is unnecessary, because the reflow already exists.**
The phone band is real, tested, and reached by width alone — a desktop
window narrowed below 600px already gets `bottom-nav` and the phone
shell. That is a better answer than refusing: it is strictly more
usable than a hard minimum, it costs nothing new, and it is the same
code Android runs, so it stays exercised.
So: the main window reflows and never becomes a mini-player; #12 stays
a separate always-on-top window and is not blocked by, or coupled to,
this decision. The window minimum stays 800×600 for the reason in
decision 1 — but the phone band is what happens below it, not a
refusal, which is why the minimum is a comfort floor rather than a
correctness one.
---
## Phases
1. **This document**, linked from #24, with the matrix reported on the
issue and #55 told whether it is unblocked. *(no code)***done**
2. **The queue's overlay mode** — computed mode attribute, scrim,
Escape and scrim-click close, focus return. The inline path is
unchanged above the threshold. — **done**
3. **The window minimum's comment** — replace both stale reasons with
the measured ones. No value change. — **done**
4. **Verification**, below. Including the specs that must change
because they assert the old behaviour. — **done**
#69 follows as its own branch; #55 became unblocked at phase 2.
## What landed, measured
Main panel width with the queue open, before and after:
| viewport | before | after | mode |
|---|---|---|---|
| 1280×800 | 759 | 759 | inline |
| 1100×720 (default window) | 579 | 579 | inline |
| 1024×768 | 503 | 503 | inline |
| 900×600 | **379** | **700** | overlay |
| 800×600 | 423 | 744 | overlay |
| 390×780 | **69** | **390** | overlay |
| 320×600 | **0** | **320** | overlay |
The scrim is perceptible but subtle on a dark ramp, which is worth
knowing before someone "fixes" it: sampled from the 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 — on purpose: the queue is not modal, and
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.
## Verification, and what each tier cannot see
- `make ui-test` — the queue panel's mode logic is component-tier
work and belongs there. It **cannot** see the shell: the threshold is
computed from the sidebar and viewport, which do not exist in that
tier.
- `make e2e``layout-overflow.spec.ts` gains the queue-open case at
every band (it has none today, which is why main=0px at 320px has
never failed anything) and **gains 900×600**, since the minimum is
not the worst case. `queue-toggle-state.spec.ts` and
`phone-shell.spec.ts` both touch the panel and must be re-read before
editing.
- **Screenshots at every band, read by a human.** This is not optional
here: `layout-overflow.spec.ts` asserts the *shell* needs no sideways
scrolling and passes on a build whose album header clips its own
buttons (measured this session at 390px; filed on #66). Clipping
*inside* a component is invisible to it, and clipping is this issue.
- `make ui-visual` **cannot help at all** — the component tier renders
the token fallbacks, because the theme only reaches `:root` in the
real app.
- Accessible names via `page.getByRole(...)`, never a shadow-root
query. A drawer with a scrim is exactly the shape that grows a
nameless control, and this repo has shipped one three times.
+79
View File
@@ -1328,6 +1328,68 @@ is 32px each. Which four is plan 016's committed subset, and everything
else — Settings included, because a phone still needs it — is behind
"More".
**There are three supported size bands, and the queue is part of the
promise.** Plan 018 (#24) wrote them down: **Phone** below 600 (bottom
nav, reflows, fits 320px exactly), **Compact** 600899 (icon sidebar),
**Desktop** from 900 (labelled sidebar) — plus one sentence across all
three, *no action is ever unreachable at any supported size*. The bands
themselves already existed; what was new is that they are a promise and
that the queue panel is inside it.
**900 is the worst desktop width, not the 800×600 minimum.** The
sidebar collapses to icons *below* 900, so the main panel is 843px at
899 and 700px at 900 — the narrowest content area any desktop width
produces is at the top of the Compact band, not at the enforced floor.
Every viewport list that stopped at "the minimum" was therefore missing
its own worst case, which is why `layout-overflow.spec.ts` carries 900
now. And **both reasons in `MinWidth`'s comment had expired** — the
subtitle is `display: none` from 899 down and the sidebar host scrolls
(`overflow-y: auto`; at 600×460 its `scrollHeight` is 434 against a
332px client) — so 800×600 is a *comfort* floor for desktop chrome and
not a correctness one. Below it the phone layout takes over, which is
also why a very small window reflows rather than becoming a
mini-player: **#12 is a second always-on-top window, not a mode of this
one**, and making it a mode would discard navigation state on a resize
and put the process-level MPRIS question on a path a drag can trigger.
**The queue panel is a column only while the content can spare the
width, and that cannot be a media query.** In flow the host is
`flex-shrink: 0`, so an open queue is paid for by the main panel: it
left 379px at 900×600 (with all three of the Playlists header's actions
clipped), 69px at 390, and **0px** at 320 — the content was not
degraded but gone. It goes to an overlay with a scrim when
`available - panelWidth < 480`, where `available` is
`.content-area`'s width and therefore already accounts for the
sidebar's collapse.
Four things about it are load-bearing. **The mode is computed, not
breakpointed**, because the panel's width is user state — drag-resizable
200500px and persisted — so a viewport breakpoint silently assumes the
default 320 and is wrong by up to 180px in the direction that hurts;
widening the panel at a fixed window size must flip it, and
`queue-overlay-mode.test.ts` is written around exactly that. **480 is a
judgement and says so**: there is no cliff to derive it from (the track
list rescales continuously, 213px to 124px columns with no row
overflow), so it is anchored to keep the default 1100px window inline
and put every measured-broken case on the overlay side. **The scrim
covers the content area only** — not the sidebar or the transport —
because the queue is not modal, and it is subtle on a dark ramp by
arithmetic rather than by accident (33,37,41 → 18,20,23). And **the
overlay is a presentation, not a fork**: #55 asks for one component
with two mount points, so the roving tab stop, Alt+Arrow reorder, drag
reorder, selection semantics and `virtualizer.requestUpdate()` all come
along untouched. Escape closes it and returns focus, and is attached
only while the overlay is up — it is a dismissal, not a shortcut, which
is why it is not a panel-scoped binding.
What this does **not** fix is `page-header` overflowing on its own:
at 900×600 "New Smart Playlist" is still clipped to 114 of 162px with
the queue *closed*. That is #69, and it cannot be fixed in
`page-header` alone — actions arrive through `<slot name="actions">` as
arbitrary light-DOM markup with their own handlers, so collapsing them
into a "More actions" menu needs an actions *API* (data, not markup)
across all three hosts that slot them.
**The phone section of `index.css` is last on purpose.** A media query
adds no specificity, so a `@media (max-width: 599px)` block placed
above the plain rules it overrides loses to them — which is how phase 1
@@ -2307,6 +2369,23 @@ Pre-commit hooks verify generated code is fresh — always run `make generate` a
mistyped `feat` ships a minor version. `make release-dry` answers "what
would this merge release" without pushing.
**The analyzer reads the type and ignores the scope, so a CI-only change
is `ci:` and never `fix(ci):`.** The scope is decoration; `fix` is a
patch whatever is in the brackets. Two commits touching nothing but
`.gitea/workflows/unclaim.yml` were written `fix(ci):` and cut `v0.2.1`
and `v0.2.2` — real releases, published to Arch, Homebrew and the APK
registry, containing no user-facing change. They were left in place
rather than deleted, because a version that vanishes is worse for
whoever pulled it than one that turns out to be empty.
**The blast radius is bigger than the version number**, which is what
makes this worth a paragraph. A merge to `main` starts two workflows;
if `release.yml` then pushes a tag, that tag push starts **four more**
(`arch-package`, `homebrew-formula`, `android-apk`, `desktop-assets`) —
on a runner with capacity 1, where the APK build alone is tens of
minutes. `make release-dry` before merging is how you find out, and it
is cheaper than every one of those.
**`@semantic-release/github` is not in that config and must not be.**
Gitea's API is `/api/v1` and is not GitHub's surface, so
`@semantic-release/exec` calls `scripts/gitea-release.sh` instead — one
+25 -7
View File
@@ -13,13 +13,31 @@ const (
// enforces this at runtime; it is also the floor below which a
// reported size is treated as bogus and not persisted.
//
// 800x600 is where the shell was measured to still work, rather
// than a round number: below ~780 the header's subtitle wraps and
// pushes the title out of the 4em top bar, and below ~600 tall the
// eleven sidebar items no longer fit at once. The previous
// 512x384 was aspirational — at 700x480 the sidebar overflowed
// behind the player bar with no scroll and Settings and Jobs could
// not be reached at all.
// **Both reasons this comment used to give have expired**, and the
// value is right for a third one. It said the floor was 800x600
// because "below ~780 the header's subtitle wraps and pushes the
// title out of the 4em top bar" and "below ~600 tall the eleven
// sidebar items no longer fit at once". Neither mechanism can
// happen now: the subtitle is display:none from 899px down
// (index.css), and the sidebar host is overflow-y:auto — measured
// at 600x460, its scrollHeight is 434 against a 332px client and
// Settings is reachable after scrolling. A floor defended by two
// mechanisms that no longer exist is a number nobody can argue
// with, which is worse than either answer.
//
// It stays 800x600 because that is where the *desktop* chrome
// stops being comfortable — the Compact band of plan 018's size
// matrix (#24) — and not because the app breaks below it. It does
// not: under 600px wide the phone layout takes over (bottom-nav,
// no sidebar) and the shell fits 320px exactly, which is what
// makes this a comfort floor rather than a correctness one, and
// why a very small window reflows instead of becoming a
// mini-player (#12 is a second always-on-top window, not a mode of
// this one).
//
// The previous 512x384 was aspirational — at 700x480 the sidebar
// overflowed behind the player bar with no scroll and Settings and
// Jobs could not be reached at all.
MinWidth = 800
// MinHeight is the smallest allowed window height in pixels.
MinHeight = 600
+94
View File
@@ -82,6 +82,100 @@ func TestPruneStaleLocalCrossReferences(t *testing.T) {
}
}
// TestPruneClearsInLibraryWithNoLocalID covers the fixed point: a row
// carrying in_library with a NULL local_*_id. The upsert's conflict
// clause is `in_library = MAX(in_library, excluded.in_library)`, so it
// can only ever raise the flag, and this pass used to be gated on the id
// being present — which meant nothing in the app could clear such a row,
// ever. It is asserted for all three entity types because the gate was
// written once and used three times, so a fix applied to one is a fix
// that looks complete.
//
// The rows are seeded with raw SQL rather than through seedIndexResult
// deliberately: upsertBatch writes a zero LocalArtistID as literal 0,
// not NULL, and 0 satisfies `IS NOT NULL` — so the old gate already
// caught that shape and a fixture built through the upsert cannot
// reproduce this at all. NULL is what the artifact importer and any
// older writer leave behind, the column being nullable with no default.
func TestPruneClearsInLibraryWithNoLocalID(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
si := NewSearchIndex(db, nil, nil, slog.Default())
// A genuinely owned artist, to prove the wider gate does not simply
// clear everything it now looks at.
database.InsertTestTrack(t, db, database.TestTrack{
FilePath: "/music/owned.mp3",
Artist: "Owned",
})
artist, err := db.Queries.GetArtistByName(t.Context(), "Owned")
if err != nil {
t.Fatalf("read seeded artist: %v", err)
}
seedIndexResult(t, db, SearchIndexResult{
EntityType: EntityArtist,
MBID: testMBID("owned"),
Title: "Owned",
ArtistName: "Owned",
ArtistMBID: testMBID("owned"),
InLibrary: true,
LocalArtistID: artist.ID,
})
orphans := []struct {
name string
entityType string
mbid string
}{
{"artist", EntityArtist, "orphan-artist"},
{"release group", EntityReleaseGroup, "orphan-release-group"},
{"recording", EntityRecording, "orphan-recording"},
}
for _, o := range orphans {
if _, err := db.ExecContext(
`INSERT INTO explore_index
(entity_type, mbid, title, artist_name, artist_mbid,
in_library,
local_artist_id, local_release_group_id, local_recording_id)
VALUES (?, ?, ?, ?, ?, 1, ?, ?, ?)`,
dbEntityType(o.entityType), dbMBID(testMBID(o.mbid)), o.name, o.name,
dbMBID(testMBID(o.mbid)),
nil, nil, nil,
); err != nil {
t.Fatalf("seed %s orphan: %v", o.name, err)
}
}
si.pruneStaleLocalCrossReferences()
inLibrary := func(t *testing.T, mbid string) int {
t.Helper()
var flag int
if err := db.QueryRowWriter(
"SELECT in_library FROM explore_index WHERE mbid = ?", dbMBID(mbid),
).Scan(&flag); err != nil {
t.Fatalf("read in_library for %q: %v", mbid, err)
}
return flag
}
for _, o := range orphans {
if got := inLibrary(t, testMBID(o.mbid)); got != 0 {
t.Errorf("%s with a NULL local id: in_library = %d, want 0", o.name, got)
}
}
if got := inLibrary(t, testMBID("owned")); got != 1 {
t.Errorf("owned artist: in_library = %d, want 1 (it still has a file)", got)
}
}
// TestUnenrichedLibraryArtistMBIDs_OrdersByOwnedTrackCount verifies the
// backfill queue prioritizes artists by how many tracks the user actually
// owns, not by how many duplicate-mbid artist rows happen to exist (the
+15 -1
View File
@@ -2562,6 +2562,19 @@ func (si *SearchIndex) PopulateLocalCrossReferences() {
// The row itself is left in place (it may still be part of the shipped
// catalog, just no longer owned) — only the "this is mine" bookkeeping
// is cleared.
//
// It is gated on the flag *or* the id, not on the id alone. Gated on
// the id, `in_library = 1 AND local_*_id IS NULL` is a fixed point: the
// upsert can only ever raise the flag and this pass skipped such a row
// by construction, so nothing in the app could clear it — a row claiming
// to be owned, permanently, with no local row to check the claim
// against. Nothing in the tree writes that shape today
// (collectLibraryEntities sets both together), which is exactly why it
// is worth closing now: the exposure is a database written by an older
// version, and the next writer that sets the flag without an id, which
// nothing structurally prevents. A NULL id fails the existence test on
// its own, so the wider gate needs no second clause to say what "not
// owned" means.
func (si *SearchIndex) pruneStaleLocalCrossReferences() {
type prune struct {
entityType string
@@ -2594,7 +2607,8 @@ func (si *SearchIndex) pruneStaleLocalCrossReferences() {
result, err := si.db.ExecContext(
`UPDATE explore_index
SET in_library = 0, `+p.column+` = NULL
WHERE entity_type = ? AND `+p.column+` IS NOT NULL
WHERE entity_type = ?
AND (`+p.column+` IS NOT NULL OR in_library = 1)
AND NOT EXISTS (`+p.exists+`)`,
dbEntityType(p.entityType),
)
+27 -11
View File
@@ -1,6 +1,12 @@
import { test, expect } from '../support/fixtures.js';
import type { Page } from '@playwright/test';
/**
* How far the scroll test scrolls. One constant, because the guard and
* the assertion have to agree about it — they did not, which is #133.
*/
const SCROLL_TARGET = 80;
/**
* Plan 007 phase 5: expanding an album shows its tracks.
*
@@ -104,20 +110,27 @@ test.describe('the album dropdown', () => {
await app.setViewportSize({ width: 900, height: 600 });
try {
await expect.poll(() => scrollRange(app)).toMatchObject({
scrollable: true,
overflowY: 'auto',
});
// Wait for the range the assertion below actually needs, not for
// "scrollable at all" (#133). The guard used to be
// `scrollHeight > clientHeight + 40` while the next line asks to
// reach 80, so any range in 41-79 satisfied it and could not
// satisfy the assertion — and the grid passes through exactly
// that while it settles, because it recomputes its columns after
// the resize rather than during it. The settled range here is
// 330, so this waits rather than weakening anything.
await expect
.poll(() => scrollRange(app))
.toMatchObject({ room: true, overflowY: 'auto' });
await app.evaluate(() => {
await app.evaluate((target) => {
const sc = document
.querySelector('cover-grid')
?.shadowRoot?.querySelector('.grid-scroll-container');
if (sc) sc.scrollTop = 80;
});
if (sc) sc.scrollTop = target;
}, SCROLL_TARGET);
expect(await scrollTop(app)).toBe(80);
expect(await scrollTop(app)).toBe(SCROLL_TARGET);
// And the dropdown it opens is on screen, wherever the manager
// decides that leaves the scroll. It is *not* "the position is
@@ -250,16 +263,19 @@ async function closeDropdown(app: Page): Promise<void> {
/** Whether the grid can scroll at all, which decides if a probe can move. */
async function scrollRange(app: Page) {
return app.evaluate(() => {
return app.evaluate((target) => {
const sc = document
.querySelector('cover-grid')
?.shadowRoot?.querySelector('.grid-scroll-container');
return {
scrollable: !!sc && sc.scrollHeight > sc.clientHeight + 40,
// `room` is the precondition of the assertion that follows it:
// enough range to actually reach the target. A threshold below
// what the caller depends on is not a guard.
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
overflowY: sc ? getComputedStyle(sc).overflowY : '',
};
});
}, SCROLL_TARGET);
}
async function scrollTop(app: Page): Promise<number> {
+6
View File
@@ -26,6 +26,12 @@ const MIN_VIEWPORT = { width: 800, height: 600 };
const VIEWPORTS = [
{ name: '1440×900', width: 1440, height: 900 },
{ name: '1024×768', width: 1024, height: 768 },
// Not the minimum, and that is the point (#24). The sidebar collapses
// to icons *below* 900, so the main panel is 843px at 899 and 700px
// at 900 — the narrowest content area any desktop width produces is
// here, not at the enforced floor. A list that stopped at the minimum
// was missing its own worst case.
{ name: '900×600 (the widest sidebar, so the narrowest content)', width: 900, height: 600 },
{ name: `the minimum (${MIN_VIEWPORT.width}×${MIN_VIEWPORT.height})`, ...MIN_VIEWPORT },
];
+187
View File
@@ -0,0 +1,187 @@
import { test, expect } from '../support/fixtures.js';
/**
* #24 — the queue panel does not take the page's width away from it.
*
* The panel is `flex-shrink: 0` in the flow of `.content-area`, so an
* open queue used to be paid for by the main panel. Measured on
* Playlists before the fix:
*
* | viewport | main panel |
* |---|---|
* | 900×600 | 379px — all three header actions clipped |
* | 390×780 | 69px |
* | 320×600 | **0px** |
*
* **900×600 is the worst desktop case, not the 800×600 minimum**, and
* that is the trap this file exists to keep closed: the sidebar
* collapses to icons *below* 900, so the main panel is 843px at 899 and
* 700px at 900. A spec that checks "the minimum" and stops has not
* checked the worst case — which is what every viewport list in this
* suite did before this.
*
* These assert the *content's* width rather than the panel's mode
* wherever they can, because the mode is the mechanism and the width is
* the complaint.
*/
/** The bands from plan 018's size matrix, plus the pixel above the collapse. */
const BANDS = [
{ name: 'a wide desktop (1280×800)', width: 1280, height: 800, inline: true },
{ name: 'the default window (1100×720)', width: 1100, height: 720, inline: true },
{ name: 'a laptop (1024×768)', width: 1024, height: 768, inline: true },
{ name: 'the worst desktop width (900×600)', width: 900, height: 600, inline: false },
{ name: 'the enforced minimum (800×600)', width: 800, height: 600, inline: false },
{ name: 'a phone (390×780)', width: 390, height: 780, inline: false },
{ name: '400% zoom (320×600)', width: 320, height: 600, inline: false },
];
/**
* How much room the content has, and whether the shell needs scrolling
* to reach any of itself.
*/
const shellGeometry = (page: import('@playwright/test').Page) =>
page.evaluate(() => {
const main = document.querySelector('#main-content')!.getBoundingClientRect();
const panel = document.querySelector('#queue-panel')!;
return {
mainWidth: Math.round(main.width),
overlay: panel.hasAttribute('overlay'),
open: panel.hasAttribute('open'),
bodyScrollWidth: document.body.scrollWidth,
bodyClientWidth: document.body.clientWidth,
};
});
async function openQueue(page: import('@playwright/test').Page) {
const toggle = page.locator('#queue-button');
if ((await toggle.getAttribute('aria-expanded')) !== 'true') {
await toggle.click();
}
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
}
test.describe('an open queue leaves the content its width', () => {
for (const band of BANDS) {
test(`at ${band.name}`, async ({ app }) => {
await app.setViewportSize({ width: band.width, height: band.height });
await openQueue(app);
// The mode is settled by a ResizeObserver, so poll rather than
// read once: a single read races the resize and reports the
// previous viewport's answer.
await expect
.poll(async () => (await shellGeometry(app)).overlay)
.toBe(!band.inline);
const geo = await shellGeometry(app);
// The floor is the point of the whole issue. Inline, the queue is
// affordable and the content keeps the rest; as an overlay the
// content keeps *everything*, which is what makes 0px at 320
// impossible rather than merely unlikely.
expect(geo.mainWidth).toBeGreaterThanOrEqual(320);
if (!band.inline) {
expect(geo.mainWidth).toBeGreaterThanOrEqual(
Math.min(band.width, 320),
);
}
// And opening the queue must not make the shell overflow.
expect(geo.bodyScrollWidth).toBeLessThanOrEqual(geo.bodyClientWidth);
});
}
});
test.describe('an overlaid queue says it is over the content', () => {
test.beforeEach(async ({ app }) => {
await app.setViewportSize({ width: 900, height: 600 });
});
test('draws a scrim and closes when it is clicked', async ({ app }) => {
await openQueue(app);
const panel = app.locator('#queue-panel');
await expect(panel).toHaveAttribute('overlay', '');
// The scrim is `aria-hidden` on purpose — it is a dismissal target,
// and the named routes out are the close button and Escape — so it
// is located structurally rather than by role.
await panel.evaluate((el) =>
el.shadowRoot!.querySelector<HTMLElement>('.scrim')!.click(),
);
await expect(app.locator('#queue-button')).toHaveAttribute(
'aria-expanded',
'false',
);
});
/**
* `getByRole`, not a shadow-root query: this repo has shipped a
* nameless control three times, and a drawer with a scrim is exactly
* the shape that grows a fourth.
*/
test('offers a named close button', async ({ app }) => {
await openQueue(app);
const close = app.getByRole('button', { name: 'Close queue' });
await expect(close).toBeVisible();
await close.click();
await expect(app.locator('#queue-button')).toHaveAttribute(
'aria-expanded',
'false',
);
});
test('closes on Escape and gives focus back to the toggle', async ({
app,
}) => {
const toggle = app.locator('#queue-button');
await toggle.focus();
await toggle.click();
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
await app.keyboard.press('Escape');
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
await expect(toggle).toBeFocused();
});
});
/**
* The inline panel is the mode that already worked, and the one every
* other queue spec is written against. It keeps its resize handle and
* gains none of the overlay's chrome.
*/
test.describe('a wide window keeps the queue beside the content', () => {
test('no scrim, no close button, and the content is narrower', async ({
app,
}) => {
await app.setViewportSize({ width: 1280, height: 800 });
const widthWithoutQueue = (await shellGeometry(app)).mainWidth;
await openQueue(app);
await expect(app.locator('#queue-panel')).not.toHaveAttribute(
'overlay',
'',
);
const geo = await shellGeometry(app);
expect(geo.mainWidth).toBeLessThan(widthWithoutQueue);
await expect(
app.getByRole('button', { name: 'Close queue' }),
).toHaveCount(0);
});
});
+8
View File
@@ -231,6 +231,14 @@ body div.sidebar {
display: flex;
overflow: hidden;
contain: layout style;
/* The containing block for the queue panel's overlay mode (plan
018, #24), which spans this box rather than taking width from
the main panel beside it. `contain: layout` already establishes
one; this says so on purpose, so that removing the containment
for a paint reason does not silently reparent the overlay to the
viewport. */
position: relative;
}
.main-panel {
+43 -20
View File
@@ -160,29 +160,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 {
@@ -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');
}
@@ -72,6 +72,24 @@ const MIN_WIDTH = 200;
const MAX_WIDTH = 500;
const DEFAULT_WIDTH = 320;
/**
* The narrowest main panel the queue is allowed to leave behind before
* it stops being a column and becomes an overlay (plan 018, issue #24).
*
* There is no cliff to derive this from, and pretending otherwise would
* be the more dishonest answer: the track list rescales its columns
* continuously (213px down to 124px between main widths of 900 and 544,
* with no row overflow at any of them) and the album grid steps 3
* columns to 2 without breaking. So this is a judgement, anchored at
* both ends — it keeps the *default* 1100px window inline, because an
* inline queue is a desktop affordance people choose and demoting the
* common case to an overlay would be a regression in feel; and it puts
* every case measured as broken on the overlay side, which is 900x600
* (main = 379px, where all three of the Playlists header's actions are
* clipped) and every phone width (main = 69px at 390, 0px at 320).
*/
const MAIN_PANEL_FLOOR = 480;
@customElement('queue-panel')
export class QueuePanel
extends LitElement
@@ -85,6 +103,22 @@ export class QueuePanel
@property({ type: Boolean, reflect: true })
open = false;
/**
* Whether the panel is covering the content instead of sitting
* beside it. **Computed, never set by a caller** — it is reflected
* so the stylesheet and a spec can both read it.
*
* It is deliberately *not* a media query, which is the whole reason
* this is a property and not a `@media` block. The panel's width is
* user state: drag-resizable between MIN_WIDTH and MAX_WIDTH and
* persisted. A breakpoint at a fixed viewport width silently
* assumes the default 320, so it is wrong by up to 180px for a user
* who has widened the panel — in the direction that hurts, since a
* wider queue is exactly when the content can least afford it.
*/
@property({ type: Boolean, reflect: true })
overlay = false;
@state()
private isDragging = false;
@@ -190,6 +224,19 @@ export class QueuePanel
private panelWidth = DEFAULT_WIDTH;
private scrollbarDragging = false;
/** Watches `.content-area`, which is the viewport minus the sidebar. */
private spaceObserver?: ResizeObserver;
/**
* What had focus when the overlay opened, so Escape and the scrim
* can give it back. Focus is only taken back if the panel had it —
* the same rule `MenuKeyboard` follows, for the same reason: the
* queue can also be closed by the button in the bottom bar, and
* yanking focus away from wherever the user actually is would be
* worse than leaving it.
*/
private overlayOpener: HTMLElement | null = null;
// _itemSize is an internal property applied via Object.assign in BaseLayout's
// config setter. Setting it to match the actual fixed .track-item height (49px)
// prevents lit-virtualizer's scroll error correction from fighting the native
@@ -265,6 +312,86 @@ export class QueuePanel
border-left: 1px solid var(--yj-border-subtle, #333);
}
/* ---------------------------------------------------------
Overlay mode (plan 018, #24).
In flow the panel takes its width *from the main panel*,
which is the reported bug: at 900x600 that left 379px and
clipped every action in the Playlists header, and at 320px
it left 0px — the content was not degraded but gone.
Here the host spans the whole content area instead and
stops being a layout participant, so the main panel keeps
its full width and the queue sits over it. The host itself
is transparent and click-through; the scrim and the panel
are what take pointer events. The containment drops paint,
which would otherwise clip the panel's own shadow.
--------------------------------------------------------- */
:host([overlay]) {
position: absolute;
inset: 0;
width: auto;
background-color: transparent;
overflow: visible;
pointer-events: none;
contain: layout style;
z-index: 20;
}
/* Closed, an overlay is not there at all. In flow the panel is
width: 0, which is its own way of saying this; absolutely
positioned there is no width to collapse. */
:host([overlay]:not([open])) {
display: none;
}
:host([overlay][open]) {
border-left: none;
}
:host([overlay]) .panel-content {
position: absolute;
top: 0;
right: 0;
bottom: 0;
width: var(--queue-width, ${unsafeCSS(DEFAULT_WIDTH)}px);
max-width: 100%;
box-sizing: border-box;
background-color: var(--yj-bg-surface, #212529);
border-left: 1px solid var(--yj-border-subtle, #333);
box-shadow: -8px 0 24px rgb(0 0 0 / 45%);
pointer-events: auto;
}
/* Dragging the edge of something that is already covering the
content answers a question nobody asked, and it is a
mouse-only affordance either way. */
:host([overlay]) .resize-handle {
display: none;
}
.scrim {
position: absolute;
inset: 0;
background-color: rgb(0 0 0 / 45%);
pointer-events: auto;
border: none;
padding: 0;
margin: 0;
cursor: pointer;
}
/* The phone gets the whole width: below 600 there is no
"beside" left to be, and this is the shape #55 turns into a
real screen. A media query inside a shadow root is answered
by the viewport, so the component states this itself rather
than the shell reaching in. */
@media (max-width: 599px) {
:host([overlay]) .panel-content {
width: 100%;
}
}
.resize-handle {
position: absolute;
top: 0;
@@ -656,6 +783,20 @@ export class QueuePanel
'--queue-width',
`${this.panelWidth}px`,
);
// The mode is a measurement, so it is observed rather than
// computed once: the parent is `.content-area`, whose width is
// the viewport minus the sidebar — including the sidebar's own
// collapse at 900px, which is what makes 900 the *worst*
// desktop width rather than the minimum.
this.updateOverlayMode();
if (this.parentElement) {
this.spaceObserver = new ResizeObserver(() =>
this.updateOverlayMode(),
);
this.spaceObserver.observe(this.parentElement);
}
document.addEventListener(
'mousemove',
this.handleMouseMove,
@@ -690,6 +831,9 @@ export class QueuePanel
super.disconnectedCallback();
this.creditsUnsub?.();
this.creditsUnsub = undefined;
this.spaceObserver?.disconnect();
this.spaceObserver = undefined;
document.removeEventListener('keydown', this.onOverlayKeydown);
document.removeEventListener(
'mousemove',
this.handleMouseMove,
@@ -736,7 +880,79 @@ export class QueuePanel
this.delegationAttached = false;
}
/**
* Decide whether the queue can afford to be a column.
*
* The parent is `.content-area`, so its width is the viewport minus
* the sidebar and the sum already accounts for the sidebar's own
* collapse. It is stable across the panel's own open/closed state
* in both modes — in flow the panel is a child of that box, and as
* an overlay it is out of flow — so this cannot oscillate.
*/
private updateOverlayMode = () => {
const available = this.parentElement?.clientWidth ?? 0;
// Before layout there is nothing to measure, and answering 0 by
// flipping to overlay would show the scrim for a frame.
if (available === 0) return;
this.overlay = available - this.panelWidth < MAIN_PANEL_FLOOR;
};
/**
* Escape closes a scrimmed overlay, which is the one keyboard rule
* every dialog in this app already follows.
*
* It is a document listener rather than a panel-scoped binding
* (`services/shortcut-scope.ts`) because it is not a *shortcut*: it
* is the dismissal of something covering the page, and it has to
* work while focus is still behind the scrim. It is attached only
* while the overlay is actually up and removed on close, so it is
* scoped to a state rather than being a permanent global. Nothing
* else binds Escape — the shortcut service only uses it to blur a
* text input.
*/
private onOverlayKeydown = (e: KeyboardEvent) => {
if (e.key !== 'Escape' || !this.open || !this.overlay) return;
e.stopPropagation();
this.closeFromOverlay();
};
private closeFromOverlay = () => {
const hadFocus = this.contains(
document.activeElement as Node | null,
);
this.open = false;
if (hadFocus) {
const back =
this.overlayOpener ??
document.getElementById('queue-button');
back?.focus();
}
this.overlayOpener = null;
};
override updated() {
// The overlay owns Escape only while it is up.
if (this.open && this.overlay) {
document.addEventListener('keydown', this.onOverlayKeydown);
this.overlayOpener ??=
document.activeElement instanceof HTMLElement &&
document.activeElement !== document.body
? document.activeElement
: null;
} else {
document.removeEventListener('keydown', this.onOverlayKeydown);
if (!this.open) this.overlayOpener = null;
}
// Closed, the panel is `width: 0` — which hides it from the eye
// and from nobody else: its Clear and Add buttons still took tab
// stops at x=1440 and were still read out (H-5). `inert` is the
@@ -1631,6 +1847,11 @@ export class QueuePanel
'--queue-width',
`${clampedWidth}px`,
);
// Widening the panel is one of the two ways the content can run
// out of room, and it is the way a viewport-width media query
// cannot see at all.
this.updateOverlayMode();
};
private handleMouseUp = () => {
@@ -1714,6 +1935,15 @@ export class QueuePanel
const tracks = this.queue.tracks;
return html`
${this.overlay
? html`<div
class="scrim"
part="scrim"
data-testid="queue-scrim"
aria-hidden="true"
@click=${this.closeFromOverlay}
></div>`
: nothing}
<div class="panel-content">
<div
class="resize-handle ${this.isDragging
@@ -1763,6 +1993,16 @@ export class QueuePanel
name=${ICON_PLAYLIST}
></wa-icon>
</button>
${this.overlay
? html`<button
class="header-action-button"
data-testid="queue-close"
aria-label="Close queue"
@click=${this.closeFromOverlay}
>
<wa-icon name="xmark"></wa-icon>
</button>`
: nothing}
</div>
</div>
@@ -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.
+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)';
@@ -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);
});
});
@@ -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',
});
});
});
@@ -0,0 +1,175 @@
/**
* #24 — the queue stops being a column when it cannot afford to be one.
*
* In flow the panel is `flex-shrink: 0`, so it takes its width *from
* the main panel* rather than covering it. Measured against the running
* app on the Playlists page, that left 379px of content at 900×600 —
* with all three of the page header's actions clipped — 69px at 390px
* wide, and **0px** at 320px, where the content was not degraded but
* gone.
*
* The rule is `available - panelWidth >= MAIN_PANEL_FLOOR`, and the
* reason it is a computed property rather than a `@media` block is the
* third test here: the panel's width is user state, drag-resizable
* between 200 and 500px and persisted, so a breakpoint on the viewport
* alone is wrong by up to 180px for a user who has widened it — in the
* direction that hurts, since a wider queue is exactly when the content
* can least afford it.
*
* The parent is `.content-area`, i.e. the viewport minus the sidebar,
* which is why these mount into a sized wrapper rather than into
* `document.body`: the width that decides this is the *parent's*, and
* `fixture()` would hand the panel the whole test window.
*/
import { describe, expect, it, afterEach } from 'vitest';
import '@components/queue-panel/queue-panel';
import type { QueuePanel } from '@components/queue-panel/queue-panel';
import { shadow } from '@test/support/render';
const wrappers: HTMLElement[] = [];
afterEach(() => {
for (const w of wrappers.splice(0)) w.remove();
});
/**
* Mount a panel inside a parent of a stated width.
*
* The wrapper is `position: relative` and `display: flex` because that
* is what `.content-area` is; the mode is measured from
* `parentElement.clientWidth`, so a wrapper that collapses to its
* content would measure the panel rather than the space around it.
*/
async function panelIn(parentWidth: number): Promise<QueuePanel> {
const wrapper = document.createElement('div');
wrapper.style.cssText = `position: relative; display: flex; width: ${parentWidth}px;`;
document.body.append(wrapper);
wrappers.push(wrapper);
const el = document.createElement('queue-panel') as QueuePanel;
el.open = true;
wrapper.append(el);
await el.updateComplete;
await settle(el);
return el;
}
/**
* A ResizeObserver delivers on a frame, not a microtask, so the mode
* lands a frame after the width that decides it.
*/
async function settle(el: QueuePanel): Promise<void> {
for (let frame = 0; frame < 4; frame += 1) {
await new Promise((r) => {
requestAnimationFrame(() => r(null));
});
await el.updateComplete;
}
}
/** Drag the resize handle by `dx`, the way a user widens the panel. */
async function dragHandleBy(el: QueuePanel, dx: number): Promise<void> {
const handle = shadow<HTMLElement>(el, '.resize-handle');
const startX = el.getBoundingClientRect().left;
if (!handle) throw new Error('no resize handle to drag');
handle.dispatchEvent(
new MouseEvent('mousedown', { clientX: startX, bubbles: true }),
);
document.dispatchEvent(
new MouseEvent('mousemove', { clientX: startX - dx, bubbles: true }),
);
document.dispatchEvent(new MouseEvent('mouseup', { bubbles: true }));
await settle(el);
}
describe('the queue panel decides whether it can be a column', () => {
it('stays inline while the content can spare the width', async () => {
const el = await panelIn(1080);
expect(el.overlay).toBe(false);
expect(el.hasAttribute('overlay')).toBe(false);
});
it('becomes an overlay when it cannot', async () => {
const el = await panelIn(700);
expect(el.overlay).toBe(true);
expect(el.hasAttribute('overlay')).toBe(true);
});
/**
* The test the media query could not have passed. The parent does not
* move; only the user's own panel width does.
*/
it('flips to overlay when the user widens the panel, at a fixed width', async () => {
const el = await panelIn(880);
expect(el.overlay).toBe(false);
await dragHandleBy(el, 180);
expect(el.overlay).toBe(true);
});
it('gives an overlay a scrim and a named way out, and an inline panel neither', async () => {
const overlaid = await panelIn(700);
expect(shadow(overlaid, '.scrim')).toBeTruthy();
const close = shadow(overlaid, '[data-testid="queue-close"]');
expect(close?.getAttribute('aria-label')).toBe('Close queue');
const inline = await panelIn(1080);
expect(inline.shadowRoot?.querySelector('.scrim')).toBeNull();
expect(
inline.shadowRoot?.querySelector('[data-testid="queue-close"]'),
).toBeNull();
});
it('closes on the scrim, on the close button and on Escape', async () => {
for (const close of [
(el: QueuePanel) => shadow<HTMLElement>(el, '.scrim')?.click(),
(el: QueuePanel) =>
shadow<HTMLElement>(el, '[data-testid="queue-close"]')?.click(),
() =>
document.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }),
),
]) {
const el = await panelIn(700);
expect(el.open).toBe(true);
close(el);
await el.updateComplete;
expect(el.open).toBe(false);
}
});
/**
* Escape belongs to the overlay, not to the queue. An inline panel is
* beside the content rather than over it, so there is nothing to
* dismiss and the key has to reach whatever else wants it.
*/
it('leaves Escape alone while inline', async () => {
const el = await panelIn(1080);
document.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }),
);
await el.updateComplete;
expect(el.open).toBe(true);
});
});
+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"; }