Compare commits

...
Author SHA1 Message Date
logan b5bdba2f38 fix(queue): name the queue header's two older actions
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m44s
CI / e2e (pull_request) Successful in 10m13s
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
2026-08-26 05:39:42 -04:00
3 changed files with 86 additions and 0 deletions
+15
View File
@@ -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
+56
View File
@@ -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
@@ -2201,11 +2201,25 @@ export class QueuePanel
`
: nothing}
</div>
<!-- **Every action here is named by aria-label**, like
the close button #24 added beside them (#170). A
title alone *is* a name, which is why a sweep for
empty names reports these clean and why an
assertion by role and name is green either way --
but it is the weakest one: title 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 as a tooltip.
The titles stay. On a desktop they are the tooltip
for an icon-only control, which is a different job
from naming it, and aria-label does not do it. -->
<div class="header-actions">
<button
class="header-action-button"
@click=${() => void this.handleClearQueue()}
?disabled=${tracks.length === 0}
aria-label="Clear queue"
title="Clear queue"
>
<wa-icon
@@ -2216,6 +2230,7 @@ export class QueuePanel
class="header-action-button add-to-playlist-button"
@click=${this.handleAddToPlaylist}
?disabled=${tracks.length === 0}
aria-label="Add queue to playlist"
title="Add queue to playlist"
>
<wa-icon