From b5bdba2f38d4c20923e582d3b80c5651376fd975 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 26 Aug 2026 05:39:42 -0400 Subject: [PATCH] fix(queue): name the queue header's two older actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clear queue and Add queue to playlist were named by a `title` attribute and nothing else, while the close button beside them has carried an `aria-label` since #24. They get one too. `title` is a name, so this is the weak-name case rather than the missing one: it is the *last* fallback in the accname order, so any content put inside the button later silently outranks it, and a phone has no hover to show it. The `title`s stay — on a desktop they are also the tooltip for an icon-only control, which is a different job. The assertion is the part worth reading. The obvious spec — `getByRole` by name, which is what `queue-overlay.spec.ts` already does for the close button — is **green on the broken build**: measured against the running pre-fix app, both buttons matched. So the second test states the property as what it is, that the name is not the tooltip: it removes the `title` attributes and asks again, which was 0 and 0 on main and is 1 and 1 now. Closes #170 --- CLAUDE.md | 15 +++++ e2e/specs/queue-overlay.spec.ts | 56 +++++++++++++++++++ .../src/components/queue-panel/queue-panel.ts | 15 +++++ 3 files changed, 86 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 3bcf933..878294c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1832,6 +1832,21 @@ is not it.** A `placeholder` is an accname fallback, so an Explore's search box — the audit's own `a11y.26` — as clean. A sweep for *empty* names cannot see a *weak* one. +**`title` is the same trap one rung lower, and it defeats the obvious +spec as well as the obvious sweep.** `queue-panel`'s Clear queue and +Add queue to playlist were named by `title` alone, so +`getByRole('button', { name: 'Clear queue' })` matched them **before** +the fix as well as after — a `getByRole` assertion, which is what +catches every other nameless control in this app, would have been +green on the broken build. `title` is the *last* fallback in the +accname order, so content put inside the button later silently +outranks it, and it is the one name a phone cannot show, having no +hover. The property is therefore asserted as *the name is not the +tooltip*: `queue-overlay.spec.ts` removes the `title` attributes and +asks again, which is 1 and 1 with `aria-label` and was measured at 0 +and 0 without it. The `title`s stay, because on a desktop they are +also the tooltip for an icon-only control and that is a different job. + **The shell scrolls sideways and not down.** `body` is `overflow-x: auto; overflow-y: hidden`, and both halves are measured. Vertically there is nothing to fix: the middle grid row is `1fr` and diff --git a/e2e/specs/queue-overlay.spec.ts b/e2e/specs/queue-overlay.spec.ts index 3c52d48..95d1307 100644 --- a/e2e/specs/queue-overlay.spec.ts +++ b/e2e/specs/queue-overlay.spec.ts @@ -157,6 +157,62 @@ test.describe('an overlaid queue says it is over the content', () => { }); }); +/** + * #170 — the other two buttons in that same row. + * + * Clear queue and Add queue to playlist predate the close button and + * were named by a `title` attribute and nothing else. Unlike the + * sliders in `control-names.spec.ts`, that is not a *missing* name: + * `title` is the last fallback in the accname order, so + * `getByRole('button', { name: 'Clear queue' })` matched them before + * this fix as well as after it — measured, 1 and 1. A sweep for empty + * names cannot see a weak one, which is `a11y.26`'s complaint and the + * reason this file could have grown a green test that proved nothing. + * + * So the name is asserted twice, and the second assertion is the one + * that fails on the broken build. Taking the tooltip away and asking + * again is the property in words: **the name is not the tooltip**. It + * is what makes the button survive content being put inside it later, + * and it is the only one of the two a phone has — there is no hover on + * the surface #55 turned into a full screen. Measured on `main` before + * the fix: 0 and 0. + * + * Both buttons are disabled here, because the queue starts empty and + * naming is not enablement. A disabled button is still in the + * accessibility tree, which is exactly where the complaint was. + */ +test.describe('the queue header says what its actions do', () => { + const ACTIONS = ['Clear queue', 'Add queue to playlist']; + + test('names both of the older actions', async ({ app }) => { + await openQueue(app); + + for (const name of ACTIONS) { + await expect( + app.getByRole('button', { name, exact: true }), + ).toHaveCount(1); + } + }); + + test('and the names do not come from the tooltip', async ({ app }) => { + await openQueue(app); + + await app.locator('#queue-panel').evaluate((el) => { + for (const button of el.shadowRoot!.querySelectorAll( + '.header-action-button', + )) { + button.removeAttribute('title'); + } + }); + + for (const name of ACTIONS) { + await expect( + app.getByRole('button', { name, exact: true }), + ).toHaveCount(1); + } + }); +}); + /** * 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 diff --git a/frontend/src/components/queue-panel/queue-panel.ts b/frontend/src/components/queue-panel/queue-panel.ts index 97d8e57..990ce2e 100644 --- a/frontend/src/components/queue-panel/queue-panel.ts +++ b/frontend/src/components/queue-panel/queue-panel.ts @@ -2201,11 +2201,25 @@ export class QueuePanel ` : nothing} +