Compare commits

...
Author SHA1 Message Date
logan d347809e6e docs: record the one statement of which view is active
CI / e2e (push) Skipped
CI / check (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 6m53s
It belongs beside the two rules that already keep the history stack and
the in-app back buttons agreeing, and for the same reason: a second
component-local idea of where the user is, is how they came to
disagree.
2026-08-19 17:00:39 -04:00
logan a5ffcc22e3 test(e2e): assert the nav highlight, not the shell's own bookkeeping
`back-navigation.spec.ts` covered exactly the journeys #72 breaks and
was green throughout it, because every assertion in it was
`data-active-view` — which the shell sets on every path including the
back one, and which was the one thing already correct. The same trap
`layout-overflow.spec.ts` set for #69: a spec named for the behaviour,
measuring the plumbing.

The assertions go here rather than in a second file, or the first would
carry on passing vacuously. They are `aria-current="page"` through
`getByRole`, which is the accessible fact — `.active` is a class and
could be restyled without breaking anything real — and the role query
resolves to whichever nav is in the accessibility tree at that
viewport, so one helper covers the sidebar and the tab bar.

Three of the four fail on the build before the fix. The fourth, the
parent staying lit while a detail view is open, passed by accident and
says so.
2026-08-19 17:00:39 -04:00
logan f18691560d fix(shell): publish the active view, so both navs follow the back path
The nav components learned where the user was from the `navigate`
CustomEvent, which only the outbound path dispatches: `popstate` calls
`handleNavigate()` directly. So a back-navigation left both of them
highlighting the view just left — desktop included, at any width, on
any back across two primary views. Opening a detail view was the same
cause wearing a different symptom: `app-sidebar` guarded on its own
item list and kept its highlight, `bottom-nav` did not and lit nothing.

It cannot be fixed by re-dispatching `navigate` — `index.ts` is that
event's document listener, so that is an infinite loop, and "please go
to X" is not the statement being made. `activeViewStore` is the shell
saying "the active view is now X", once per navigation, `popstate`
included; both navs read it through a controller and hold no
`activeView` of their own.

A store rather than an event because a component that mounts *after* a
navigation still has to know: `bottom-nav`'s drawer builds its
`app-sidebar` on open, and that copy had heard nothing at all, so the
drawer opened on Home from any page in the app.

Closes #72
2026-08-19 17:00:39 -04:00
logan c84a9069ef Merge pull request 'Collapse the page header actions that do not fit, instead of clipping them' (#136) from fix/69-page-header-action-overflow into main
CI / check (push) Successful in 2m33s
CI / e2e (push) Successful in 6m53s
Closes #69
2026-08-19 19:29:15 +00:00
logan f967916550 fix(page-header): collapse the actions that do not fit into a menu
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m36s
CI / e2e (pull_request) Successful in 7m17s
Playlists slotted three buttons totalling 390px into a header that gets
700px at 900x600, so "New Smart Playlist" rendered 114 of its 162px
with the queue closed, and 158 of 162 at the 800x600 enforced minimum.
On a phone none of the three could be reached at all, which is what the
Android report said. Plan 018's size matrix promises the opposite: no
action is ever unreachable at any supported size.

The header could not fix that for slotted markup, and that is a fact
about the API rather than an effort estimate — a component cannot move
another component's light-DOM children into a dropdown and keep their
behaviour, and arbitrary markup offers nothing generic to render as a
menu item. So a host passes `PageAction[]` and the header chooses the
rendering; the slot survives for markup a data list cannot express, at
the stated cost that a slotted action does not collapse.

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

Four things in it are load-bearing:

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

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

Closes #69
2026-08-19 15:08:57 -04:00
logan 3fa7c7734b docs: record the page-header actions rule, and complete plan 018
The `page-header` paragraph already stated "the header asks for a sort,
it does not perform one"; actions now follow the same division and it
belongs beside it — the header decides what fits, the host decides what
happens.

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

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

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

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

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

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

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

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

Closes #68
2026-08-19 14:08:06 -04:00
yonlu 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
43 changed files with 3741 additions and 247 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,327 @@
# 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:** complete — #24 shipped as PR #132, and the matrix's last
unkept promise closed with #69.
#73 puts this first in Phase 2 and hangs the rest of the phase off it,
so the decision has to be written down and arguable before any CSS
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.
## What #69 did with the promise, and one thing this plan got wrong
#69 landed on its own branch as decision 3 said it would, and the
matrix's *no action is ever unreachable at any supported size* is now
kept rather than promised. Measured on Playlists, actions clipped:
| viewport | before #24 | after #24 | after #69 |
|---|---|---|---|
| 900×600, queue open | all three | one (114/162px) | none |
| 900×600, queue closed | one | one | none |
| 800×600, queue closed | one (158/162px) | one | none |
| 390×780 | all three | all three | none |
| 320×600 | all three | all three | none |
The shape was the one decision 3 predicted — an actions API first, an
overflow rule second — and all three hosts that slot actions migrated.
**What this document got wrong is smaller and worth keeping.** Decision
1 says the header's minimum is a *comfort* floor and that only the
queue and the actions compete for the header's width. They are not the
only two: every child of that flex row was `flex-shrink: 0`, so
whatever came last lost, and the actions come last. At 320px the sort
control alone is 172px of the header — so with every action already
collapsed into the menu, the *menu button* was 76px off the right edge.
The promise was still broken with nothing left to collapse.
That is why #69 also had to decide what gives way: the title (which the
navigation also states) and, below 600px, the word "Sort:" (which the
direction arrow implies). Neither is an action, which is the rule the
matrix actually encodes — **an action is a capability and everything
else on that row is a label.**
## Verification, and what each tier cannot see
- `make ui-test` — the queue panel's mode logic is component-tier
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.
+196
View File
@@ -952,6 +952,50 @@ kept beside it, because two stacks is precisely how a view's own back
button and the phone's gesture come to disagree about what one press
means.
**And there is one statement of which view is active**, for the same
reason: `popstate` calls `handleNavigate()` directly and dispatches no
`navigate`, so the two nav components — which learned the active view
from that event — kept highlighting the view the user had just *left*.
`store/active-view-store.ts` is the shell saying where the user is, and
both navs read it through `ActiveViewController` rather than holding an
`activeView` of their own.
Four things about it are load-bearing.
**"Please go to X" and "the active view is now X" are different
statements**, and only the first existed — dispatched from 28 call
sites across 18 files. A re-dispatch from inside `handleNavigate` is
not the fix and cannot be: that function is the `document` listener for
`navigate`, so it is an infinite loop.
**It is a store rather than an event, because a component that mounts
after a navigation still has to know.** `bottom-nav`'s "More" drawer
creates its `<app-sidebar>` on open, and that copy had heard no
`navigate` at all — standing on Albums, the drawer opened highlighting
Home. An event has no answer for a listener that was not there.
**A detail view is not a view here**, so the destination it was opened
from stays lit. `app-sidebar` did that by accident (it guarded on
`navItems.some(...)`, so an unmatched name left its highlight alone)
and `bottom-nav` had no such guard and so lit *nothing* — which is why
one looked right and the other looked broken on the same screen.
Whether a view is primary is the shell's fact: `view in VIEW_TAGS` is
passed to `setView`, never re-derived, because a second copy of that
list is a second thing to forget.
**Nothing is lit until the shell has navigated.** The store starts
empty rather than defaulting to `home`, which is what `app-sidebar`'s
field used to do to match the landing view — a default that is correct
only while `GetDefaultPage()` agrees with it.
The assertion is `aria-current="page"`, in
`e2e/specs/back-navigation.spec.ts`. That file existed throughout the
bug, covered exactly these journeys, and asserted only
`data-active-view` — the shell's own bookkeeping, which was right the
whole way through — so it was green on the broken build. Same trap as
`layout-overflow.spec.ts` and `page-header`: a spec named for the
behaviour, measuring the plumbing.
**A primary view is cached, not unmounted.** `index.ts` keeps every
primary view in the DOM and toggles a `.view-hidden` class, because that
is what preserves `scrollTop` across navigation — so
@@ -1328,6 +1372,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
@@ -1918,6 +2024,79 @@ that corrects itself a moment later is worse than saying nothing. And
the field, the direction and their persistence, so the control cannot
disagree with the list.
**And an action is data, on that same rule: the header decides what
fits, the host decides what happens.** Playlists slotted three buttons
totalling 390px into a header that gets 700px at 900×600, so "New Smart
Playlist" rendered **114 of its 162px** with the queue closed — and on a
phone none of them could be reached at all, which is what #69 reported.
A host passes `PageAction[]` (`{id, label, icon, onSelect, priority,
drop?}`) and `page-header` renders each one as a button or as an item in
one "More actions" menu.
**It could not have been a rule added in one place**, and that is a fact
about the API rather than an effort estimate: actions used to arrive
through `<slot name="actions">` as arbitrary light-DOM markup, and a
component cannot move another component's light-DOM children into a
dropdown and keep their behaviour — there is nothing generic in markup
to render as a menu item. The slot survives for markup a data list
cannot express, at the stated cost that **a slotted action does not
collapse** and must therefore fit at 800×600.
Six things about it are load-bearing:
- **The fit is measured, never breakpointed.** A ResizeObserver drives
it, and each pass starts from *all visible* and hides the
lowest-priority action until it fits — so the collapsed set is a pure
function of the current width rather than of how the window got
there. A rule that only ever added to the set would never give a
button back, and one that adjusted by a step would need a hysteresis
band to stop it oscillating on the pixel where a button exactly fits.
- **"Fits" means nothing is clipped, which is not the same as the
header not overflowing.** The title can ellipsis, and the moment it
can it absorbs the pressure: `scrollWidth` reports a header that fits
perfectly while the heading reads "Playlis…". That is this bug moved
from the button to the title, invisible to the same measurement that
missed it the first time — so the heading's own truncation counts as
not fitting, and an action is collapsed before the title gives way.
Below that, at 320px, the title *is* what yields: the navigation also
says which page you are on, and an action has nowhere else to be said.
- **The measurement flips `hidden` on the rendered nodes rather than
re-rendering between steps.** Reading `scrollWidth` forces layout,
which is the point; awaiting a Lit update between steps instead lets
the intermediate all-visible state paint, so the fix would flash the
overflow it exists to prevent.
- **Priority is what a *capability* costs, not what a button is worth.**
New Playlist is highest because it is the **drop target** and a closed
menu cannot be one; that is also why `PageAction.drop` carries the
host's own `dragover`/`dragleave`/`drop` handlers rather than the
header owning a notion of dropping, and why the affordance is simply
absent from the overflow rather than approximated there.
- **`aria-controls` names a panel that is always in the DOM** —
`config-section`'s rule, and `wa-popup` hides it when inactive — and
the keyboard model is `MenuKeyboard`, shared with every other menu in
the app so this is not a second one.
- **It is checked per button, because `layout-overflow.spec.ts` cannot
see this.** That spec asserts the *shell* needs no sideways
scrolling and passed on the broken build; clipping *inside* a
component is invisible to it, which is exactly why the defect
survived a spec named for it.
`e2e/specs/header-action-overflow.spec.ts` measures each button
against its header at 900×600, 800×600, 390×780 and 320×600, and
asserts buttons **plus** menu account for every declared action —
without that half it would pass vacuously on a build that renders no
actions at all.
One thing it deliberately does **not** grow is a phone mode for the
actions. `PHONE_COLUMN_IDS` is the precedent for "what is drawn and
what can be sorted are different questions", but it exists because the
track list's columns cannot be derived from a width; these can, and a
second declaration of what a phone shows is a second thing to keep in
step. What the header *does* state at phone width is one word: below
600px the sort control's "Sort:" label is visually hidden — 172px of a
320px header for a label the adjacent direction arrow implies — and it
stays in the accessibility tree, because it is the select's accessible
name and hiding it outright is `config-field`'s bug one component over.
**The header search box is view-scoped, and now says so.** It sits in
the app header and reads as global; typing `tide` on Playlists answered
"No playlists match your search" with three *Tideline* tracks in the
@@ -2307,6 +2486,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> {
+136
View File
@@ -15,12 +15,49 @@ import { test, expect } from '../support/fixtures.js';
*
* What it cannot answer is whether Android's *gesture* reaches the
* WebView, which is between the OS and the scaffold.
*
* **And `data-active-view` is not the behaviour.** Every assertion here
* used to be that attribute, which the shell sets on every path
* including `_isBack` — so this file was green throughout #72, in
* which both navs highlighted the view the user had just *left*. The
* shell's own bookkeeping was the one thing that was already right;
* what a person sees is `aria-current`, and that is asserted below as
* well. This is the same trap `layout-overflow.spec.ts` set for #69: a
* spec named for the behaviour, measuring the plumbing.
*/
type Page = import('@playwright/test').Page;
const activeView = (page: Page) =>
page.getByTestId('main-content');
/** A common phone, where the bottom bar is the primary navigation. */
const PHONE = { width: 390, height: 844 };
/**
* The nav item for a destination, in whichever navigation is on screen.
*
* Both navs carry a button named `Albums`, and only one of them is ever
* in the accessibility tree — the other is `display: none` — so the
* role query resolves to the one the user can see at this viewport.
* That is the point: the highlight has to be right in both, and #72 was
* two different-looking symptoms of one cause.
*/
const navItem = (page: Page, label: string) =>
page.getByRole('button', { name: label, exact: true });
/**
* `aria-current="page"` is the accessible fact and the assertion worth
* making; `.active` is a class and could be restyled without breaking
* anything real.
*/
async function expectHighlighted(page: Page, label: string): Promise<void> {
await expect(navItem(page, label)).toHaveAttribute('aria-current', 'page');
}
async function expectNotHighlighted(page: Page, label: string): Promise<void> {
await expect(navItem(page, label)).toHaveAttribute('aria-current', 'false');
}
/**
* Open an artist's detail view, which is the deepest ordinary route.
*
@@ -71,6 +108,105 @@ test.describe('the back gesture', () => {
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
});
test('leaves the nav highlighting the view it landed on, not the one it left', async ({
app,
}) => {
await app.getByTestId('nav-albums').click();
await expectHighlighted(app, 'Albums');
await app.getByTestId('nav-tracks').click();
await expectHighlighted(app, 'Tracks');
await app.goBack();
// #72, and the half of it the report did not describe: this is
// desktop, and before the shell published the active view *both*
// navs stayed on Tracks. An absent highlight reads as a glitch; a
// confident wrong one is worse, and any back across two primary
// views produced it.
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
await expectHighlighted(app, 'Albums');
await expectNotHighlighted(app, 'Tracks');
});
test('keeps the parent destination lit while a detail view is open', async ({
app,
}) => {
await app.getByTestId('nav-artists').click();
await expectHighlighted(app, 'Artists');
await openAnArtist(app);
// A detail view is not a destination in either nav, and the user is
// still inside Artists. `app-sidebar` did this by accident -- it
// guarded on its own item list, so an unmatched name left the
// highlight alone -- and that accident is why the sidebar looked
// right on a detail view while the tab bar lit nothing. This test
// therefore passed before the fix and is here to keep the rule from
// being lost while the others are made to pass; the *tab bar's*
// half of it is the phone test below, which did not.
await expectHighlighted(app, 'Artists');
await app.goBack();
await expectHighlighted(app, 'Artists');
});
test('the tab bar survives the same journey on a phone', async ({ app }) => {
await app.setViewportSize(PHONE);
// The reported shape: Albums, open an album, press back. The tab
// bar had a highlight, then no highlight at all, and never got it
// back — `bottom-nav` took the detail view's name, matched it
// against no tab, and lit nothing.
await navItem(app, 'Albums').click();
await expectHighlighted(app, 'Albums');
await app.locator('cover-grid').getByText('Glass Harbour').first().click();
await expect(activeView(app)).toHaveAttribute(
'data-active-view',
'explore-album-details',
);
await expectHighlighted(app, 'Albums');
await app.goBack();
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
await expectHighlighted(app, 'Albums');
});
test('the drawer sidebar opens on the page you are standing on', async ({
app,
}) => {
await app.setViewportSize(PHONE);
await navItem(app, 'Tracks').click();
await expectHighlighted(app, 'Tracks');
// A third symptom of the same cause, found while measuring #72 and
// not in the report: `bottom-nav` mounts its `<app-sidebar>` when
// the drawer opens, so that copy had heard no `navigate` at all and
// showed its own default — Home, from any page in the app. An event
// has no answer for a listener that was not there; a store does.
await navItem(app, 'More').click();
// The element carrying the testid is the `wa-drawer` host, which
// always reports hidden -- what is visible is the `<dialog>` in its
// shadow root -- so the drawer being open is asserted of the
// sidebar it holds rather than of itself.
const drawer = app.getByTestId('nav-drawer');
await expect(drawer.locator('app-sidebar')).toBeVisible();
await expect(drawer.getByTestId('nav-tracks')).toHaveAttribute(
'aria-current',
'page',
);
await expect(drawer.getByTestId('nav-home')).toHaveAttribute(
'aria-current',
'false',
);
});
test('an in-app back button consumes exactly one entry', async ({ app }) => {
await app.getByTestId('nav-tracks').click();
await openAnArtist(app);
+318
View File
@@ -0,0 +1,318 @@
import { test, expect } from '../support/fixtures.js';
/**
* #69: the Playlists header's buttons could not be reached.
*
* Three text buttons — Import (91px), New Playlist (122px), New Smart
* Playlist (162px), 390px in total — inside a header that gets 700px at
* 900×600. "New Smart Playlist" rendered **114 of its 162px**, and at
* phone width the Android report was the plain version of it: you
* cannot scroll to reach them, and scrolling is not how page controls
* should be exposed anyway.
*
* **`layout-overflow.spec.ts` passes on the broken build**, which is why
* this file exists rather than a case being added there. That spec
* asserts the *shell* needs no sideways scrolling; clipping *inside* a
* component is invisible to it. So the measurement here is per-button
* and per-header, against the widths the app promises.
*
* Plan 018's size matrix is the promise being kept: **no action is ever
* unreachable at any supported size.** These are its three bands.
*/
const VIEWPORTS = [
// Desktop's worst case, and not the enforced minimum: the sidebar
// collapses to icons *below* 900, so the content area is 843px at 899
// and 700px at 900. Testing "the minimum" and stopping misses it.
{ name: '900×600 (widest sidebar, narrowest content)', width: 900, height: 600 },
{ name: '800×600 (the enforced minimum)', width: 800, height: 600 },
{ name: '390×780 (phone)', width: 390, height: 780 },
// WCAG 1.4.10's reflow target, which plan 018 promises the app fits.
{ name: '320×600 (400% zoom)', width: 320, height: 600 },
];
/** Every action the Playlists header can offer, in declared order. */
const ACTIONS = ['Import', 'New Playlist', 'New Smart Playlist'];
/**
* What the header is actually rendering, measured rather than inferred.
*
* A shadow query is the wrong tool for *asserting* — that is what
* `getByRole` below is for — but it is the right one for a measurement,
* because the number this issue is about (a button 48px wider than the
* box holding it) is not in the accessibility tree at all.
*/
const headerFit = (page: import('@playwright/test').Page) =>
page.evaluate(() => {
const root = document
.querySelector('[data-testid="main-content"] playlist-view')
?.shadowRoot?.querySelector('page-header')?.shadowRoot;
if (!root) return null;
const header = root.querySelector<HTMLElement>('.page-header')!;
const box = header.getBoundingClientRect();
const title = root.querySelector<HTMLElement>('h1')!;
const clipped = [
...root.querySelectorAll<HTMLElement>('.action, .more-button'),
]
.filter((b) => !b.hidden)
.filter((b) => {
const r = b.getBoundingClientRect();
return r.right > box.right + 1 || r.left < box.left - 1;
})
.map((b) => b.dataset['actionId'] ?? 'more');
return {
overflow: header.scrollWidth - header.clientWidth,
clipped,
titleTruncated: title.scrollWidth > title.clientWidth + 1,
buttons: [...root.querySelectorAll<HTMLElement>('.action')]
.filter((b) => !b.hidden)
.map((b) => b.textContent?.trim() ?? ''),
menu: [
...root.querySelectorAll('#page-header-overflow wa-dropdown-item'),
].map((i) => i.textContent?.trim() ?? ''),
};
});
test.describe('the page header never clips an action', () => {
test.beforeEach(async ({ app }) => {
await app.getByTestId('nav-playlists').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'playlists',
);
});
test.afterEach(async ({ app }) => {
await app.setViewportSize({ width: 1280, height: 800 });
});
for (const vp of VIEWPORTS) {
test(`every action is reachable at ${vp.name}`, async ({ app }) => {
await app.setViewportSize({ width: vp.width, height: vp.height });
// Polled: the fit is decided by a ResizeObserver, so it settles a
// frame after the resize rather than with it.
await expect
.poll(async () => (await headerFit(app))?.clipped)
.toEqual([]);
const fit = (await headerFit(app))!;
expect(fit.overflow).toBeLessThanOrEqual(0);
// Between them, buttons and menu account for all three. This is
// the assertion the issue asks for: not "it fits" but "nothing
// was dropped to make it fit".
expect([...fit.buttons, ...fit.menu].sort()).toEqual([...ACTIONS].sort());
});
}
/**
* The title gives way before an action does.
*
* Once the heading can ellipsis it absorbs the pressure, and
* `scrollWidth` then reports a header that fits perfectly while the
* heading reads "Playlis…" — this issue's own failure mode moved from
* the button to the title, and invisible to exactly the measurement
* that missed it the first time. At the desktop sizes there is always
* an action to collapse instead.
*/
test('does not truncate the heading to keep a button', async ({ app }) => {
for (const vp of VIEWPORTS.slice(0, 2)) {
await app.setViewportSize({ width: vp.width, height: vp.height });
await expect
.poll(async () => (await headerFit(app))?.titleTruncated)
.toBe(false);
}
});
/**
* Asserted through the accessibility tree, never a shadow query. An
* overflow menu is exactly the shape that grows a nameless control,
* and this repo has shipped one four times — most recently the
* queue's own close button.
*/
test('the overflow is a named control that opens a named menu', async ({
app,
}) => {
await app.setViewportSize({ width: 900, height: 600 });
const more = app.getByRole('button', { name: 'More actions' });
await expect(more).toBeVisible();
await expect(more).toHaveAttribute('aria-expanded', 'false');
await more.click();
await expect(more).toHaveAttribute('aria-expanded', 'true');
const menu = app.getByRole('menu', { name: 'More actions' });
await expect(menu).toBeVisible();
// Collapsed at 900×600: Import (lowest priority) and New Smart
// Playlist. New Playlist stays a button because it is the drop
// target, and a closed menu cannot be one.
await expect(
menu.getByRole('menuitem', { name: 'Import' }),
).toBeVisible();
await expect(
app.getByRole('button', { name: 'New Playlist', exact: true }),
).toBeVisible();
});
/**
* The phone case is the original report. Every action is in the menu
* at 390px, and the menu is reachable by name — which is the whole of
* "these need to be reachable in a sensible way".
*/
test('offers every action from the menu on a phone', async ({ app }) => {
await app.setViewportSize({ width: 390, height: 780 });
const more = app.getByRole('button', { name: 'More actions' });
await expect(more).toBeVisible();
await more.click();
const menu = app.getByRole('menu', { name: 'More actions' });
for (const label of ACTIONS) {
await expect(menu.getByRole('menuitem', { name: label })).toBeVisible();
}
});
/**
* Escape closes it and focus goes back to the trigger — `MenuKeyboard`
* is shared with every other menu in the app precisely so this is not
* a second keyboard model, and this is what proves it was wired up
* rather than merely imported.
*/
test('takes the keyboard, and gives it back', async ({ app }) => {
await app.setViewportSize({ width: 900, height: 600 });
const more = app.getByRole('button', { name: 'More actions' });
await more.click();
const menu = app.getByRole('menu', { name: 'More actions' });
await expect(menu).toBeVisible();
// The first item takes focus on open. `wa-dropdown-item` sets its
// own role in its own first update, so this is polled rather than
// read: a query at the host's updateComplete finds nothing, which
// reads exactly like a menu that refused to take focus.
await expect
.poll(async () =>
app.evaluate(() => {
// Stops where `MenuKeyboard`'s own `deepActiveElement` stops:
// on the *host* whose shadow root has no active element.
// Descending unconditionally lands inside the focused
// `wa-dropdown-item`'s own shadow root, where nothing is
// focused — which reads exactly like a menu that refused the
// keyboard, on a build where it did not.
let el = document.activeElement;
while (el?.shadowRoot?.activeElement) el = el.shadowRoot.activeElement;
return el?.textContent?.trim() ?? null;
}),
)
.toBe('Import');
await app.keyboard.press('Escape');
await expect(more).toHaveAttribute('aria-expanded', 'false');
await expect(more).toBeFocused();
});
/**
* New Playlist is a drop target, and declaring it as data must not
* take that away — which is why a `PageAction` carries the drop
* handlers rather than the header owning a notion of dropping.
*
* Nothing covered this before, in either tier, and it is the one
* behaviour the migration could plausibly have destroyed silently:
* dragging still *looks* fine against a button that no longer
* accepts anything.
*/
test('New Playlist still accepts a dropped track', async ({ app }) => {
await app.setViewportSize({ width: 1280, height: 800 });
const button = app.getByRole('button', {
name: 'New Playlist',
exact: true,
});
await expect(button).toBeVisible();
const result = await app.evaluate(async () => {
const view = document.querySelector(
'[data-testid="main-content"] playlist-view',
)!;
const target = view.shadowRoot!
.querySelector('page-header')!
.shadowRoot!.querySelector('[data-testid="page-action-new-playlist"]')!;
const data = new DataTransfer();
data.setData(
'application/x-yj-tracks',
JSON.stringify({ filePaths: ['/tmp/dropped.mp3'] }),
);
const fire = (type: string) =>
target.dispatchEvent(
new DragEvent(type, {
bubbles: true,
cancelable: true,
dataTransfer: data,
}),
);
fire('dragover');
await new Promise((r) => setTimeout(r, 50));
// The affordance is the host's state reaching the header's
// button, which is the half a plain handler call would not prove.
const highlighted = target.classList.contains('drag-over');
fire('drop');
await new Promise((r) => setTimeout(r, 200));
return {
highlighted,
opened: view.shadowRoot!.querySelector('.create-form') !== null,
};
});
expect(result).toEqual({ highlighted: true, opened: true });
// Leave the view as it was found.
await app.keyboard.press('Escape');
});
/**
* An action given back when the window widens again. The collapsed
* set is a function of the current width and not of how it got there
* — a rule that only ever *added* to it would never widen.
*/
test('gives the buttons back when the window grows', async ({ app }) => {
await app.setViewportSize({ width: 390, height: 780 });
await expect.poll(async () => (await headerFit(app))?.buttons).toEqual([]);
await app.setViewportSize({ width: 1440, height: 900 });
await expect
.poll(async () => (await headerFit(app))?.buttons)
.toEqual(ACTIONS);
await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]);
});
});
+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 {
+15
View File
@@ -40,6 +40,7 @@ import { setBasePath } from '@awesome.me/webawesome/dist/webawesome.js';
import { registerBundledIcons } from './src/icons';
import { queueStore } from '@store/queue-store';
import { searchStore } from '@store/search-store';
import { activeViewStore } from '@store/active-view-store';
import * as Player from '@go/player/player.js';
import * as Queue from '@go/queue/queue.js';
import { GetDefaultPage } from '@go/config/config.js';
@@ -279,6 +280,20 @@ async function handleNavigate(
// attribute keeps e2e selectors semantic instead of structural.
mainContent.dataset.activeView = view;
// And publishing it as a *value* is what the nav components read.
// They used to learn the active view from the `navigate` event,
// which only the outbound path dispatches -- so a back-navigation
// left both of them highlighting the view it had just left (#72).
// Re-dispatching `navigate` here is not the fix: this file is a
// document listener for it, so that is an infinite loop, and
// "please go to X" is not the statement being made.
//
// `view in VIEW_TAGS` is the primary/detail split, and it is passed
// rather than re-derived because this table is where it is written
// down. A detail view therefore leaves the tab it was opened from
// lit, which is what the report asks for.
activeViewStore.setView(view, view in VIEW_TAGS);
// --- Primary (cacheable) views ----------------------------------------
if (view in VIEW_TAGS) {
// Remove any active detail view first
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M0 256a56 56 0 1 1 112 0 56 56 0 1 1 -112 0zm168 0a56 56 0 1 1 112 0 56 56 0 1 1 -112 0zm224-56a56 56 0 1 1 0 112 56 56 0 1 1 0-112z"/></svg>

After

Width:  |  Height:  |  Size: 443 B

@@ -7,6 +7,7 @@ import { designTokens } from '../../styles/tokens.css';
import '../sidebar/app-sidebar.js';
import { nameDialog } from '@utils/name-dialog';
import { ICON_PLAYLIST } from '@utils/icon-language';
import { ActiveViewController } from '@store/controllers/active-view-controller';
type View = 'home' | 'albums' | 'tracks' | 'playlists';
@@ -114,8 +115,19 @@ export class BottomNav extends LitElement {
}
`];
@state()
private activeView = 'home';
/**
* Which tab is lit, read from the shell rather than tracked here.
*
* This was a `@state()` field set from the `navigate` event, which
* only the outbound path dispatches -- so backing out of a detail
* view left the highlight wherever it had been (#72). It had no
* equivalent of `app-sidebar`'s `navItems.some(...)` guard either,
* so a detail view set it to a name matching no tab and *nothing*
* was lit; that asymmetry is why one nav looked broken and the
* other looked fine. The store answers both: a detail view leaves
* the tab it was opened from lit, in both components.
*/
private activeCtrl = new ActiveViewController(this);
/**
* Whether the drawer has been asked for.
@@ -167,12 +179,9 @@ export class BottomNav extends LitElement {
nameDialog(this.drawer);
}
private onGlobalNavigate = (e: Event) => {
const detail = (e as CustomEvent<{ view?: string }>).detail;
if (detail?.view) this.activeView = detail.view;
private onGlobalNavigate = () => {
// A navigation from inside the drawer is the drawer's job done.
// The highlight is not this listener's business any more.
this.drawerOpen = false;
};
@@ -206,9 +215,11 @@ export class BottomNav extends LitElement {
<li>
<button
type="button"
class=${this.activeView === tab.id ? 'active' : ''}
class=${this.activeCtrl.isActive(tab.id)
? 'active'
: ''}
data-testid="tab-${tab.id}"
aria-current=${this.activeView === tab.id
aria-current=${this.activeCtrl.isActive(tab.id)
? 'page'
: 'false'}
@click=${() => this.navigate(tab.id)}
@@ -3,6 +3,7 @@ import { customElement, state } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/button/button.js';
import '@components/page-header/page-header';
import type { PageAction } from '@components/page-header/page-header';
import { designTokens } from '../../styles/tokens.css';
import { downloadStore, stateLabel } from '@store/download-store';
import type { Request, RequestSummary, DownloadView as DownloadRecord } from '@store/download-store';
@@ -246,23 +247,23 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
override render() {
return html`
<page-header heading="Downloads">
${this.tab === 'requests'
? html`
<wa-button
slot="actions"
size="small"
appearance="outlined"
?disabled=${this.checking}
title="Search every download client for everything on this list right now, instead of waiting for the next scheduled check"
@click=${() => void this.checkNow()}
>
<wa-icon slot="start" name="rotate"></wa-icon>
${this.checking ? 'Searching…' : 'Check now'}
</wa-button>
`
: nothing}
</page-header>
<page-header
heading="Downloads"
.actions=${this.tab === 'requests'
? ([
{
id: 'check-now',
label: this.checking
? 'Searching\u2026'
: 'Check now',
icon: 'rotate',
disabled: this.checking,
title: 'Search every download client for everything on this list right now, instead of waiting for the next scheduled check',
onSelect: () => void this.checkNow(),
},
] satisfies PageAction[])
: []}
></page-header>
<p class="subtitle">
Music you have requested, and the download attempts that
+63 -38
View File
@@ -1,8 +1,9 @@
import { LitElement, html, css, nothing } from 'lit';
import { customElement, state } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/button/button.js';
import { GetShelves } from '@go/home/service.js';
import { ICON_SHUFFLE } from '@utils/icon-language';
import type { PageAction } from '@components/page-header/page-header';
import { GetAlbumTracks } from '@go/library/library.js';
import type * as home from '@go/home/models.js';
import type * as library from '@go/library/models.js';
@@ -160,29 +161,52 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
user-select: none;
}
/*
* The hover play button is a *hover* affordance, so it is
* gated on the device having hover rather than on width. A
* touch long-press synthesises a hover state in the WebView,
* so on a phone it flashed into view during the 500ms hold
* that utils/long-press.ts is measuring for a context menu
* a control appearing because you were reaching for a
* different one. A phone user taps the album and plays from
* the detail view, so there is nothing to replace it with.
*
* display:none outside the query rather than opacity:0 on
* its own: an opacity-0 button still takes taps and is
* still in the accessibility tree, so the invisible control
* would keep the hit area it was never meant to have on
* touch. Everything else stays inside, so the desktop
* animation is unchanged.
*/
.play {
position: absolute;
right: 8px;
bottom: 8px;
width: 38px;
height: 38px;
border: none;
border-radius: 50%;
background: var(--yj-accent, #ffd43b);
color: var(--yj-accent-fg, #000);
display: flex;
align-items: center;
justify-content: center;
cursor: pointer;
opacity: 0;
transform: translateY(6px);
transition: opacity 0.12s ease, transform 0.12s ease;
display: none;
}
.card:hover .play,
.card:focus-within .play {
opacity: 1;
transform: translateY(0);
@media (hover: hover) and (pointer: fine) {
.play {
position: absolute;
right: 8px;
bottom: 8px;
width: 38px;
height: 38px;
border: none;
border-radius: 50%;
background: var(--yj-accent, #ffd43b);
color: var(--yj-accent-fg, #000);
display: flex;
align-items: center;
justify-content: center;
cursor: pointer;
opacity: 0;
transform: translateY(6px);
transition: opacity 0.12s ease, transform 0.12s ease;
}
.card:hover .play,
.card:focus-within .play {
opacity: 1;
transform: translateY(0);
}
}
.name {
@@ -260,23 +284,24 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
override render() {
return html`
<page-header heading="Home">
<!-- "Shuffle" alone was two different controls with one
name: this one and the transport's shuffle mode.
They were never on screen together until the app
started landing on Home (H-8), and a cached view is
in the accessibility tree either way. -->
<wa-button
slot="actions"
size="small"
appearance="plain"
title="Reshuffle the suggestions"
@click=${() => void this.load()}
>
<wa-icon slot="start" name="shuffle"></wa-icon>
Shuffle suggestions
</wa-button>
</page-header>
<page-header
heading="Home"
.actions=${[
{
// "Shuffle" alone was two different controls
// with one name: this one and the transport's
// shuffle mode. They were never on screen
// together until the app started landing on
// Home (H-8), and a cached view is in the
// accessibility tree either way.
id: 'shuffle-suggestions',
label: 'Shuffle suggestions',
icon: ICON_SHUFFLE,
title: 'Reshuffle the suggestions',
onSelect: () => void this.load(),
},
] satisfies PageAction[]}
></page-header>
<p class="lede">Somewhere to start listening.</p>
${this.renderBody()}
`;
@@ -13,6 +13,7 @@ import {
isQueueSourceNavigable,
navigateToQueueSource,
} from '@utils/queue-source-link';
import { PHONE_QUERY } from '@utils/breakpoints';
import { PlayerController } from '@store/controllers/player-controller';
import { creditStore } from '@store/credit-store';
import { QueueController } from '@store/controllers/queue-controller';
@@ -80,6 +81,19 @@ export class NowPlaying extends LitElement {
private reduceMotionQuery?: MediaQueryList;
/**
* Phone width, from the shell's own breakpoint.
*
* This is in JS rather than in the stylesheet because what changes
* is the *content*, not its appearance: the title, artist and
* source render as plain text instead of as links, and no CSS rule
* can take a click handler off an element.
*/
@state()
private phone = false;
private phoneQuery?: MediaQueryList;
/** Whether each field is actively mid-scroll (class toggle). */
@state()
private titleScrolling = false;
@@ -341,6 +355,12 @@ export class NowPlaying extends LitElement {
this.reduceMotion = this.reduceMotionQuery?.matches ?? false;
this.reduceMotionQuery?.addEventListener('change', this.handleReduceMotionChange);
// Same reasoning as above: looked up here, not at module load,
// so a test can install its own matchMedia first.
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
this.phone = this.phoneQuery?.matches ?? false;
this.phoneQuery?.addEventListener('change', this.handlePhoneChange);
this.resizeObserver = new ResizeObserver(() => {
this.geometryDirty = true;
this.requestUpdate();
@@ -364,6 +384,7 @@ export class NowPlaying extends LitElement {
this.attachDragListeners(false);
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
this.phoneQuery?.removeEventListener('change', this.handlePhoneChange);
this.resizeObserver?.disconnect();
this.stopScrollCycle('title');
this.stopScrollCycle('artist');
@@ -488,7 +509,7 @@ export class NowPlaying extends LitElement {
@mouseleave=${this.handleTitleMouseLeave}
@transitionend=${() => this.onScrollCycleEnd('title')}
>
<span class="scroll-content">${trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
<span class="scroll-content">${this.phone ? track.title : trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
</span>
<span
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
@@ -498,14 +519,15 @@ export class NowPlaying extends LitElement {
@mouseleave=${this.handleArtistMouseLeave}
@transitionend=${() => this.onScrollCycleEnd('artist')}
>
<span class="scroll-content">${creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
<span class="scroll-content">${this.phone ? track.artist || 'Unknown Artist' : creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
</span>
${describeQueueSource(this.queue.source)
? html`
<span
class="track-source ${isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
class="track-source ${!this.phone && isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
data-testid="now-playing-source"
@click=${(e: MouseEvent) => {
if (this.phone) return;
if (!isQueueSourceNavigable(this.queue.source)) return;
navigateToQueueSource(
e.currentTarget as EventTarget,
@@ -571,6 +593,10 @@ export class NowPlaying extends LitElement {
this.reduceMotion = e.matches;
};
private handlePhoneChange = (e: MediaQueryListEvent): void => {
this.phone = e.matches;
};
private shouldScroll(field: 'title' | 'artist'): boolean {
const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
@@ -606,6 +632,12 @@ export class NowPlaying extends LitElement {
track?.artist ?? '',
this.shouldScroll('title') ? '1' : '0',
this.shouldScroll('artist') ? '1' : '0',
// Crossing the breakpoint swaps a link for a bare string,
// and a link is not guaranteed to measure the same as the
// text inside it. The marquee travels a distance read from
// that measurement, so this belongs in the key even though
// the words are identical either side.
this.phone ? '1' : '0',
].join('\u0000');
}
@@ -1,8 +1,16 @@
import { LitElement, html, css, nothing } from 'lit';
import { customElement, property } from 'lit/decorators.js';
import { customElement, property, query, state } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
import { designTokens } from '../../styles/tokens.css';
import {
MenuKeyboard,
contextMenuStyles,
} from '../../utils/context-menu-controller';
import { ICON_MORE_ACTIONS } from '../../utils/icon-language';
/**
* The one arrangement every primary view uses to say what it is.
@@ -18,6 +26,19 @@ import { designTokens } from '../../styles/tokens.css';
* Title, count, sort, actions in that order, in one component, so a
* new view gets the shape by using it rather than by copying whichever
* neighbour it happened to read.
*
* **Actions are data, and `<slot name="actions">` is the exception.**
* Playlists' three buttons totalled 390px inside a header that gets
* 700px at 900×600 and clipped "New Smart Playlist" to 114 of its 162
* (#69) a live defect at a size the app promises, against plan 018's
* *no action is ever unreachable at any supported size*. The header
* cannot fix that for slotted markup: it cannot move another
* component's light-DOM children into a dropdown and keep their
* behaviour, and arbitrary markup offers nothing generic to render as
* a menu item. So a host declares `PageAction[]` and the header picks
* the rendering. The slot survives for markup a data list genuinely
* cannot express, at the stated cost that **a slotted action does not
* collapse** and must therefore fit at 800×600.
*/
export interface SortOption {
@@ -27,6 +48,50 @@ export interface SortOption {
export type SortDirection = 'asc' | 'desc';
/**
* An action that only makes sense while it is a button.
*
* A drop target is the case: you cannot drag a track onto a closed
* menu, so the affordance is absent from the overflow rather than
* approximated there. The header wires these onto the button it
* renders and owns none of them the same division the sort control
* already lives by.
*/
export interface PageActionDrop {
/** True while an acceptable payload is over the button. */
active?: boolean;
onDragOver: (e: DragEvent) => void;
onDragLeave: (e: DragEvent) => void;
onDrop: (e: DragEvent) => void;
}
/**
* One thing a view can do, as data rather than as markup.
*
* `<slot name="actions">` cannot be collapsed, and that is a fact about
* the API rather than an effort estimate (#69): a component cannot move
* another component's light-DOM children into a dropdown and keep their
* behaviour, and there is nothing generic in arbitrary markup to render
* as a menu item. Declaring an action instead is what lets the header
* choose between the two renderings.
*/
export interface PageAction {
id: string;
label: string;
/** From `utils/icon-language`, never a literal. */
icon: string;
onSelect: () => void;
/**
* Higher survives longer. The lowest collapses first, ties broken
* by declaration order from the right, so a host that says nothing
* gets "the last one written goes first".
*/
priority?: number;
disabled?: boolean;
title?: string;
drop?: PageActionDrop;
}
@customElement('page-header')
export class PageHeader extends LitElement {
/**
@@ -80,8 +145,82 @@ export class PageHeader extends LitElement {
@property({ type: Boolean })
busy = false;
/**
* What this view can do, in the order it wants them shown.
*
* The header decides what *fits*; the host decides what *happens*.
* That is the rule the sort control already lives by it asks for
* a sort rather than performing one and actions follow it, which
* is why an action carries a handler rather than the header
* carrying a verb it would have to interpret.
*/
@property({ attribute: false })
actions: PageAction[] = [];
/** Action ids currently in the overflow menu. Derived, never set by a host. */
@state()
private collapsed: ReadonlySet<string> = new Set();
@state()
private menuOpen = false;
@query('.page-header')
private headerEl?: HTMLElement;
@query('.more-button')
private moreButton?: HTMLButtonElement;
@query('#page-header-overflow')
private menuPanel?: HTMLElement;
@query('wa-popup')
private popup?: WaPopup;
private menuKeyboard = new MenuKeyboard(() => this.closeMenu());
private resizeObserver?: ResizeObserver;
/**
* Whether the outside-click listener is attached.
*
* A `removeEventListener` with no matching `add` is not harmless
* here: `view-lifecycle.test.ts` counts document listeners across a
* view's life and an unconditional detach on disconnect shows up as
* `held: -1`, which is the same accounting that would hide a real
* leak in the other direction.
*/
private outsideCloseAttached = false;
/**
* What the last fit was measured against.
*
* `updated()` runs on every pass, so it has to say what it depends
* on or it re-measures and a measurement here forces synchronous
* layout. Width changes arrive through the ResizeObserver; this key
* covers everything *else* in the flex row that can change how much
* of it the actions are left.
*/
private lastFitKey = '';
override connectedCallback(): void {
super.connectedCallback();
this.resizeObserver = new ResizeObserver(() => this.measureFit());
this.resizeObserver.observe(this);
}
override disconnectedCallback(): void {
super.disconnectedCallback();
this.resizeObserver?.disconnect();
this.resizeObserver = undefined;
this.detachOutsideClose();
}
static override styles = [
designTokens,
contextMenuStyles,
css`
:host {
display: block;
@@ -96,12 +235,26 @@ export class PageHeader extends LitElement {
border-bottom: 1px solid var(--yj-border-subtle, #333);
}
/* The title gives way before an action does.
Everything in this row was flex-shrink: 0, so whatever
came last lost and the actions come last, which is how
the "More actions" button ended up 76px off the right
edge of a 320px viewport with every action already
collapsed into it. The title is the one thing here the
navigation also says (the sidebar item is selected, the
bottom-nav tab is current), so it is the cheapest thing
to truncate; the count, the sort and the actions are each
the only place they are said. */
h1 {
margin: 0;
font-size: var(--yj-text-xl, 18px);
font-weight: 600;
color: var(--yj-text-primary, #fff);
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
min-width: 0;
}
.count {
@@ -191,6 +344,98 @@ export class PageHeader extends LitElement {
::slotted(*) {
flex-shrink: 0;
}
.actions {
display: flex;
align-items: center;
gap: 8px;
flex-shrink: 0;
}
.action,
.more-button {
background: none;
border: 1px solid var(--yj-border-subtle, #555);
border-radius: 4px;
color: var(--yj-text-primary, #fff);
padding: 6px 12px;
font-size: var(--yj-text-md, 13px);
font-family: inherit;
cursor: pointer;
display: flex;
align-items: center;
gap: 6px;
white-space: nowrap;
flex-shrink: 0;
}
.more-button {
padding: 6px 10px;
}
/* The display: flex above outranks the UA stylesheet's
rule for [hidden], and hiding is how an action
collapses. (No backticks in here: one ends the css
literal, and what you get is "css(...) is not a
function" a long way from the cause.) */
.action[hidden],
.more-button[hidden] {
display: none;
}
.action:hover,
.more-button:hover,
.action.drag-over {
border-color: var(--yj-accent, #ffd43b);
color: var(--yj-accent-text, #ffd43b);
}
.action.drag-over {
background-color: var(
--yj-accent-bg-strong,
rgba(255, 212, 59, 0.15)
);
}
.action:disabled {
opacity: 0.5;
cursor: default;
}
.action:focus-visible,
.more-button:focus-visible {
outline: 2px solid var(--yj-accent, #ffd43b);
outline-offset: -1px;
}
wa-popup {
z-index: 200;
}
/* A component states what it drops at phone width itself,
in its own stylesheet, because a media query inside a
shadow root is answered by the viewport and the shell
cannot reach in. Here that is one word: the sort control
is 172px of a 320px header, and "Sort:" is ~40px of it
for a label the adjacent direction arrow already implies.
It stays in the accessibility tree it is the select's
accessible name, so hiding it outright would rename the
control to nothing which is config-field's bug, one
component over. clip-path rather than display: none for
the reason styles/sr-only.css.ts gives. */
@media (max-width: 599px) {
.sort-label {
position: absolute;
width: 1px;
height: 1px;
margin: -1px;
padding: 0;
overflow: hidden;
clip-path: inset(50%);
white-space: nowrap;
border: 0;
}
}
`,
];
@@ -212,11 +457,293 @@ export class PageHeader extends LitElement {
${this.renderCount()}
<div class="spacer"></div>
${this.renderScope()} ${this.renderSort()}
${this.renderActions()}
<slot name="actions"></slot>
</header>
`;
}
protected override updated(): void {
const key = [
this.heading,
this.count,
this.countNoun,
this.countPlural,
this.searchTerm,
this.sortOptions.length,
this.sortField,
this.sortDirection,
this.busy,
this.actions.map((a) => `${a.id}:${a.label}:${a.disabled ?? false}`).join(','),
].join('|');
if (key === this.lastFitKey) return;
this.lastFitKey = key;
this.measureFit();
}
// =================================================================
// What fits
// =================================================================
/**
* Decide which actions are buttons and which are menu items.
*
* Two things about the shape of this are load-bearing.
*
* **Every pass starts from all-visible**, so the collapsed set is a
* pure function of the current width rather than of the order the
* widths arrived in. A rule that only ever *added* to the set would
* never give an action back when the window grew, and one that
* adjusted by a step would need a hysteresis band to stop it
* oscillating on the pixel where a button exactly fits.
*
* **It flips `hidden` on the rendered nodes rather than re-rendering
* between steps.** Reading `scrollWidth` forces layout, which is the
* point; awaiting a Lit update between steps instead would let the
* intermediate all-visible state paint, so the fix would flash the
* overflow it exists to prevent. The reactive state is set once, at
* the end, and the next render agrees with what was measured.
*
* The budget is the *header's* overflow and not the actions row's,
* because the count and the sort control are `flex-shrink: 0` and
* are therefore competing for the same width only `.scope` gives
* way, which is what it has an ellipsis for.
*/
private measureFit(): void {
const header = this.headerEl;
if (!header) return;
if (this.actions.length === 0) {
this.commitCollapsed(new Set());
return;
}
const buttons = new Map<string, HTMLElement>();
for (const el of this.renderRoot.querySelectorAll<HTMLElement>(
'[data-action-id]',
)) {
const id = el.dataset['actionId'];
if (id !== undefined) buttons.set(id, el);
}
const more = this.moreButton;
const title = this.renderRoot.querySelector('h1');
/**
* Nothing is clipped which is not the same as the header not
* overflowing, and the difference is a trap worth naming.
*
* Once the title can ellipsis, it absorbs the pressure and
* `scrollWidth` reports a header that fits perfectly while the
* heading reads "Playlis…". That is this issue's own failure
* mode moved from the button to the title, and it is invisible
* to exactly the same measurement that missed it the first time.
* So the title's own truncation counts as not fitting, and
* collapsing an action is tried before the title gives way.
*/
const fits = () =>
header.scrollWidth <= header.clientWidth &&
(title === null || title.scrollWidth <= title.clientWidth + 1);
for (const el of buttons.values()) el.hidden = false;
if (more) more.hidden = true;
const collapsed = new Set<string>();
if (!fits()) {
if (more) more.hidden = false;
for (const action of this.collapseOrder()) {
collapsed.add(action.id);
const el = buttons.get(action.id);
if (el) el.hidden = true;
if (fits()) break;
}
}
this.commitCollapsed(collapsed);
}
/** Lowest priority first; ties broken from the right. */
private collapseOrder(): PageAction[] {
return this.actions
.map((action, index) => ({ action, index }))
.sort(
(a, b) =>
(a.action.priority ?? 0) - (b.action.priority ?? 0) ||
b.index - a.index,
)
.map(({ action }) => action);
}
private commitCollapsed(next: Set<string>): void {
const same =
next.size === this.collapsed.size &&
[...next].every((id) => this.collapsed.has(id));
if (same) return;
this.collapsed = next;
// Nothing left to show in it. Closing rather than leaving an
// empty menu open is the same rule the shelves follow.
if (next.size === 0 && this.menuOpen) this.closeMenu();
}
// =================================================================
// Rendering
// =================================================================
private renderActions() {
if (this.actions.length === 0) return nothing;
const overflowed = this.actions.filter((a) => this.collapsed.has(a.id));
return html`
<div class="actions">
${this.actions.map((a) => this.renderActionButton(a))}
<wa-popup
placement="bottom-end"
flip
shift
.active=${this.menuOpen}
>
<button
slot="anchor"
class="more-button"
type="button"
data-testid="page-actions-more"
aria-label="More actions"
aria-haspopup="menu"
aria-expanded=${this.menuOpen ? 'true' : 'false'}
aria-controls="page-header-overflow"
?hidden=${overflowed.length === 0}
@click=${this.onMoreClick}
>
<wa-icon name=${ICON_MORE_ACTIONS}></wa-icon>
</button>
<div
id="page-header-overflow"
class="context-menu-panel"
role="menu"
aria-label="More actions"
>
${overflowed.map(
(a) => html`
<wa-dropdown-item
?disabled=${a.disabled ?? false}
@click=${() => this.onActionSelect(a)}
>
<wa-icon
slot="icon"
name=${a.icon}
></wa-icon>
${a.label}
</wa-dropdown-item>
`,
)}
</div>
</wa-popup>
</div>
`;
}
private renderActionButton(a: PageAction) {
const drop = a.drop;
return html`
<button
class="action ${drop?.active === true ? 'drag-over' : ''}"
type="button"
data-action-id=${a.id}
data-testid=${`page-action-${a.id}`}
title=${a.title ?? nothing}
?disabled=${a.disabled ?? false}
?hidden=${this.collapsed.has(a.id)}
@click=${() => a.onSelect()}
@dragover=${(e: DragEvent) => drop?.onDragOver(e)}
@dragleave=${(e: DragEvent) => drop?.onDragLeave(e)}
@drop=${(e: DragEvent) => drop?.onDrop(e)}
>
<wa-icon name=${a.icon}></wa-icon>
${a.label}
</button>
`;
}
// =================================================================
// The overflow menu
// =================================================================
private onActionSelect(a: PageAction): void {
if (a.disabled === true) return;
this.closeMenu();
a.onSelect();
}
private onMoreClick = (): void => {
if (this.menuOpen) {
this.closeMenu();
return;
}
this.menuOpen = true;
void this.updateComplete.then(() => {
if (!this.menuOpen) return;
this.popup?.reposition();
this.menuKeyboard.open(this.menuPanel ?? null, this.moreButton);
this.attachOutsideClose();
});
};
private closeMenu(): void {
if (!this.menuOpen) return;
this.detachOutsideClose();
this.menuKeyboard.close();
this.menuOpen = false;
}
/**
* A click anywhere else closes it. `composedPath` rather than
* `contains`, because the trigger and the panel are both inside
* this shadow root and a click retargets at the host.
*/
private onOutsideDown = (e: Event): void => {
if (e.composedPath().includes(this.menuPanel as EventTarget)) return;
if (e.composedPath().includes(this.moreButton as EventTarget)) return;
this.closeMenu();
};
private attachOutsideClose(): void {
if (this.outsideCloseAttached) return;
this.outsideCloseAttached = true;
document.addEventListener('mousedown', this.onOutsideDown, true);
}
private detachOutsideClose(): void {
if (!this.outsideCloseAttached) return;
this.outsideCloseAttached = false;
document.removeEventListener('mousedown', this.onOutsideDown, true);
}
private renderCount() {
if (this.count === null) return nothing;
@@ -258,7 +785,10 @@ export class PageHeader extends LitElement {
if (this.sortOptions.length === 1) {
return html`
<div class="sort">
<span>Sort: ${this.sortOptions[0]?.label}</span>
<span
><span class="sort-label">Sort: </span
>${this.sortOptions[0]?.label}</span
>
${this.renderDirectionButton(ascending)}
</div>
`;
@@ -267,7 +797,7 @@ export class PageHeader extends LitElement {
return html`
<div class="sort">
<label>
Sort:
<span class="sort-label">Sort:</span>
<select
data-testid="page-sort"
.value=${this.sortField}
@@ -40,7 +40,10 @@ import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/
import {
ICON_NEW,
ICON_PLAYLIST,
ICON_SMART_PLAYLIST,
} from '@utils/icon-language';
import '@components/page-header/page-header';
import type { PageAction } from '@components/page-header/page-header';
const SCROLL_DEBOUNCE_MS = 100;
@@ -194,11 +197,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
contain: layout style;
}
.header-actions {
display: flex;
gap: 8px;
}
.header-spinner {
display: inline-block;
width: 14px;
@@ -209,33 +207,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
animation: spin 0.6s linear infinite;
}
.new-playlist-button {
background: none;
border: 1px solid var(--yj-border-subtle, #555);
border-radius: 4px;
color: var(--yj-text-primary, #fff);
padding: 6px 12px;
font-size: 13px;
cursor: pointer;
display: flex;
align-items: center;
gap: 6px;
font-family: inherit;
}
.new-playlist-button:hover,
.new-playlist-button.drag-over {
border-color: var(--yj-accent, #ffd43b);
color: var(--yj-accent-text, #ffd43b);
}
.new-playlist-button.drag-over {
background-color: var(
--yj-accent-bg-strong,
rgba(255, 212, 59, 0.15)
);
}
.create-form {
display: flex;
align-items: center;
@@ -455,25 +426,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
min-width: 0;
}
.import-button {
background: none;
border: 1px solid var(--yj-border-subtle, #555);
border-radius: 4px;
color: var(--yj-text-primary, #fff);
padding: 6px 12px;
font-size: 13px;
cursor: pointer;
display: flex;
align-items: center;
gap: 6px;
font-family: inherit;
}
.import-button:hover {
border-color: var(--yj-accent, #ffd43b);
color: var(--yj-accent-text, #ffd43b);
}
.import-error {
padding: 0.5em 0.75em;
margin: 0.5em 16px 0;
@@ -1022,10 +974,11 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
) => {
const related =
e.relatedTarget as Node | null;
const btn =
this.shadowRoot?.querySelector(
'.new-playlist-button',
);
// The button the event was bound to, rather than a selector for
// it: `page-header` renders it now, so it is not in this shadow
// root at all and the old `.new-playlist-button` lookup would
// find nothing and leave the highlight stuck on.
const btn = e.currentTarget as Element | null;
if (btn && !btn.contains(related)) {
this.dragOverNewButton = false;
@@ -1470,6 +1423,49 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
this.saveSortPreferences();
};
/**
* The three things this page can do, as data.
*
* The priority order is what #69's Direction asks for and it is
* only interesting for one of them: **New Playlist is highest
* because it is the drop target**. You cannot drag a track onto a
* closed menu, so collapsing it is the one collapse here that
* removes a capability rather than relocating it. Import is lowest
* because it is the rarest, and at 900×600 it is the only one that
* has to go.
*/
private headerActions(): PageAction[] {
return [
{
id: 'import',
label: 'Import',
icon: 'file-import',
priority: 0,
onSelect: () => void this.handleImportPlaylist(),
},
{
id: 'new-playlist',
label: 'New Playlist',
icon: ICON_NEW,
priority: 2,
onSelect: () => this.handleNewPlaylistClick(),
drop: {
active: this.dragOverNewButton,
onDragOver: this.onNewButtonDragOver,
onDragLeave: this.onNewButtonDragLeave,
onDrop: this.onNewButtonDrop,
},
},
{
id: 'new-smart-playlist',
label: 'New Smart Playlist',
icon: ICON_SMART_PLAYLIST,
priority: 1,
onSelect: () => this.handleNewSmartPlaylistClick(),
},
];
}
override render() {
return html`
<page-header
@@ -1484,33 +1480,8 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
search-term=${this.searchCtrl.term}
?busy=${this.refreshing}
@sort-change=${this.onPageHeaderSort}
.actions=${this.headerActions()}
>
<div slot="actions" class="header-actions">
<button
class="import-button"
@click=${this.handleImportPlaylist}
>
<wa-icon name="file-import"></wa-icon>
Import
</button>
<button
class="new-playlist-button ${this.dragOverNewButton ? 'drag-over' : ''}"
@click=${this.handleNewPlaylistClick}
@dragover=${this.onNewButtonDragOver}
@dragleave=${this.onNewButtonDragLeave}
@drop=${this.onNewButtonDrop}
>
<wa-icon name=${ICON_NEW}></wa-icon>
New Playlist
</button>
<button
class="new-playlist-button"
@click=${this.handleNewSmartPlaylistClick}
>
<wa-icon name="filter"></wa-icon>
New Smart Playlist
</button>
</div>
</page-header>
${this.importError
@@ -1765,7 +1736,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
: entry.summary.IsSmart
? html`<wa-icon
class="playlist-icon"
name="filter"
name=${ICON_SMART_PLAYLIST}
></wa-icon>`
: nothing}
${isRenaming
@@ -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>
+23 -29
View File
@@ -4,6 +4,7 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
import { designTokens } from '../../styles/tokens.css';
import type { DragActiveDetail } from '@utils/drag-controller';
import { ActiveViewController } from '@store/controllers/active-view-controller';
import {
ICON_PLAYLIST,
ICON_AUTOTAG,
@@ -159,11 +160,20 @@ export class AppSidebar extends LitElement {
/** Delay in ms before a drag-hover triggers navigation. */
private static readonly HOVER_NAV_DELAY = 600;
/** Home, because that is where `index.ts` now navigates on startup
* (H-8). The sidebar does not hear a `navigate` it did not send,
* so this default is what keeps `aria-current` honest on arrival. */
@state()
private activeView: View = 'home';
/**
* Which item is lit, read from the shell rather than tracked here.
*
* This used to be a `@state()` field defaulting to `home` -- the
* landing view -- because "the sidebar does not hear a `navigate`
* it did not send". That default was the only honest moment it
* ever had: a back-navigation dispatches no `navigate`, so the
* highlight stayed on the view the user had just left (#72), and
* the copy of this component that `bottom-nav` mounts inside its
* drawer opened on `home` from whatever page you were standing on.
* The shell publishes the active view now, so there is nothing to
* default and nothing to keep in step.
*/
private activeCtrl = new ActiveViewController(this);
@state()
private isDragging = false;
@@ -237,10 +247,6 @@ export class AppSidebar extends LitElement {
'yj-drag-active',
this.onDragActive as EventListener,
);
document.addEventListener(
'navigate',
this.onGlobalNavigate as EventListener,
);
}
override disconnectedCallback() {
@@ -262,10 +268,6 @@ export class AppSidebar extends LitElement {
'yj-drag-active',
this.onDragActive as EventListener,
);
document.removeEventListener(
'navigate',
this.onGlobalNavigate as EventListener,
);
this.clearDragHoverTimer();
}
@@ -282,8 +284,9 @@ export class AppSidebar extends LitElement {
<nav aria-label="Main">
<ul>
${this.navItems.map((item) => {
const active = this.activeCtrl.isActive(item.id);
const classes = [
this.activeView === item.id
active
? 'active'
: '',
this.dragHoverView === item.id
@@ -299,7 +302,7 @@ export class AppSidebar extends LitElement {
type="button"
class=${classes}
data-testid="nav-${item.id}"
aria-current=${this.activeView === item.id
aria-current=${active
? 'page'
: 'false'}
@click=${() =>
@@ -382,19 +385,6 @@ export class AppSidebar extends LitElement {
private static readonly DROP_VIEWS: Set<View> =
new Set(['playlists']);
/** Keeps the highlighted nav item in sync with navigation that
* originates outside the sidebar itself (e.g. the launch-page
* dispatch in index.ts). */
private onGlobalNavigate = (
e: CustomEvent<{ view?: string }>,
) => {
const view = e.detail.view;
if (view && this.navItems.some((item) => item.id === view)) {
this.activeView = view as View;
}
};
private onDragActive = (
e: CustomEvent<DragActiveDetail>,
) => {
@@ -460,7 +450,11 @@ export class AppSidebar extends LitElement {
}
private navigate(view: View) {
this.activeView = view;
// No optimistic highlight: the shell answers, and it answers
// synchronously in `handleNavigate` before it awaits anything.
// Setting it here as well is the second opinion this fix
// removes -- it is what let a click's highlight survive a
// navigation the shell then handled differently.
this.dispatchEvent(new CustomEvent('navigate', {
detail: { view },
bubbles: true,
@@ -64,6 +64,7 @@ import { list } from '@utils/binding';
import {
ICON_PLAYLIST,
ICON_QUEUE,
ICON_SMART_PLAYLIST,
} from '@utils/icon-language';
@@ -1214,7 +1215,7 @@ export class SmartPlaylistDetails
<wa-icon name="arrow-left"></wa-icon>
</button>
<div class="playlist-avatar">
<wa-icon name="filter"></wa-icon>
<wa-icon name=${ICON_SMART_PLAYLIST}></wa-icon>
</div>
<div class="playlist-info">
<h1
@@ -11,6 +11,7 @@ import {
import { SelectionController } from '@utils/selection-controller';
import type { SelectionHost } from '@utils/selection-controller';
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
import { PHONE_QUERY } from '@utils/breakpoints';
import {
ContextMenuController,
contextMenuStyles,
@@ -105,9 +106,6 @@ const ROW_CHROME_WIDTH =
const ROW_HEIGHT = 33;
const PHONE_ROW_HEIGHT = 52;
/** The shell's phone breakpoint, as `index.css` and every component
* stylesheet spells it. */
const PHONE_QUERY = '(max-width: 599px)';
// Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM
// overhead (30-50 shadow roots during scroll). Font Awesome 6 paths.
+1
View File
@@ -41,6 +41,7 @@ solid/compact-disc
solid/copy
solid/database
solid/download
solid/ellipsis
solid/file-import
solid/filter
solid/floppy-disk
+90
View File
@@ -0,0 +1,90 @@
/**
* Which primary view the app is showing.
*
* The shell has always known this -- `handleNavigate()` sets
* `#main-content`'s `data-active-view` on every path, `_isBack`
* included -- and never told anyone. The nav components learned it
* from the `navigate` CustomEvent instead, which only the *outbound*
* path dispatches: the `popstate` listener calls `handleNavigate()`
* directly. So both navs kept highlighting the view you had just left
* (#72).
*
* The fix cannot be a re-dispatch of `navigate`. `index.ts` is itself a
* document listener for it, so emitting one from inside
* `handleNavigate` is an infinite loop -- and the two statements are
* different anyway: `navigate` means *please go to X*, and 28 call
* sites across 18 files say it. This says *the active view is now X*,
* which only the shell is in a position to say and only once per
* navigation.
*
* Three things about it are load-bearing.
*
* **It is a store rather than an event**, because a component that
* mounts *after* a navigation still has to know. `bottom-nav`'s "More"
* drawer creates its `<app-sidebar>` on open, and that copy had heard
* no `navigate` at all: standing on Albums, the drawer highlighted
* Home -- its `activeView` default, which existed to match the landing
* view and matched nothing else ever after. An event has no answer for
* a listener that was not there; a value does.
*
* **A detail view is not a view here.** Opening one leaves the primary
* view it was opened from lit, which is what #72 asks for and what
* `app-sidebar` used to do by accident -- it guarded on
* `navItems.some(...)`, so a name matching no item left its highlight
* alone. `bottom-nav` had no such guard and so lit nothing on a detail
* view. Neither was correct; the sidebar was stale-but-lucky, and
* stating the rule once is what makes the two agree.
*
* **Whether a view is primary is the shell's fact, not this store's.**
* `VIEW_TAGS` in `index.ts` is the list, and a copy of it here is a
* second list to forget -- so the caller passes the answer it already
* has rather than this file re-deriving it.
*/
type Subscriber = () => void;
class ActiveViewStore {
/** Empty until the shell's first navigation, which happens at
* startup from `GetDefaultPage()`. Nothing is highlighted for that
* moment, which is honest: the alternative is a written-down
* default that is right only when the default page agrees with it. */
private activeView = '';
private subscribers = new Set<Subscriber>();
/** The active primary view, e.g. `albums`. */
get(): string {
return this.activeView;
}
isActive(view: string): boolean {
return this.activeView !== '' && this.activeView === view;
}
/**
* Called by the shell on every navigation, `popstate` included.
*
* `isPrimary` is `view in VIEW_TAGS` at the call site: a detail
* view reports itself and deliberately changes nothing, so the view
* it was opened from stays lit until the user picks another one.
*/
setView(view: string, isPrimary: boolean): void {
if (!isPrimary) return;
if (view === this.activeView) return;
this.activeView = view;
this.notify();
}
subscribe(fn: Subscriber): () => void {
this.subscribers.add(fn);
return () => this.subscribers.delete(fn);
}
private notify(): void {
this.subscribers.forEach((fn) => fn());
}
}
export const activeViewStore = new ActiveViewStore();
@@ -0,0 +1,59 @@
import type {
ReactiveController,
ReactiveControllerHost,
} from 'lit';
import { activeViewStore } from '../active-view-store';
/**
* ActiveViewController connects a Lit component to the
* ActiveViewStore.
*
* Usage in a component:
*
* private activeCtrl = new ActiveViewController(this);
*
* render() {
* const lit = this.activeCtrl.isActive('albums');
* }
*
* It reads through to the store rather than copying the value into a
* `@state()` field, which is the point of #72: two components holding
* their own idea of the active view is what let them disagree with the
* shell and with each other.
*/
export class ActiveViewController implements ReactiveController {
private host: ReactiveControllerHost;
private unsubscribe?: () => void;
constructor(host: ReactiveControllerHost) {
this.host = host;
host.addController(this);
}
// ===============================================================
// LIFECYCLE HOOKS
// ===============================================================
hostConnected(): void {
this.unsubscribe = activeViewStore.subscribe(() => {
this.host.requestUpdate();
});
}
hostDisconnected(): void {
this.unsubscribe?.();
}
// ===============================================================
// DATA ACCESS
// ===============================================================
/** The active primary view, e.g. `albums`. */
get current(): string {
return activeViewStore.get();
}
isActive(view: string): boolean {
return activeViewStore.isActive(view);
}
}
+2
View File
@@ -9,6 +9,8 @@ export type { ThemeState, BackgroundShade } from './theme-store';
export { ThemeController } from './controllers/theme-controller';
export { searchStore } from './search-store';
export { SearchController } from './controllers/search-controller';
export { activeViewStore } from './active-view-store';
export { ActiveViewController } from './controllers/active-view-controller';
export { shortcutsStore } from './shortcuts-store';
export type { ShortcutsState } from './shortcuts-store';
export { ShortcutsController } from './controllers/shortcuts-controller';
+23
View File
@@ -0,0 +1,23 @@
/**
* The shell's breakpoints, where JavaScript has to agree with CSS.
*
* A media query inside a shadow root is answered by the viewport, so a
* component normally states what it drops at phone width in its own
* stylesheet and needs nothing from here. This exists for the cases
* where the decision is not a style: `track-list` computes its grid in
* JS from the host width, and `now-playing` renders *different content*
* on a phone a plain string instead of a link which no stylesheet
* can express.
*
* One breakpoint, several expressions of it. It was a private const in
* track-list.ts when there was one; a second reader is where a copy
* would start drifting from index.css.
*/
/**
* Phone width. 600px rather than the sidebar's 900px because 900 is a
* laptop: the answer there is a narrower sidebar, which is still a
* sidebar. Below this the shell drops the sidebar column entirely and
* bottom-nav takes over.
*/
export const PHONE_QUERY = '(max-width: 599px)';
+24
View File
@@ -54,6 +54,18 @@ export const ICON_PLAYLIST = 'list';
*/
export const ICON_NEW = 'plus';
/**
* A smart playlist the rule, and the thing the rule makes.
*
* Governed for the reason `ICON_AUTOTAG` states: it was already at
* three call sites (the Playlists header, the row marker beside a smart
* playlist's name, and `smart-playlist-details`'s avatar), and a name
* stops being a detail of one component the moment there are two. It is
* deliberately *not* `ICON_NEW`, even on the button that makes one:
* an icon names the noun it acts on, and the noun here is the rule.
*/
export const ICON_SMART_PLAYLIST = 'filter';
/**
* The request ("want") toggle, as an outline/solid pair.
*
@@ -100,6 +112,18 @@ export const ICON_AUTOTAG = 'tag';
*/
export const ICON_DOWNLOADING = 'download';
/**
* The rest of what this thing can do.
*
* `page-header` collapses the actions that do not fit into one menu
* behind this, so the glyph has to name *more of the same nouns* rather
* than any one of them which is what an ellipsis is and what `bars`
* (the navigation drawer, one component over in `bottom-nav`) is not.
* It is deliberately the only meaning it carries: an overflow menu that
* shared an icon with a destination would be the `list` problem again.
*/
export const ICON_MORE_ACTIONS = 'ellipsis';
/**
* Take this away.
*
+54 -23
View File
@@ -3,15 +3,21 @@
*
* Three of these are about the thing that makes a second nav dangerous:
* it has to agree with the first one. `bottom-nav` emits the same
* bubbling, composed `navigate` event `app-sidebar` does and listens
* for that event globally, so a navigation from anywhere a card, a
* detail view, the drawer's own sidebar moves its highlight too. A
* tab bar that only tracks its own clicks looks right until the moment
* the user arrives somewhere by another route.
* bubbling, composed `navigate` event `app-sidebar` does, and reads
* which tab is lit from `activeViewStore` the shell's one statement
* of where the user is so it follows a navigation from anywhere: a
* card, a detail view, the drawer's own sidebar, or the back gesture.
*
* That last one is why the source is the store and not the `navigate`
* event these tests used to dispatch. `popstate` dispatches no
* `navigate` (index.ts calls `handleNavigate` directly), so a tab bar
* listening for the event looked right until the user pressed back
* #72.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import '@components/bottom-nav/bottom-nav';
import { activeViewStore } from '@store/active-view-store';
import type { BottomNav } from '@components/bottom-nav/bottom-nav';
import { fixture, shadow, shadowAll, update } from '@test/support/render';
import { resetHarness } from '@test/support/harness';
@@ -21,6 +27,12 @@ type Nav = BottomNav;
const tabs = (el: HTMLElement) =>
shadowAll<HTMLButtonElement>(el, 'nav button');
/** The testids of whatever the bar says is the current page. */
const current = (el: HTMLElement) =>
tabs(el)
.filter((b) => b.getAttribute('aria-current') === 'page')
.map((b) => b.dataset.testid);
/** Resolve on one occurrence of an event, or reject loudly on time. */
const once = (el: Element, name: string, timeoutMs = 2000) =>
new Promise<void>((resolve, reject) => {
@@ -70,36 +82,55 @@ describe('bottom-nav', () => {
it('follows a navigation it did not send', async () => {
const el = await fixture<Nav>('bottom-nav');
document.dispatchEvent(new CustomEvent('navigate', {
detail: { view: 'tracks' },
bubbles: true,
composed: true,
}));
activeViewStore.setView('tracks', true);
await update(el, {});
const current = tabs(el)
.filter((b) => b.getAttribute('aria-current') === 'page')
.map((b) => b.dataset.testid);
expect(current).toEqual(['tab-tracks']);
expect(current(el)).toEqual(['tab-tracks']);
});
it('marks exactly one tab current, and none for a view it has no tab for', async () => {
const el = await fixture<Nav>('bottom-nav');
document.dispatchEvent(new CustomEvent('navigate', {
detail: { view: 'settings' },
bubbles: true,
composed: true,
}));
activeViewStore.setView('settings', true);
await update(el, {});
// Settings lives in the drawer, so nothing in the bar is current.
// Leaving Home highlighted would be a tab bar lying about where
// the user is.
expect(
tabs(el).filter((b) => b.getAttribute('aria-current') === 'page'),
).toHaveLength(0);
expect(current(el)).toEqual([]);
});
it('keeps the parent tab lit while a detail view is open', async () => {
const el = await fixture<Nav>('bottom-nav');
activeViewStore.setView('albums', true);
// A detail view reports itself and is not primary, so it changes
// nothing. This is the first half of #72: the bar used to take the
// name, match it against no tab, and light nothing at all — while
// `app-sidebar`, which guarded on its own item list, kept the
// highlight. Neither was deliberate and the two disagreed.
activeViewStore.setView('explore-album-details', false);
await update(el, {});
expect(current(el)).toEqual(['tab-albums']);
});
it('follows the back path, which dispatches no navigate event', async () => {
const el = await fixture<Nav>('bottom-nav');
activeViewStore.setView('albums', true);
activeViewStore.setView('tracks', true);
await update(el, {});
expect(current(el)).toEqual(['tab-tracks']);
// What `popstate` does: the shell replays the entry through
// `handleNavigate` without dispatching `navigate`. A bar listening
// for the event stayed on Tracks — the view just left, confidently
// wrong rather than merely blank.
activeViewStore.setView('albums', true);
await update(el, {});
expect(current(el)).toEqual(['tab-albums']);
});
it('closes the drawer when a navigation happens', async () => {
+36 -1
View File
@@ -10,6 +10,7 @@ import '@components/sidebar/app-sidebar';
import '@components/library-filter/library-filter';
import '@components/library-status-indicator/library-status-indicator';
import { Events } from '../../src/events';
import { activeViewStore } from '@store/active-view-store';
import { emit, stub, flush, calls, lastArgs } from '@test/support/harness';
import {
fixture,
@@ -49,6 +50,8 @@ describe('<app-sidebar>', () => {
});
it('marks exactly one item as the current page', async () => {
activeViewStore.setView('home', true);
const el = await fixture('app-sidebar');
const current = shadowAll(el, 'li button').filter(
@@ -72,17 +75,49 @@ describe('<app-sidebar>', () => {
expect(seen).toEqual(['artists']);
});
it('moves aria-current to the clicked destination', async () => {
it('moves aria-current with the shell, not with the click', async () => {
activeViewStore.setView('home', true);
const el = await fixture('app-sidebar');
shadow<HTMLElement>(el, '[data-testid="nav-genres"]')?.click();
await el.updateComplete;
// The click asks; it does not answer. The sidebar used to move its
// own highlight optimistically, which is the second opinion #72
// removed -- one component deciding where the user is, while the
// shell decided separately and `bottom-nav` decided a third way.
expect(
shadow(el, '[data-testid="nav-genres"]')?.getAttribute('aria-current'),
).toBe('false');
// What the shell does with that event, in one line.
activeViewStore.setView('genres', true);
await update(el, {});
expect(
shadow(el, '[data-testid="nav-genres"]')?.getAttribute('aria-current'),
).toBe('page');
});
it('follows the back path, which dispatches no navigate event', async () => {
activeViewStore.setView('albums', true);
const el = await fixture('app-sidebar');
// `popstate` replays an entry through `handleNavigate` directly, so
// there is no `navigate` event to hear -- which is why the sidebar
// stayed on the view the user had just left (#72).
activeViewStore.setView('tracks', true);
await update(el, {});
expect(
shadowAll(el, 'li button')
.filter((item) => item.getAttribute('aria-current') === 'page')
.map((item) => item.getAttribute('data-testid')),
).toEqual(['nav-tracks']);
});
it('looks the way it did last time', async () => {
const el = await fixture('app-sidebar');
+12 -1
View File
@@ -7,6 +7,7 @@
* opening it.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/home-view/home-view';
import { stub, calls, lastArgs, stubFailure } from '@test/support/harness';
@@ -166,7 +167,17 @@ describe('home view', () => {
const before = calls('home.Service.GetShelves').length;
shadow<HTMLElement>(el, 'wa-button')!.click();
// The action is declared to `page-header` rather than slotted as
// markup (#69), so it is a button in *that* shadow root now.
const header = shadow<HTMLElement>(el, 'page-header')!;
await (header as LitElement).updateComplete;
header.shadowRoot!
.querySelector<HTMLButtonElement>(
'[data-testid="page-action-shuffle-suggestions"]',
)!
.click();
await el.updateComplete;
expect(calls('home.Service.GetShelves').length).toBe(before + 1);
@@ -0,0 +1,85 @@
/**
* A hover affordance is gated on the device having hover.
*
* The home page's cover cards reveal a play button on :hover. A touch
* long-press synthesises a hover state in the WebView, so on a phone
* that button flashed into view during the 500ms hold that
* utils/long-press.ts is measuring for a context menu a control
* appearing because the user was reaching for a different one.
*
* This is asserted against the *parsed stylesheet* rather than by
* emulating a touch device, and that is a limitation worth stating
* rather than hiding. CDP's Emulation.setEmulatedMedia does not reach
* this tier's iframe matchMedia still answers `hover: hover` after it
* is set so there is no way here to render the component as a phone
* would and read the computed style. What can be checked is the shape
* the browser actually built from the css`` literal: that the reveal
* lives inside a hover media query and that the default is display:none.
*
* Which is the regression worth catching anyway. The failure mode is
* someone hoisting the rule back out of the query for a one-line tidy
* a change nothing renders differently on a desktop, so every other
* assertion in this repo passes and the phone silently regresses.
*/
import { describe, expect, it } from 'vitest';
import '@components/home-view/home-view';
import { fixture } from '@test/support/render';
/** Every rule in the element's own adopted stylesheets, flattened. */
function rulesOf(host: Element): { text: string; condition: string | null }[] {
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
const out: { text: string; condition: string | null }[] = [];
for (const sheet of sheets) {
for (const rule of Array.from(sheet.cssRules)) {
if (rule instanceof CSSMediaRule) {
for (const inner of Array.from(rule.cssRules)) {
out.push({ text: inner.cssText, condition: rule.conditionText });
}
continue;
}
out.push({ text: rule.cssText, condition: null });
}
}
return out;
}
describe('the home card play button', () => {
it('reveals itself only where the device has hover', async () => {
const el = await fixture('home-view', {});
const rules = rulesOf(el);
// The sweep is worth nothing if it read no rules at all — the same
// first assertion icon-language.test.ts makes for the same reason.
expect(rules.length).toBeGreaterThan(0);
const reveals = rules.filter(
(r) => r.text.includes('.play') && /opacity:\s*1/.test(r.text),
);
expect(reveals.length).toBeGreaterThan(0);
for (const rule of reveals) {
expect(rule.condition).toMatch(/hover:\s*hover/);
expect(rule.condition).toMatch(/pointer:\s*fine/);
}
});
it('is display:none rather than transparent where it is absent', async () => {
const el = await fixture('home-view', {});
// opacity:0 alone would leave a button that still takes taps and is
// still in the accessibility tree, so a phone would keep the hit
// area for a control it can never see.
const unconditional = rulesOf(el).filter(
(r) => r.condition === null && r.text.startsWith('.play'),
);
expect(unconditional.length).toBeGreaterThan(0);
expect(unconditional.some((r) => /display:\s*none/.test(r.text))).toBe(true);
});
});
@@ -44,6 +44,8 @@ const GOVERNED = [
'regular/bookmark',
'bars-staggered',
'tag',
'filter',
'ellipsis',
];
/** The one file allowed to say them, plus its own test. */
@@ -0,0 +1,166 @@
/**
* The mini player's links are a desktop affordance.
*
* `utils/explore-link.ts` makes every track and artist name navigate,
* and `utils/queue-source-link.ts` makes "Playing from X" navigate in
* the bottom bar those are a few characters of text at a font size
* chosen for a bar, which is not a touch target. Worse, explore-link
* holds the navigation for one double-click interval and drops it if a
* second click arrives: a gesture that exists so double-clicking a row
* can play it, and which means nothing at all on touch.
*
* So below the shell's phone breakpoint the three render as plain text
* and the whole bar's cover art opens the full-screen Now Playing view,
* which is where the links live.
*
* The breakpoint is stubbed rather than emulated for the reason
* track-list-phone.test.ts states: this tier's viewport is fixed at
* 1280x800 by the runner, and the component reads matchMedia in
* connectedCallback precisely so a test can answer it first.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import '@components/now-playing/now-playing';
import { Events } from '../../src/events';
import { emit, flush } from '@test/support/harness';
import { fixture, shadow, shadowAll, text } from '@test/support/render';
import type { TrackInfo } from '@store/player-store';
import type { QueueTrack } from '@store/queue-store';
const TRACK: TrackInfo = {
fileName: 'ashes.mp3',
filePath: '/music/ashes.mp3',
trackLength: 215,
seekPosition: 0,
state: 'playing',
title: 'Ashes to Ashes',
artist: 'David Bowie',
album: 'Scary Monsters',
coverArt: '',
coverArtSmall: '',
coverArtMedium: '',
coverArtLarge: '',
trackChangeId: 1,
artistMbid: '',
releaseGroupMbid: '',
recordingMbid: '',
};
function queueTrack(n: number, title: string): QueueTrack {
return {
id: n,
audioFileId: n,
filePath: `/music/${n}.mp3`,
position: n,
title,
artist: 'David Bowie',
album: 'Scary Monsters',
coverArtPath: '',
artistMbid: '',
releaseGroupMbid: '',
recordingMbid: '',
};
}
/** Mount the bar with the phone breakpoint answering `matches`. */
async function mountAt(phone: boolean) {
const real = window.matchMedia.bind(window);
window.matchMedia = ((q: string) =>
q.includes('max-width: 599px')
? {
matches: phone,
media: q,
addEventListener() {},
removeEventListener() {},
}
: real(q)) as typeof window.matchMedia;
try {
const el = await fixture('now-playing');
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 });
emit(Events.QueueChanged, {
tracks: [queueTrack(1, 'Ashes to Ashes')],
currentIndex: 0,
source: { type: 'album', id: 7, label: 'Scary Monsters' },
});
await flush();
await el.updateComplete;
return el;
} finally {
window.matchMedia = real;
}
}
describe('the mini player on a phone', () => {
beforeEach(() => {
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 1 });
});
it('renders the title and artist as plain text', async () => {
const el = await mountAt(true);
expect(shadowAll(el, '.explore-link').length).toBe(0);
// The words are unchanged — this is about what they are, not about
// hiding them. A fix that dropped the text would pass an assertion
// about links alone.
expect(text(el, '[data-testid="now-playing-title"]')).toContain(
'Ashes to Ashes',
);
expect(text(el, '[data-testid="now-playing-artist"]')).toContain(
'David Bowie',
);
});
it('does not navigate from the source line', async () => {
const el = await mountAt(true);
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
expect(source?.classList.contains('navigable')).toBe(false);
let navigated = false;
el.addEventListener('navigate', () => {
navigated = true;
});
source?.click();
expect(navigated).toBe(false);
});
it('still says where the queue came from', async () => {
const el = await mountAt(true);
// Dropping the *link* is the change; dropping the information would
// be a different and worse one.
expect(text(el, '[data-testid="now-playing-source"]')).toBe(
'Playing from Scary Monsters',
);
});
it('leaves the desktop bar exactly as it was', async () => {
const el = await mountAt(false);
expect(shadowAll(el, '.explore-link').length).toBeGreaterThan(0);
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
expect(source?.classList.contains('navigable')).toBe(true);
let detail: unknown;
el.addEventListener('navigate', (e) => {
detail = (e as CustomEvent).detail;
});
source?.click();
expect(detail).toEqual({
view: 'explore-album-details',
localAlbumId: 7,
albumName: 'Scary Monsters',
});
});
});
+320 -1
View File
@@ -9,7 +9,7 @@
* the thing no assertion can the header looking wrong.
*/
import { describe, expect, it } from 'vitest';
import type { PageHeader } from '@components/page-header/page-header';
import type { PageAction, PageHeader } from '@components/page-header/page-header';
import '@components/page-header/page-header';
import { fixture, shadow, shadowAll, update, visual } from '@test/support/render';
@@ -19,6 +19,67 @@ const SORTS = [
{ id: 'tracks', label: 'Tracks' },
];
/**
* Three actions of the shape that broke: Playlists' own, whose widths
* (91 + 122 + 162 = 390px) are what a 700px header could not hold.
*/
function playlistActions(seen: string[]): PageAction[] {
return [
{
id: 'import',
label: 'Import',
icon: 'file-import',
priority: 0,
onSelect: () => seen.push('import'),
},
{
id: 'new-playlist',
label: 'New Playlist',
icon: 'plus',
priority: 2,
onSelect: () => seen.push('new-playlist'),
},
{
id: 'new-smart-playlist',
label: 'New Smart Playlist',
icon: 'filter',
priority: 1,
onSelect: () => seen.push('new-smart-playlist'),
},
];
}
/**
* Resize and let the fit settle.
*
* The rule is driven by a ResizeObserver, which delivers before paint
* and therefore after the microtask queue an `updateComplete` drains
* so this waits on frames rather than on promises, and then on the
* render the measurement asks for.
*/
async function widthOf(el: PageHeader, px: number): Promise<void> {
el.style.width = `${px}px`;
for (let frame = 0; frame < 3; frame += 1) {
await new Promise((r) => requestAnimationFrame(r));
await el.updateComplete;
}
}
/** The labels currently rendered as buttons, in order. */
function buttons(el: PageHeader): string[] {
return shadowAll<HTMLButtonElement>(el, '.action')
.filter((b) => !b.hidden)
.map((b) => b.textContent?.trim() ?? '');
}
/** The labels currently in the overflow menu, in order. */
function menu(el: PageHeader): string[] {
return shadowAll(el, '#page-header-overflow wa-dropdown-item').map(
(i) => i.textContent?.trim() ?? '',
);
}
describe('<page-header>', () => {
it('renders the heading as the page\u2019s only h1', async () => {
const el = await fixture<PageHeader>('page-header', {
@@ -153,6 +214,264 @@ describe('<page-header>', () => {
});
});
/**
* #69: Playlists slotted three buttons totalling 390px into a header
* that gets 700px at 900×600, and "New Smart Playlist" rendered 114 of
* its 162. It survived a spec named `layout-overflow` because that one
* asserts the *shell* needs no sideways scrolling clipping inside a
* component is invisible to it.
*
* The header can only fix that for actions it renders itself, which is
* why they are data now. These are the assertions about the rule; the
* e2e spec is what checks it against the real widths.
*/
describe('<page-header> actions', () => {
it('renders a declared action, and asks the host to perform it', async () => {
// Same division the sort control already lives by: the header
// decides what fits, the host decides what happens.
const seen: string[] = [];
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
actions: playlistActions(seen),
});
await widthOf(el, 1200);
expect(buttons(el)).toEqual([
'Import',
'New Playlist',
'New Smart Playlist',
]);
shadow<HTMLButtonElement>(el, '[data-testid="page-action-import"]')!.click();
expect(seen).toEqual(['import']);
});
it('hides the overflow trigger while everything fits', async () => {
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
actions: playlistActions([]),
});
await widthOf(el, 1200);
expect(shadow<HTMLButtonElement>(el, '.more-button')!.hidden).toBe(true);
expect(menu(el)).toEqual([]);
});
it('collapses the lowest priority first', async () => {
// Import is lowest because it is rarest; New Playlist is highest
// because it is the drop target, and a closed menu cannot be one.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions([]),
});
// Asserted as the *order* rather than at two chosen widths: which
// pixel drops which button depends on the font and on the shell
// this tier does not have, and pinning those numbers here would be
// a test of the fixture. What the host declares is a sequence.
const states: string[][] = [];
for (let width = 1200; width >= 300; width -= 40) {
await widthOf(el, width);
const now = menu(el);
const last = states[states.length - 1];
if (last === undefined || last.join() !== now.join()) states.push(now);
}
expect(states).toEqual([
[],
['Import'],
['Import', 'New Smart Playlist'],
['Import', 'New Playlist', 'New Smart Playlist'],
]);
// The menu lists them in the host's declared order, not in the
// order they happened to collapse — a menu that reshuffles itself
// as the window narrows is a menu nobody can learn.
expect(buttons(el)).toEqual([]);
});
it('gives an action back when the width returns', async () => {
// Every pass starts from all-visible, so the collapsed set is a
// function of the current width and not of how it got there. A rule
// that only ever added to the set would never widen again.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions([]),
});
await widthOf(el, 420);
expect(buttons(el)).toEqual([]);
await widthOf(el, 1200);
expect(menu(el)).toEqual([]);
expect(buttons(el)).toEqual([
'Import',
'New Playlist',
'New Smart Playlist',
]);
});
it('collapses an action before it truncates the title', async () => {
// The title can ellipsis, which means `scrollWidth` reports a
// header that fits perfectly while the heading reads "Playlis…" —
// this issue's failure mode moved from the button to the title, and
// invisible to the same measurement that missed it the first time.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions([]),
});
await widthOf(el, 700);
const h1 = shadow<HTMLElement>(el, 'h1')!;
expect(h1.scrollWidth).toBeLessThanOrEqual(h1.clientWidth + 1);
expect(menu(el).length).toBeGreaterThan(0);
});
it('names the overflow trigger and says what it controls', async () => {
// An overflow menu is exactly the shape that grows a nameless
// control, and `aria-controls` cannot name an element that is not
// in the DOM — which is why the panel renders unconditionally and
// `wa-popup` hides it, the same rule `config-section` follows.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions([]),
});
await widthOf(el, 480);
const more = shadow<HTMLButtonElement>(el, '.more-button')!;
expect(more.hidden).toBe(false);
expect(more.getAttribute('aria-label')).toBe('More actions');
expect(more.getAttribute('aria-expanded')).toBe('false');
expect(more.getAttribute('aria-haspopup')).toBe('menu');
const panel = shadow<HTMLElement>(el, '#page-header-overflow')!;
expect(more.getAttribute('aria-controls')).toBe(panel.id);
expect(panel.getAttribute('role')).toBe('menu');
more.click();
await el.updateComplete;
expect(
shadow<HTMLButtonElement>(el, '.more-button')!.getAttribute(
'aria-expanded',
),
).toBe('true');
});
it('runs a collapsed action from the menu, and closes it', async () => {
const seen: string[] = [];
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 4,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
actions: playlistActions(seen),
});
await widthOf(el, 700);
shadow<HTMLButtonElement>(el, '.more-button')!.click();
await el.updateComplete;
shadowAll<HTMLElement>(el, '#page-header-overflow wa-dropdown-item')[0]!.click();
await el.updateComplete;
expect(seen).toEqual(['import']);
expect(
shadow<HTMLButtonElement>(el, '.more-button')!.getAttribute(
'aria-expanded',
),
).toBe('false');
});
it('keeps a drop target a drop target, and does not fake one in the menu', async () => {
// You cannot drag a track onto a closed menu, so the affordance is
// absent from the overflow rather than approximated there. The
// header wires the handlers onto the button and owns none of them.
const dropped: string[] = [];
const actions: PageAction[] = [
{
id: 'new-playlist',
label: 'New Playlist',
icon: 'plus',
onSelect: () => undefined,
drop: {
active: true,
onDragOver: () => dropped.push('over'),
onDragLeave: () => dropped.push('leave'),
onDrop: () => dropped.push('drop'),
},
},
];
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
actions,
});
await widthOf(el, 1200);
const button = shadow<HTMLElement>(
el,
'[data-testid="page-action-new-playlist"]',
)!;
expect(button.classList.contains('drag-over')).toBe(true);
button.dispatchEvent(new DragEvent('dragover', { bubbles: true }));
button.dispatchEvent(new DragEvent('drop', { bubbles: true }));
expect(dropped).toEqual(['over', 'drop']);
// …and collapsed, it is a menu item with no drop wiring at all.
await widthOf(el, 120);
expect(menu(el)).toEqual(['New Playlist']);
expect(
shadow<HTMLElement>(el, '[data-testid="page-action-new-playlist"]')
?.hidden,
).toBe(true);
});
it('renders nothing at all for a view with no actions', async () => {
// Two of the three hosts have one action and one has none while its
// other tab is up; an empty actions row is not a mode.
const el = await fixture<PageHeader>('page-header', { heading: 'Albums' });
await widthOf(el, 900);
expect(shadow(el, '.actions')).toBeNull();
});
});
describe('<page-header> as each view wears it', () => {
// One baseline per arrangement rather than per view: the point is
// that eight views produce four shapes, not eight.
@@ -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);
});
});
+56 -3
View File
@@ -1,11 +1,13 @@
/**
* The three small stores behind view chrome: the global search term,
* the track list's column set, and the explore cache that keeps detail
* pages from re-fetching what a search already returned.
* The small stores behind view chrome: the global search term, the
* active view both navs highlight, the track list's column set, and
* the explore cache that keeps detail pages from re-fetching what a
* search already returned.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import { searchStore } from '@store/search-store';
import { activeViewStore } from '@store/active-view-store';
import { trackListStore } from '@store/tracklist-store';
import { exploreCache, ARTIST_IMAGE_CACHE_LIMIT } from '@store/explore-cache';
import { Events } from '../../src/events';
@@ -80,6 +82,57 @@ describe('search store', () => {
});
});
describe('active view store', () => {
beforeEach(() => {
activeViewStore.setView('home', true);
});
it('holds the primary view the shell navigated to', () => {
activeViewStore.setView('albums', true);
expect(activeViewStore.get()).toBe('albums');
expect(activeViewStore.isActive('albums')).toBe(true);
expect(activeViewStore.isActive('tracks')).toBe(false);
});
it('leaves the primary view lit while a detail view is open', () => {
activeViewStore.setView('albums', true);
activeViewStore.setView('explore-album-details', false);
// #72's third finding, made deliberate: a detail view is not a
// destination in either nav, and the tab it was opened from is
// where the user still is. `app-sidebar` did this by accident (it
// guarded on its own item list) and `bottom-nav` did not do it at
// all, which is why one looked right and the other looked broken.
expect(activeViewStore.get()).toBe('albums');
});
it('does not notify when the view is unchanged', () => {
let notifications = 0;
const off = activeViewStore.subscribe(() => {
notifications += 1;
});
activeViewStore.setView('albums', true);
activeViewStore.setView('albums', true);
activeViewStore.setView('explore-album-details', false);
off();
expect(notifications).toBe(1);
});
it('lights nothing for a view with no name', () => {
// The store starts empty rather than defaulting to a view, because
// a written-down default is right only while `GetDefaultPage()`
// agrees with it. That is only safe if the empty value matches
// nothing: `isActive` compares strings, and a component asking
// about an id it does not have must not light up.
activeViewStore.setView('', true);
expect(activeViewStore.isActive('')).toBe(false);
});
});
describe('track list store', () => {
it('starts from the default column set', () => {
expect(trackListStore.getState().columnIds.length).toBeGreaterThan(0);
+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"; }