Compare commits
2
Commits
| 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
|
||||
|
||||
**Status:** active — Phases 1, 2 and 3 shipped, each with one or two
|
||||
deliberate deferrals (see the "what actually shipped" sections below).
|
||||
**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:** —
|
||||
**Status:** implemented — all six phases shipped.
|
||||
**Branch:** main
|
||||
**Created:** 2026-08-11
|
||||
**Follows:** 006-orientation-fixes
|
||||
**Followed by:** 008-the-last-audit
|
||||
**Source:** `.planning/audits/2026-08-11-ui/` — `hands-on.md` (24
|
||||
findings reproduced in the running app), `a11y.md` (34), `perf.md`
|
||||
(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
|
||||
|
||||
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,
|
||||
for the same reason.
|
||||
|
||||
See `.planning/plans/active/005-agent-development-harness.md`.
|
||||
See `.planning/plans/completed/005-agent-development-harness.md`.
|
||||
|
||||
## 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()
|
||||
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). */
|
||||
@state()
|
||||
private titleScrolling = false;
|
||||
@@ -200,9 +215,19 @@ export class NowPlaying extends LitElement {
|
||||
color: var(--yj-text-tertiary, #666);
|
||||
}
|
||||
|
||||
/* Static ellipsis when not scrolling */
|
||||
.track-title:not(.will-scroll),
|
||||
.track-artist:not(.will-scroll) {
|
||||
/* Static ellipsis when not scrolling.
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -248,6 +273,12 @@ export class NowPlaying extends LitElement {
|
||||
this.updateWidth(DEFAULT_WIDTH);
|
||||
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.geometryDirty = true;
|
||||
this.requestUpdate();
|
||||
@@ -259,6 +290,7 @@ export class NowPlaying extends LitElement {
|
||||
// A drag interrupted by the bar going away still has to clean up.
|
||||
this.attachDragListeners(false);
|
||||
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
||||
this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
|
||||
this.resizeObserver?.disconnect();
|
||||
this.stopScrollCycle('title');
|
||||
this.stopScrollCycle('artist');
|
||||
@@ -371,6 +403,7 @@ export class NowPlaying extends LitElement {
|
||||
<span
|
||||
class="track-title ${titleScrolling ? 'will-scroll' : ''} ${this.titleScrolling ? 'scrolling' : ''}"
|
||||
data-testid="now-playing-title"
|
||||
title=${track.title}
|
||||
@mouseenter=${this.handleTitleMouseEnter}
|
||||
@mouseleave=${this.handleTitleMouseLeave}
|
||||
@transitionend=${() => this.onScrollCycleEnd('title')}
|
||||
@@ -380,6 +413,7 @@ export class NowPlaying extends LitElement {
|
||||
<span
|
||||
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
|
||||
data-testid="now-playing-artist"
|
||||
title=${track.artist || 'Unknown Artist'}
|
||||
@mouseenter=${this.handleArtistMouseEnter}
|
||||
@mouseleave=${this.handleArtistMouseLeave}
|
||||
@transitionend=${() => this.onScrollCycleEnd('artist')}
|
||||
@@ -429,9 +463,21 @@ export class NowPlaying extends LitElement {
|
||||
this.loadScrollMode();
|
||||
};
|
||||
|
||||
private handleReduceMotionChange = (e: MediaQueryListEvent): void => {
|
||||
this.reduceMotion = e.matches;
|
||||
};
|
||||
|
||||
private shouldScroll(field: 'title' | 'artist'): boolean {
|
||||
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 (this.scrollMode === 'always') return true;
|
||||
|
||||
@@ -477,9 +523,18 @@ export class NowPlaying extends LitElement {
|
||||
const content = el.querySelector<HTMLElement>('.scroll-content');
|
||||
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 {
|
||||
overflows: el.scrollWidth > width,
|
||||
distance: content ? content.scrollWidth - width : 0,
|
||||
overflows: full > width,
|
||||
distance: content ? full - width : 0,
|
||||
};
|
||||
};
|
||||
|
||||
|
||||
@@ -232,6 +232,61 @@ describe('<now-playing>', () => {
|
||||
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 () => {
|
||||
const el = await fixture('now-playing');
|
||||
|
||||
|
||||
Reference in New Issue
Block a user