From 254646da5ead55ba8b71332a07d1b9bd67ff932a Mon Sep 17 00:00:00 2001 From: Logan Date: Thu, 13 Aug 2026 02:14:55 -0400 Subject: [PATCH] fix(a11y): mark the playing row with a shape, not only a colour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit a11y.22, WCAG 1.4.1: `.track-row.active` was a background tint and a text colour, and the row markup carried no aria-current either — so a colour-blind user could not find the playing row and AT had no signal at all. The queue panel had aria-current from Phase 1 and the same colour-only visual. A triangle drawn in each row's own left padding by `::before`. It is a shape that is present or absent, and it costs no layout: track-list's grid columns are computed from the host width, so a marker in the flow would move every cell on the playing row and nothing else. Both directions are asserted in both tiers. A marker that renders on every row satisfies "the playing row has one" for free, which is this plan's oldest rule. And one thing the reproduction found: a track started from the *list* leaves the queue's currentIndex at -1, so the panel has no current row in that flow at all. Pre-existing, and the reason this looked broken the first time it was checked in the running app. --- e2e/specs/playback.spec.ts | 35 ++++++++ .../src/components/queue-panel/queue-panel.ts | 16 ++++ .../src/components/track-list/track-list.ts | 20 +++++ frontend/test/components/aria-tail.test.ts | 84 +++++++++++++++++++ .../test/components/queue-reorder.test.ts | 15 ++++ 5 files changed, 170 insertions(+) diff --git a/e2e/specs/playback.spec.ts b/e2e/specs/playback.spec.ts index 6cd8959..f5f5f19 100644 --- a/e2e/specs/playback.spec.ts +++ b/e2e/specs/playback.spec.ts @@ -141,3 +141,38 @@ test.describe('queue', () => { await expect(shuffle).toHaveAttribute('aria-pressed', 'false'); }); }); + +test.describe('the playing row is findable without colour vision', () => { + test('the queue marks its current row with a shape and aria-current', async ({ + app, + }) => { + await app.getByTestId('nav-tracks').click(); + await app.getByTestId('track-row').first().dblclick(); + + const queueToggle = app.locator('#queue-button'); + + await queueToggle.click(); + + const row = app.getByTestId('queue-row').first(); + + await expect(row).toBeVisible(); + + // Played *from the queue*, because a track started from the list + // leaves `currentIndex` at -1 — so the panel has no current row at + // all in that flow, which is what made this marker look broken the + // first time it was checked. + await row.dblclick(); + await expect(row).toHaveAttribute('aria-current', 'true'); + + const marker = await row.evaluate( + (el) => getComputedStyle(el, '::before').borderLeftWidth, + ); + + // `a11y.22`: a background tint and a text colour were the only two + // signals, and both are hue (WCAG 1.4.1). + expect(parseFloat(marker)).toBeGreaterThan(0); + + await queueToggle.click(); + await expect(app.getByTestId('queue-row')).toHaveCount(0); + }); +}); diff --git a/frontend/src/components/queue-panel/queue-panel.ts b/frontend/src/components/queue-panel/queue-panel.ts index 6453424..1b8e8d4 100644 --- a/frontend/src/components/queue-panel/queue-panel.ts +++ b/frontend/src/components/queue-panel/queue-panel.ts @@ -378,6 +378,22 @@ export class QueuePanel color: var(--yj-accent-text, #ffd43b); } + /* a11y.22, the same rule as track-list one panel over: the + playing row was a tint and a text colour and nothing else. + The triangle lives in the row's own 16px left padding, so it + is a shape that is present or absent and the flex row does + not move. */ + .track-item.active::before { + content: ''; + position: absolute; + left: 5px; + top: 50%; + transform: translateY(-50%); + border-left: 5px solid var(--yj-accent-text, #ffd43b); + border-top: 4px solid transparent; + border-bottom: 4px solid transparent; + } + .track-art { width: 32px; height: 32px; diff --git a/frontend/src/components/track-list/track-list.ts b/frontend/src/components/track-list/track-list.ts index 4a5e48b..a82b071 100644 --- a/frontend/src/components/track-list/track-list.ts +++ b/frontend/src/components/track-list/track-list.ts @@ -1002,6 +1002,7 @@ export class TrackList } .track-row { + position: relative; display: grid; grid-template-columns: var(--grid-cols); font-size: var(--yj-text-sm); @@ -1042,6 +1043,24 @@ export class TrackList color: var(--yj-accent-text, #ffd43b); } + /* a11y.22: the playing row was a background tint and a text colour + and nothing else, so a colour-blind user could not find it + (WCAG 1.4.1). A triangle drawn in the row's own 8px left padding + is a *shape* that is present or absent, and it costs no layout: + the grid columns are computed from the host width and every one + of them would have had to move for a marker in the flow. Sized + to that padding rather than to the glyph a font would give. */ + .track-row.active::before { + content: ''; + position: absolute; + left: 1px; + top: 50%; + transform: translateY(-50%); + border-left: 5px solid currentColor; + border-top: 4px solid transparent; + border-bottom: 4px solid transparent; + } + .track-row.selected.active { background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15)); } @@ -1808,6 +1827,7 @@ export class TrackList role="row" aria-rowindex=${index + 1} aria-selected=${selected} + aria-current=${active ? 'true' : 'false'} tabindex=${index === this.focusedIndex ? 0 : -1} draggable="true" data-index=${index} diff --git a/frontend/test/components/aria-tail.test.ts b/frontend/test/components/aria-tail.test.ts index ba31564..19a51ee 100644 --- a/frontend/test/components/aria-tail.test.ts +++ b/frontend/test/components/aria-tail.test.ts @@ -235,3 +235,87 @@ describe('a clipped value is readable somewhere', () => { expect(shadow(el, '.secondary')?.getAttribute('title')).toBeTruthy(); }); }); + +describe('the playing row is more than a colour', () => { + beforeEach(async () => { + resetHarness(); + searchStore.setTerm(''); + stub('library.Library.GetAllTracks', TRACKS); + stub('library.Library.GetAllAlbums', []); + emit(Events.LibraryScanComplete); + }); + + /** The row the player says it is on, once the list has settled. */ + async function listWithPlayingTrack(): Promise { + const el = await fixture('track-list'); + + sized(el); + await settle(el); + + emit(Events.TrackChanged, { + fileName: 'a.mp3', + filePath: '/m/a.mp3', + trackLength: 4, + seekPosition: 0, + state: 'playing', + title: 'Departure', + artist: 'Aurora Fields', + album: 'Glass Harbour', + coverArt: '', + coverArtSmall: '', + coverArtMedium: '', + coverArtLarge: '', + trackChangeId: 1, + artistMbid: '', + releaseGroupMbid: '', + recordingMbid: '', + }); + await settle(el); + + return el; + } + + it('marks exactly one row aria-current', async () => { + const el = await listWithPlayingTrack(); + + const current = shadowAll(el, '.track-row[aria-current="true"]'); + + // The positive case first: a guard that marks nothing passes + // "at most one" for free. + expect(current).toHaveLength(1); + expect(current[0]!.classList.contains('active')).toBe(true); + }); + + it('draws a marker that is a shape, not a hue', async () => { + const el = await listWithPlayingTrack(); + + const active = shadow(el, '.track-row.active'); + const other = shadowAll(el, '.track-row:not(.active)')[0]; + + expect(active, 'no active row rendered').toBeTruthy(); + + // `::before` has no box unless it has content, so a width of zero + // on the inactive row is the assertion that the marker is *absent* + // there — which is the half that makes the present one mean + // something (a11y.22, WCAG 1.4.1). + const width = (el: Element) => + parseFloat(getComputedStyle(el, '::before').borderLeftWidth) || 0; + + expect(width(active!)).toBeGreaterThan(0); + expect(width(other!)).toBe(0); + }); + + it('does not move the row it marks', async () => { + const el = await listWithPlayingTrack(); + + const rows = shadowAll(el, '.track-row'); + const lefts = rows.map( + (r) => r.querySelector('.cell')!.getBoundingClientRect().left, + ); + + // The marker lives in the row's own padding: the grid columns are + // computed from the host width, so anything in the flow would move + // every cell on the playing row and nothing else. + expect(new Set(lefts.map(Math.round)).size).toBe(1); + }); +}); diff --git a/frontend/test/components/queue-reorder.test.ts b/frontend/test/components/queue-reorder.test.ts index 75a8542..c1e7bee 100644 --- a/frontend/test/components/queue-reorder.test.ts +++ b/frontend/test/components/queue-reorder.test.ts @@ -194,6 +194,21 @@ describe('a queue row says which track its controls act on', () => { ]); }); + it('marks the playing row with a shape, not only a colour', async () => { + const el = await panelWithQueue(); + + const rows = shadowAll(el, '.track-item'); + const width = (r: Element) => + parseFloat(getComputedStyle(r, '::before').borderLeftWidth) || 0; + + // `a11y.22`, the same rule as track-list one panel over. Both + // directions: absent on the rows that are not playing is what + // makes the present one mean anything. + expect(rows[0]!.getAttribute('aria-current')).toBe('true'); + expect(width(rows[0]!)).toBeGreaterThan(0); + expect(width(rows[1]!)).toBe(0); + }); + it('gives the title and artist a tooltip, since the panel is resizable', async () => { const el = await panelWithQueue();