Compare commits
2
Commits
1e4a4e6f8e
...
11b4aaef6a
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
11b4aaef6a | ||
|
|
0a0da0c19c |
@@ -0,0 +1,299 @@
|
|||||||
|
# 008 — The last audit, and the one binding that outlived six phases
|
||||||
|
|
||||||
|
**Status:** active
|
||||||
|
**Branch:** main
|
||||||
|
**Created:** 2026-08-12
|
||||||
|
**Follows:** 007-ui-reconciliation
|
||||||
|
**Source:** `.planning/audits/2026-08-11-ui/a11y.md` (34 findings), plus
|
||||||
|
one item inherited through all six phases of 007.
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
Three of the four audits from 2026-08-11 are closed. `a11y.md` is not,
|
||||||
|
and it is **the least verified material in the repo** — 007's own notes
|
||||||
|
say so twice, and every pass that touched it found the audit wrong about
|
||||||
|
something.
|
||||||
|
|
||||||
|
Two things follow from that, and they are the shape of this plan.
|
||||||
|
|
||||||
|
**The coverage map lies, and it lies in the direction of more work than
|
||||||
|
exists.** 007's map assigns `a11y.7` (top-results cards are click-only
|
||||||
|
divs) to Phase 6; the code has carried `role="button" tabindex="0"` and
|
||||||
|
an Enter/Space handler since Phase 1. It is not alone. A grep pass over
|
||||||
|
all 34 findings against the current tree closes at least five the map
|
||||||
|
still shows open, and three of those (`17`, `19`, `27`) were closed by
|
||||||
|
phases that were not about them.
|
||||||
|
|
||||||
|
**And what survives is not evenly distributed.** Of the ~13 that look
|
||||||
|
open, three are genuine loss of function and the rest are Minor or
|
||||||
|
Polish. One of the three — `15` — is a hard WCAG conformance failure
|
||||||
|
that has been sitting under a "Major" heading being read as a nice-to-
|
||||||
|
have.
|
||||||
|
|
||||||
|
Cutting across both: **two items in this audit were never measured at
|
||||||
|
all.** Colour contrast is flagged borderline (`--yj-text-tertiary` on
|
||||||
|
`--yj-bg-surface` ≈ 4.1:1 against 11 px text) with "that needs a real
|
||||||
|
measurement" written next to it, and 007 parked it under "deliberately
|
||||||
|
not planned — worth measuring before planning". The mouse-only resize
|
||||||
|
handles (`28`) were dropped as "cosmetic preference, no function lost",
|
||||||
|
which is a judgement made by reading. Both are claims with no number
|
||||||
|
behind them, which by this repo's own standard is not a finding yet.
|
||||||
|
|
||||||
|
Separately, and not from any audit: **`tracklist.delete`**. Advertised
|
||||||
|
in Settings as configurable, bound to nothing, and carried through six
|
||||||
|
phases because it needs an operation that does not exist.
|
||||||
|
|
||||||
|
## The triage, as of 2026-08-12
|
||||||
|
|
||||||
|
Grep-level against `1e4a4e6`. **Every row is a hypothesis** — this is
|
||||||
|
where the audit's claims are, not where the code is. Nothing here is
|
||||||
|
fixed until it has been reproduced in the running app.
|
||||||
|
|
||||||
|
**Closed** (verified present in code): `1`, `2`, `3`, `4`, `5`, `6`,
|
||||||
|
`7`, `8`, `12`, `13`, `16`, `17`, `19`, `27`, `33`.
|
||||||
|
|
||||||
|
**Closed by argument rather than by code**, to confirm by reading:
|
||||||
|
`20` (the type-scale/`_itemSize` coupling is now documented in
|
||||||
|
`tokens.css.ts`, which is what the finding asked for), `31` (one of
|
||||||
|
`cover-grid`'s two `<img>`s has an `alt`; the finding named one).
|
||||||
|
|
||||||
|
**Open:**
|
||||||
|
|
||||||
|
| # | Level | What the grep says |
|
||||||
|
|---|---|---|
|
||||||
|
| `15` | Major | `now-playing` is not among the four files carrying `prefers-reduced-motion`. WCAG 2.2.2: moving content over 5 s with no pause mechanism. |
|
||||||
|
| `14` | Major | `combobox.ts` has no `aria-controls`, no `aria-activedescendant`, no option ids. |
|
||||||
|
| `11` | Major | No `altKey` handler in `queue-panel`. The *other* half of this finding — "no keyboard path to add a track to the queue or a playlist" — was closed by Phase 5's `MenuKeyboard`. |
|
||||||
|
| `21` | Minor | `body { height: 100vh; overflow: hidden }` unchanged. WCAG 1.4.10. |
|
||||||
|
| `22` | Minor | `queue-panel` gained `aria-current`; `track-list` did not, and neither has a non-colour marker. |
|
||||||
|
| `24` | Minor | No `title` on the truncating element in `track-info`, `playlist-view`, `queue-panel` or `track-list`. |
|
||||||
|
| `25` | Minor | `<wa-progress-bar value=…>` with no label, verbatim as filed. |
|
||||||
|
| `28` | dropped | Four `@mousedown` `<div>`s with no `role="separator"`. Never measured. |
|
||||||
|
| `29` | Polish | `<h3 class="subtitle">` for type size. |
|
||||||
|
| `30` | Polish | No skip link anywhere. |
|
||||||
|
| `32` | Polish | `title="Remove from queue"`, not identifying the track. |
|
||||||
|
| `34` | Polish | The 10 px sort arrow, unchanged. |
|
||||||
|
| — | — | Colour contrast, never measured. |
|
||||||
|
|
||||||
|
## Ordering principle
|
||||||
|
|
||||||
|
By **what is lost**, then by containment.
|
||||||
|
|
||||||
|
Phase 1 first because it is the only phase where something a user needs
|
||||||
|
is unavailable: a marquee they cannot stop, a combobox that announces
|
||||||
|
nothing while they arrow through it, and a queue whose order cannot be
|
||||||
|
changed without a mouse.
|
||||||
|
|
||||||
|
Phase 2 second because both of its items are *questions*, and the
|
||||||
|
answers change what Phase 3 contains. If the contrast measurement comes
|
||||||
|
back below 4.5:1 it is a Phase 1 item wearing a Polish hat; if the
|
||||||
|
resize handles turn out to lose function rather than preference, `28`
|
||||||
|
stops being dropped.
|
||||||
|
|
||||||
|
Phase 3 is the tail, batched, because each item is a line and the cost
|
||||||
|
is in the verification rather than the change.
|
||||||
|
|
||||||
|
Phase 4 is `tracklist.delete`, last, because it is the only work in this
|
||||||
|
plan that can destroy a user's data and it should not share a pass with
|
||||||
|
anything.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Phase 1 — The three that lose function
|
||||||
|
|
||||||
|
### `15` — the marquee cannot be stopped
|
||||||
|
|
||||||
|
`now-playing`'s title and artist scroll continuously while a track plays
|
||||||
|
when `scrollMode === 'always'` (persisted in localStorage), re-armed in
|
||||||
|
a loop by `onScrollCycleEnd`.
|
||||||
|
|
||||||
|
**Ships:** a `prefers-reduced-motion: reduce` guard that treats `always`
|
||||||
|
as `never` and disables the transition. The setting stays; the query
|
||||||
|
overrides it, which is the right precedence — a stated OS-level
|
||||||
|
accessibility preference outranks an app default the user may never have
|
||||||
|
touched.
|
||||||
|
|
||||||
|
**Reproduce first:** the guard is two lines and will look like it
|
||||||
|
worked whether or not it did. Check under an emulated
|
||||||
|
`prefers-reduced-motion` in the running app, and check that the *hover*
|
||||||
|
scroll (`scrollMode === 'hover'`, the default) is also covered — the
|
||||||
|
finding names `always` and the mechanism is shared.
|
||||||
|
|
||||||
|
**Watch for:** `now-playing`'s geometry work in `updated()` keys on the
|
||||||
|
two scroll flags, because `.will-scroll .scroll-content` carries
|
||||||
|
`padding-right: 2em` and changing the class changes the distance the
|
||||||
|
marquee travels. A guard that suppresses the animation without telling
|
||||||
|
the geometry key will leave a stale measurement behind.
|
||||||
|
|
||||||
|
### `14` — the combobox announces nothing
|
||||||
|
|
||||||
|
`role="combobox" aria-expanded aria-autocomplete="list"` on the input
|
||||||
|
and `role="listbox"`/`role="option"` below it, with no `id` on the
|
||||||
|
listbox, no `aria-controls`, no `aria-activedescendant`, and no `id` on
|
||||||
|
the options. `aria-selected` is used to mean "highlighted".
|
||||||
|
|
||||||
|
**Ships:** ids on the listbox and each option, `aria-controls`,
|
||||||
|
`aria-activedescendant` tracking the highlight, and `aria-selected`
|
||||||
|
meaning *chosen*.
|
||||||
|
|
||||||
|
**Reproduce first:** the a11y snapshot could not see a dialog's name and
|
||||||
|
may not see this either — 007 lost twenty minutes to exactly that.
|
||||||
|
CDP's `Accessibility.getFullAXTree` reports the computed value and where
|
||||||
|
it came from; use it, not the snapshot.
|
||||||
|
|
||||||
|
### `11` — queue order cannot be changed without a mouse
|
||||||
|
|
||||||
|
Reordering is `draggable="true"` with the drop index computed from
|
||||||
|
cursor Y. There is no keyboard equivalent and no `aria-` substitute.
|
||||||
|
|
||||||
|
**Ships:** Alt+ArrowUp / Alt+ArrowDown moves the focused queue item, on
|
||||||
|
the roving tab stop `utils/roving-rows.ts` already gives that list, with
|
||||||
|
a live region announcing the new position.
|
||||||
|
|
||||||
|
**Watch for:** Alt+Arrow is unmodified-adjacent but not unmodified, so
|
||||||
|
`focusedControlOwnsKey` does not apply — this is a panel binding in
|
||||||
|
`backend/shortcuts/config.go`, registered the way `tracklist.*` is, not
|
||||||
|
a document listener. And the queue panel renders no list at all when
|
||||||
|
closed, so anything asserting on it has to open it first.
|
||||||
|
|
||||||
|
**This is the risky one.** It is a new interaction model, it touches the
|
||||||
|
backend shortcut table, and the drag path it parallels computes its drop
|
||||||
|
index geometrically. It lands last in the phase, alone.
|
||||||
|
|
||||||
|
### Verification
|
||||||
|
|
||||||
|
`make ui-test` per rule, and then **run the existing tests** — that is
|
||||||
|
what has caught every bad version of a new rule in 007, including twice
|
||||||
|
in the last pass. `make ui-visual` for `15` (it changes what renders).
|
||||||
|
An e2e case for `11`, because the queue panel's animated width means a
|
||||||
|
click issued while it moves lands on whatever slid under the pointer.
|
||||||
|
A manual pass per landing, with a screenshot read.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Phase 2 — The two that were never measured
|
||||||
|
|
||||||
|
Neither is a fix. Both are a number, and the number decides whether
|
||||||
|
there is work.
|
||||||
|
|
||||||
|
**Colour contrast.** Measured against the rendered app, not against the
|
||||||
|
token file: the tokens are what a component *may* use, and what matters
|
||||||
|
is the pairs that actually appear. Sample the real computed colours at
|
||||||
|
the real sizes, report the ratios, and only then decide. The audit's own
|
||||||
|
number (≈ 4.1:1) is a hand calculation from two hex values and has the
|
||||||
|
status of a hypothesis.
|
||||||
|
|
||||||
|
**`a11y.28`, the resize handles.** Four of them: the sidebar, the queue
|
||||||
|
panel, the now-playing column, and the track-list column resizers.
|
||||||
|
"Cosmetic preference, no function lost" is the claim to test. The
|
||||||
|
track-list one is the suspicious member — a column narrowed to its floor
|
||||||
|
clips its label (007 phase 5 found "Durat…" at 800 px), so widening a
|
||||||
|
column may be the only way to read a value, which is function.
|
||||||
|
|
||||||
|
**Record both outcomes either way.** A measurement that closes a finding
|
||||||
|
is worth as much as one that opens it, and this plan's predecessor got
|
||||||
|
about a third of its value from findings that evaporated.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Phase 3 — The tail
|
||||||
|
|
||||||
|
`21`, `22`, `24`, `25`, `29`, `30`, `32`, `34`, plus whatever Phase 2
|
||||||
|
promotes or closes. One landing, batched, each item confirmed against
|
||||||
|
the code before it is touched.
|
||||||
|
|
||||||
|
Two of them are not one-liners and should be treated as such:
|
||||||
|
|
||||||
|
- **`21`** (the shell is `100vh; overflow: hidden`) is a layout change
|
||||||
|
to the app frame, and 007 phase 5 already measured the frame's real
|
||||||
|
minimum at 800×600. Reflow at high zoom is the same question one
|
||||||
|
variable over. It may want its own landing.
|
||||||
|
- **`22`** asks for a non-colour marker on the playing row, which is a
|
||||||
|
visual change to the densest list in the app and moves a baseline.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Phase 4 — `tracklist.delete`, and the operation behind it
|
||||||
|
|
||||||
|
### The decision
|
||||||
|
|
||||||
|
*(Decided 2026-08-12, before any code.)*
|
||||||
|
|
||||||
|
**"Remove from library" removes the database row and excludes the path
|
||||||
|
from future scans. It does not touch the file.**
|
||||||
|
|
||||||
|
The comment at `backend/shortcuts/config.go:36` states the fork exactly:
|
||||||
|
the row (which the next scan puts back unless the path is also excluded)
|
||||||
|
or the file (a delete-your-music button one keystroke from a focused
|
||||||
|
row). Three shapes were considered:
|
||||||
|
|
||||||
|
- **A — row + path exclusion.** Reversible, needs an exclusions table,
|
||||||
|
so a schema file *and* a migration.
|
||||||
|
- **B — delete the file**, to the platform trash. Real user intent for
|
||||||
|
an app with duplicate detection, genuinely destructive, and a new
|
||||||
|
cross-platform dependency.
|
||||||
|
- **C — ship the operation as a menu command only**, leave `Delete`
|
||||||
|
unbound.
|
||||||
|
|
||||||
|
**A, delivered as C**, and then the keystroke. Without the exclusion,
|
||||||
|
A is a button that undoes itself on the next scan, which is worse than
|
||||||
|
no button — so the exclusion is not an enhancement, it is what makes the
|
||||||
|
operation mean anything. `Delete` is bound only to *open the
|
||||||
|
confirmation*, never to perform the removal: that makes the keystroke a
|
||||||
|
request rather than an action, which is the only version defensible one
|
||||||
|
key from a focused row.
|
||||||
|
|
||||||
|
**B is not foreclosed and is not in this plan.** It deserves its own
|
||||||
|
argument.
|
||||||
|
|
||||||
|
### What ships
|
||||||
|
|
||||||
|
- An exclusions table, following the two-file schema discipline
|
||||||
|
(`sql/schemas/` for the target shape, `sql/migrations/` for the
|
||||||
|
existing install, column order matching, no index on a migrated
|
||||||
|
column in the schema file).
|
||||||
|
- `RemoveFromLibrary(filePaths)` — rows deleted, paths excluded, one
|
||||||
|
event carrying enough for the stores to patch rather than invalidate.
|
||||||
|
It is a *write*, so it goes through `ExecContext`, not the read pool.
|
||||||
|
- A context-menu command behind `confirmAction()`, with impact copy
|
||||||
|
naming the count and saying explicitly that files on disk are not
|
||||||
|
touched.
|
||||||
|
- `tracklist.delete` re-advertised, bound to opening that dialog.
|
||||||
|
- The scanner honouring the exclusion list, which is the half that makes
|
||||||
|
the rest true.
|
||||||
|
|
||||||
|
### Verification
|
||||||
|
|
||||||
|
A Go test that a removed path survives a rescan; an e2e case that the
|
||||||
|
row is gone, the dialog said so, and the file still exists. Both halves
|
||||||
|
matter — the second is the promise the copy makes.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Deliberately not in this plan
|
||||||
|
|
||||||
|
- **Splitting `explore-view.ts`** (1 900 lines). The shelves are in it
|
||||||
|
because a separate component would need its own art fetching and
|
||||||
|
therefore its own cache, cap and probe, and `perf.M7` exists because
|
||||||
|
that view never unmounts. The reasoning holds; the size is the price.
|
||||||
|
- **A `make perf` before/after for Phase 6's shelves.** Both seeds get
|
||||||
|
their catalog from the artifact rather than from the seed tarball, so
|
||||||
|
a before and an after are not the same corpus unless the e2e staging
|
||||||
|
fixture is extended to bulk scale. Recorded as unmeasured in 007
|
||||||
|
rather than implied to be free.
|
||||||
|
- **The unowned badge's `+` glyph.** It becomes correct the day the
|
||||||
|
badge becomes a button. Changing it now touches four components'
|
||||||
|
visual baselines for a call better made then.
|
||||||
|
- **The albums shelf leading with one act.** Needs dump-side data to
|
||||||
|
express "these eight artists are one group and its solo members".
|
||||||
|
A plan, not a fix.
|
||||||
|
- **WebKit2GTK-specific behaviour** (page zoom in the Wails shell, how
|
||||||
|
Orca traverses the virtualizer's windowed DOM). Only answerable on the
|
||||||
|
real shell, and CI is the only place WebKit runs.
|
||||||
|
|
||||||
|
## First step
|
||||||
|
|
||||||
|
Phase 1, and within it `15` — reproduced under an emulated
|
||||||
|
`prefers-reduced-motion` **before** the guard is written, because a
|
||||||
|
two-line CSS change looks identical whether or not it worked, and this
|
||||||
|
plan's predecessor met that failure in seven different costumes.
|
||||||
+41
-7
@@ -1,18 +1,52 @@
|
|||||||
# 007 — UI reconciliation: lifecycle, truth, voice, scale, shape, and one thing that was never built
|
# 007 — UI reconciliation: lifecycle, truth, voice, scale, shape, and one thing that was never built
|
||||||
|
|
||||||
**Status:** active — Phases 1, 2 and 3 shipped, each with one or two
|
**Status:** implemented — all six phases shipped.
|
||||||
deliberate deferrals (see the "what actually shipped" sections below).
|
**Branch:** main
|
||||||
**Phase 4 is complete** after six passes; the sixth landed `m5`, `m4`
|
|
||||||
and `m2`, which was the whole remaining tail.
|
|
||||||
Phase 3 was the plan's own clean cut: stopping here leaves the data-loss
|
|
||||||
bug, the lying player and the silent failures all fixed.
|
|
||||||
**Branch:** —
|
|
||||||
**Created:** 2026-08-11
|
**Created:** 2026-08-11
|
||||||
**Follows:** 006-orientation-fixes
|
**Follows:** 006-orientation-fixes
|
||||||
|
**Followed by:** 008-the-last-audit
|
||||||
**Source:** `.planning/audits/2026-08-11-ui/` — `hands-on.md` (24
|
**Source:** `.planning/audits/2026-08-11-ui/` — `hands-on.md` (24
|
||||||
findings reproduced in the running app), `a11y.md` (34), `perf.md`
|
findings reproduced in the running app), `a11y.md` (34), `perf.md`
|
||||||
(30), `errors.md` (30).
|
(30), `errors.md` (30).
|
||||||
|
|
||||||
|
## Recap
|
||||||
|
|
||||||
|
~118 findings, which were five problems each spread by copying. All six
|
||||||
|
phases shipped, over eleven passes, and three of the four source audits
|
||||||
|
are closed: `hands-on.md`, `perf.md` and `errors.md` have nothing open.
|
||||||
|
`a11y.md` does, and is plan 008.
|
||||||
|
|
||||||
|
- **Phase 1 — a cached view now has a lifecycle.** `viewActivated` /
|
||||||
|
`viewDeactivated`, the ambient shortcut scope the mechanism was built
|
||||||
|
for and had never had a caller, and keyboard reach for the sidebar,
|
||||||
|
the track rows, the card grids and the closed queue panel.
|
||||||
|
- **Phase 2 — the player reports its own position.** 1 Hz from the
|
||||||
|
backend, `PlaybackFailed` from both failure paths, auto-advance that
|
||||||
|
skips, and a bar that no longer counts itself 30 s adrift.
|
||||||
|
- **Phase 3 — one notification surface, four levels**, replacing 84
|
||||||
|
`catch` blocks that ended at `console.error` and two private toasts.
|
||||||
|
- **Phase 4 — works offline and at 50 000 tracks**, over six passes:
|
||||||
|
bundled icons, route splitting, the `TrackPlayCountChanged` split,
|
||||||
|
bounded caches, `utils/track-index.ts` (3–6 s → 68 ms), and the
|
||||||
|
virtualizer repaint rule.
|
||||||
|
- **Phase 5 — one app, not eleven pages**, over four passes: one
|
||||||
|
`<page-header>`, a measured window minimum, every dialog a named
|
||||||
|
`wa-dialog`, one menu keyboard model, the `?` overlay, and an album
|
||||||
|
page with a primary action that means the same thing in three
|
||||||
|
different ownership states.
|
||||||
|
- **Phase 6 — Explore opens with shelves**, plus the two inherited
|
||||||
|
one-liners: the badge that was an inert button, and the card grids
|
||||||
|
that moved to the end rather than by a row.
|
||||||
|
|
||||||
|
**What it is worth reading for**: the seven "where the plan was wrong"
|
||||||
|
lists below, seventy-nine entries across the passes. About a third are
|
||||||
|
the audit being wrong rather than the code, and they are the reason
|
||||||
|
plan 008 treats every remaining `a11y.md` claim as a hypothesis.
|
||||||
|
|
||||||
|
**Inherited, unfinished, and carried into 008**: `tracklist.delete`,
|
||||||
|
which survived all six phases because it needs a "remove from library"
|
||||||
|
operation that does not exist and a decision about what it removes.
|
||||||
|
|
||||||
## Problem
|
## Problem
|
||||||
|
|
||||||
A full pass over the UI — the app driven by hand headless, plus three
|
A full pass over the UI — the app driven by hand headless, plus three
|
||||||
@@ -147,7 +147,7 @@ tree; `make bindings` regenerates it for real.
|
|||||||
`config.toml` and DB rows — the same discipline `sql/schemas/` gets,
|
`config.toml` and DB rows — the same discipline `sql/schemas/` gets,
|
||||||
for the same reason.
|
for the same reason.
|
||||||
|
|
||||||
See `.planning/plans/active/005-agent-development-harness.md`.
|
See `.planning/plans/completed/005-agent-development-harness.md`.
|
||||||
|
|
||||||
## Architecture
|
## Architecture
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,104 @@
|
|||||||
|
import { test, expect, callBinding, waitForEvent } from '../support/fixtures.js';
|
||||||
|
import type { Page } from '@playwright/test';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* `a11y.15` — WCAG 2.2.2. The bottom bar's title and artist scroll
|
||||||
|
* continuously while a track plays, re-armed in a loop by
|
||||||
|
* `transitionend`, with no pause mechanism and no reduced-motion guard.
|
||||||
|
*
|
||||||
|
* The component test for this fakes `window.matchMedia`, which is a
|
||||||
|
* stub of the thing being tested. This spec sets the real context
|
||||||
|
* option, so the real media query answers.
|
||||||
|
*
|
||||||
|
* Both directions are here on purpose. A guard that suppressed
|
||||||
|
* everything would pass the reduce case for free, and so would a bar
|
||||||
|
* whose text simply does not overflow at this viewport — which is what
|
||||||
|
* the component test failed on first.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/** The fixture track whose title is long enough to overflow the bar. */
|
||||||
|
const LONG_TITLE = 'An Exhaustively Overlong Track Title';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Read the title line's classes from inside `now-playing`'s shadow root.
|
||||||
|
*
|
||||||
|
* `will-scroll` is the class that carries both the transition and the
|
||||||
|
* `padding-right` the scroll distance is measured against, so its
|
||||||
|
* absence is the whole fix: suppressing only the animation leaves the
|
||||||
|
* text translated off its own box with nothing to bring it back.
|
||||||
|
*/
|
||||||
|
async function titleClasses(app: Page): Promise<string> {
|
||||||
|
return app.evaluate(() => {
|
||||||
|
const np = document.querySelector('now-playing');
|
||||||
|
const title = np?.shadowRoot?.querySelector('.track-title');
|
||||||
|
|
||||||
|
return title?.className ?? '';
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
async function playTheLongOne(app: Page): Promise<void> {
|
||||||
|
await app.getByTestId('nav-tracks').click();
|
||||||
|
|
||||||
|
// The scroll mode defaults to `hover`, and a fresh context has no
|
||||||
|
// persisted setting — so without this the positive case never
|
||||||
|
// scrolls and reports the same thing a broken build would. Set it
|
||||||
|
// rather than hovering, because `always` is also the mode the
|
||||||
|
// finding is about: continuous motion for as long as the track
|
||||||
|
// plays, with nothing the user has to do to provoke it.
|
||||||
|
await app.evaluate(() => {
|
||||||
|
localStorage.setItem('yj-now-playing-scroll-mode', 'always');
|
||||||
|
window.dispatchEvent(new CustomEvent('yj-scroll-mode-changed'));
|
||||||
|
});
|
||||||
|
|
||||||
|
const paths: string[] = await app.evaluate(async (needle) => {
|
||||||
|
const tracks = await window.__yjEvents.call(
|
||||||
|
'library.Library.GetAllTracks',
|
||||||
|
[],
|
||||||
|
10_000,
|
||||||
|
);
|
||||||
|
|
||||||
|
return (tracks as { TrackName: string; FilePath: string }[])
|
||||||
|
.filter((t) => t.TrackName.startsWith(needle))
|
||||||
|
.map((t) => t.FilePath);
|
||||||
|
}, LONG_TITLE);
|
||||||
|
|
||||||
|
expect(paths.length).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
await callBinding(app, 'queue.Queue.SetQueue', [paths, 0, false]);
|
||||||
|
await waitForEvent(app, 'TrackChanged');
|
||||||
|
|
||||||
|
// The scroll cycle is armed 1500 ms after the geometry is measured,
|
||||||
|
// and the geometry is measured after the render that puts the title
|
||||||
|
// on screen. Reading before that reports "not scrolling" on a build
|
||||||
|
// that scrolls — the same shape as every probe read too early in
|
||||||
|
// plan 007.
|
||||||
|
await expect
|
||||||
|
.poll(() => titleClasses(app), { timeout: 10_000 })
|
||||||
|
.toContain('track-title');
|
||||||
|
}
|
||||||
|
|
||||||
|
test.describe('the marquee under prefers-reduced-motion', () => {
|
||||||
|
test.use({ contextOptions: { reducedMotion: 'reduce' } });
|
||||||
|
|
||||||
|
test('does not scroll the now-playing text at all', async ({ app }) => {
|
||||||
|
await playTheLongOne(app);
|
||||||
|
|
||||||
|
// Give the cycle longer than the 1500 ms arming delay to prove it
|
||||||
|
// never arms, rather than catching it before it would have.
|
||||||
|
await app.waitForTimeout(2500);
|
||||||
|
|
||||||
|
expect(await titleClasses(app)).not.toContain('will-scroll');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test.describe('the marquee without a motion preference', () => {
|
||||||
|
test.use({ contextOptions: { reducedMotion: 'no-preference' } });
|
||||||
|
|
||||||
|
test('still scrolls an overflowing title', async ({ app }) => {
|
||||||
|
await playTheLongOne(app);
|
||||||
|
|
||||||
|
await expect
|
||||||
|
.poll(() => titleClasses(app), { timeout: 10_000 })
|
||||||
|
.toContain('will-scroll');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -57,6 +57,21 @@ export class NowPlaying extends LitElement {
|
|||||||
@state()
|
@state()
|
||||||
private artistHovered = false;
|
private artistHovered = false;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* `prefers-reduced-motion: reduce`, live (a11y.15).
|
||||||
|
*
|
||||||
|
* It is a `@state` and not a CSS query because suppressing the
|
||||||
|
* *transition* is not enough: the cycle is a transition out, a
|
||||||
|
* `transitionend`, and a transition back, so removing the animation
|
||||||
|
* leaves the text translated off its own box and `onScrollCycleEnd`
|
||||||
|
* never fires to bring it back. The scroll has to not be armed at
|
||||||
|
* all, which is a decision `shouldScroll()` already owns.
|
||||||
|
*/
|
||||||
|
@state()
|
||||||
|
private reduceMotion = false;
|
||||||
|
|
||||||
|
private reduceMotionQuery?: MediaQueryList;
|
||||||
|
|
||||||
/** Whether each field is actively mid-scroll (class toggle). */
|
/** Whether each field is actively mid-scroll (class toggle). */
|
||||||
@state()
|
@state()
|
||||||
private titleScrolling = false;
|
private titleScrolling = false;
|
||||||
@@ -200,9 +215,19 @@ export class NowPlaying extends LitElement {
|
|||||||
color: var(--yj-text-tertiary, #666);
|
color: var(--yj-text-tertiary, #666);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Static ellipsis when not scrolling */
|
/* Static ellipsis when not scrolling.
|
||||||
.track-title:not(.will-scroll),
|
|
||||||
.track-artist:not(.will-scroll) {
|
It has to be on .scroll-content, not on the outer span: the child
|
||||||
|
is an inline-block, so it is the box that overflows, and
|
||||||
|
text-overflow on an ancestor does not ellipsise an overflowing
|
||||||
|
inline-block descendant — it clips it. The outer rule was there
|
||||||
|
from the start and never produced an ellipsis in any mode; the
|
||||||
|
default mode is hover, so what every user saw when not hovering
|
||||||
|
was a title cut mid-glyph. Only visible in a screenshot. */
|
||||||
|
.track-title:not(.will-scroll) .scroll-content,
|
||||||
|
.track-artist:not(.will-scroll) .scroll-content {
|
||||||
|
display: block;
|
||||||
|
overflow: hidden;
|
||||||
text-overflow: ellipsis;
|
text-overflow: ellipsis;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -248,6 +273,12 @@ export class NowPlaying extends LitElement {
|
|||||||
this.updateWidth(DEFAULT_WIDTH);
|
this.updateWidth(DEFAULT_WIDTH);
|
||||||
window.addEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
window.addEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
||||||
|
|
||||||
|
// Looked up here rather than at module load so a test can install
|
||||||
|
// its own matchMedia before the element is created.
|
||||||
|
this.reduceMotionQuery = window.matchMedia?.('(prefers-reduced-motion: reduce)');
|
||||||
|
this.reduceMotion = this.reduceMotionQuery?.matches ?? false;
|
||||||
|
this.reduceMotionQuery?.addEventListener('change', this.handleReduceMotionChange);
|
||||||
|
|
||||||
this.resizeObserver = new ResizeObserver(() => {
|
this.resizeObserver = new ResizeObserver(() => {
|
||||||
this.geometryDirty = true;
|
this.geometryDirty = true;
|
||||||
this.requestUpdate();
|
this.requestUpdate();
|
||||||
@@ -259,6 +290,7 @@ export class NowPlaying extends LitElement {
|
|||||||
// A drag interrupted by the bar going away still has to clean up.
|
// A drag interrupted by the bar going away still has to clean up.
|
||||||
this.attachDragListeners(false);
|
this.attachDragListeners(false);
|
||||||
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
||||||
|
this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
|
||||||
this.resizeObserver?.disconnect();
|
this.resizeObserver?.disconnect();
|
||||||
this.stopScrollCycle('title');
|
this.stopScrollCycle('title');
|
||||||
this.stopScrollCycle('artist');
|
this.stopScrollCycle('artist');
|
||||||
@@ -371,6 +403,7 @@ export class NowPlaying extends LitElement {
|
|||||||
<span
|
<span
|
||||||
class="track-title ${titleScrolling ? 'will-scroll' : ''} ${this.titleScrolling ? 'scrolling' : ''}"
|
class="track-title ${titleScrolling ? 'will-scroll' : ''} ${this.titleScrolling ? 'scrolling' : ''}"
|
||||||
data-testid="now-playing-title"
|
data-testid="now-playing-title"
|
||||||
|
title=${track.title}
|
||||||
@mouseenter=${this.handleTitleMouseEnter}
|
@mouseenter=${this.handleTitleMouseEnter}
|
||||||
@mouseleave=${this.handleTitleMouseLeave}
|
@mouseleave=${this.handleTitleMouseLeave}
|
||||||
@transitionend=${() => this.onScrollCycleEnd('title')}
|
@transitionend=${() => this.onScrollCycleEnd('title')}
|
||||||
@@ -380,6 +413,7 @@ export class NowPlaying extends LitElement {
|
|||||||
<span
|
<span
|
||||||
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
|
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
|
||||||
data-testid="now-playing-artist"
|
data-testid="now-playing-artist"
|
||||||
|
title=${track.artist || 'Unknown Artist'}
|
||||||
@mouseenter=${this.handleArtistMouseEnter}
|
@mouseenter=${this.handleArtistMouseEnter}
|
||||||
@mouseleave=${this.handleArtistMouseLeave}
|
@mouseleave=${this.handleArtistMouseLeave}
|
||||||
@transitionend=${() => this.onScrollCycleEnd('artist')}
|
@transitionend=${() => this.onScrollCycleEnd('artist')}
|
||||||
@@ -429,9 +463,21 @@ export class NowPlaying extends LitElement {
|
|||||||
this.loadScrollMode();
|
this.loadScrollMode();
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private handleReduceMotionChange = (e: MediaQueryListEvent): void => {
|
||||||
|
this.reduceMotion = e.matches;
|
||||||
|
};
|
||||||
|
|
||||||
private shouldScroll(field: 'title' | 'artist'): boolean {
|
private shouldScroll(field: 'title' | 'artist'): boolean {
|
||||||
const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
|
const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
|
||||||
|
|
||||||
|
// A stated OS-level preference outranks an app default the user
|
||||||
|
// may never have touched, so this comes before the mode — and it
|
||||||
|
// covers `hover` as well as `always`. Hover-scrolling is
|
||||||
|
// user-initiated and so arguably passes WCAG 2.2.2 on its own,
|
||||||
|
// but `reduce` is a request about motion, not about autoplay.
|
||||||
|
// The text falls back to the ellipsis every other mode uses.
|
||||||
|
if (this.reduceMotion) return false;
|
||||||
|
|
||||||
if (!overflows || this.scrollMode === 'never') return false;
|
if (!overflows || this.scrollMode === 'never') return false;
|
||||||
if (this.scrollMode === 'always') return true;
|
if (this.scrollMode === 'always') return true;
|
||||||
|
|
||||||
@@ -477,9 +523,18 @@ export class NowPlaying extends LitElement {
|
|||||||
const content = el.querySelector<HTMLElement>('.scroll-content');
|
const content = el.querySelector<HTMLElement>('.scroll-content');
|
||||||
const width = el.clientWidth;
|
const width = el.clientWidth;
|
||||||
|
|
||||||
|
// Both numbers come from the *child*, which is the box that
|
||||||
|
// holds the text. Asking the outer span whether it overflows
|
||||||
|
// only works while the child is an overflowing inline-block:
|
||||||
|
// once the non-scrolling state gives the child its own
|
||||||
|
// `overflow: hidden` (for the ellipsis), the parent stops
|
||||||
|
// overflowing and nothing ever arms the scroll again.
|
||||||
|
// `scrollWidth` reports the content size either way.
|
||||||
|
const full = content?.scrollWidth ?? 0;
|
||||||
|
|
||||||
return {
|
return {
|
||||||
overflows: el.scrollWidth > width,
|
overflows: full > width,
|
||||||
distance: content ? content.scrollWidth - width : 0,
|
distance: content ? full - width : 0,
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -232,6 +232,61 @@ describe('<now-playing>', () => {
|
|||||||
expect(queries()).toBeGreaterThan(0);
|
expect(queries()).toBeGreaterThan(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// a11y.15 (WCAG 2.2.2). Reproduced in the running app first: under an
|
||||||
|
// emulated `prefers-reduced-motion: reduce` the title still carried
|
||||||
|
// `will-scroll` with a 15s transition and the transform was still
|
||||||
|
// moving — the read landed in the *snap-back* half of the cycle,
|
||||||
|
// which is why suppressing the transition alone is not the fix.
|
||||||
|
//
|
||||||
|
// Both directions are asserted because a guard that suppresses
|
||||||
|
// everything passes the negative case for free, and a component that
|
||||||
|
// never scrolls at this width would too.
|
||||||
|
const LONG =
|
||||||
|
'An Exhaustively Overlong Track Title That Exists Solely To Find Out ' +
|
||||||
|
'Whether The Bottom Bar Truncates Or Overflows';
|
||||||
|
|
||||||
|
async function mountScrolling(reduce: boolean) {
|
||||||
|
const real = window.matchMedia.bind(window);
|
||||||
|
|
||||||
|
window.matchMedia = ((q: string) =>
|
||||||
|
q.includes('prefers-reduced-motion')
|
||||||
|
? {
|
||||||
|
matches: reduce,
|
||||||
|
media: q,
|
||||||
|
addEventListener() {},
|
||||||
|
removeEventListener() {},
|
||||||
|
}
|
||||||
|
: real(q)) as typeof window.matchMedia;
|
||||||
|
|
||||||
|
try {
|
||||||
|
localStorage.setItem('yj-now-playing-scroll-mode', 'always');
|
||||||
|
|
||||||
|
const el = await fixture('now-playing');
|
||||||
|
|
||||||
|
// The real host is sized by `--now-playing-width` on `.bottom-bar`,
|
||||||
|
// which the fixture does not have — so it is document-width here
|
||||||
|
// and nothing overflows, which made the positive case fail first.
|
||||||
|
el.style.width = '320px';
|
||||||
|
|
||||||
|
emit(Events.TrackChanged, { ...TRACK, title: LONG, trackChangeId: 10 });
|
||||||
|
await flush();
|
||||||
|
await settle(el);
|
||||||
|
|
||||||
|
return shadow(el, '.track-title')?.className ?? '';
|
||||||
|
} finally {
|
||||||
|
window.matchMedia = real;
|
||||||
|
localStorage.removeItem('yj-now-playing-scroll-mode');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
it('scrolls an overflowing title when motion is not a problem', async () => {
|
||||||
|
expect(await mountScrolling(false)).toContain('will-scroll');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not scroll at all under prefers-reduced-motion', async () => {
|
||||||
|
expect(await mountScrolling(true)).not.toContain('will-scroll');
|
||||||
|
});
|
||||||
|
|
||||||
it('looks the way it did last time', async () => {
|
it('looks the way it did last time', async () => {
|
||||||
const el = await fixture('now-playing');
|
const el = await fixture('now-playing');
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user