fix(a11y): name the sliders and the progress bar where the role is
`a11y.md` lists `seek-bar` and `volume-control` under "what is already correct" because both pass `aria-label`. Measured with Accessibility.getFullAXTree against the running app on all eleven views, both sliders compute a name of "": `wa-slider` puts role="slider" on a div inside its own shadow root, pointing aria-labelledby at an empty internal <label>, and that IDREF outranks the host's aria-label. `volume-control` did not have the aria-label the audit credits it with at all. The name comes from `label` now, which is the library's own API — and for a slider that is visible, so `styles/wa-slider-label.css.ts` hides it by part. Preferred over reaching into the shadow root the way name-dialog.ts must: if Web Awesome renames the part the label becomes visible rather than silently nameless. The second rule in that file is load-bearing — `#slider` takes an 8px margin the moment a label exists, which grows the bar from 6px to 14px and moves the transport with it. a11y.25 is the same family: wa-progress-bar maps `label` onto its inner aria-label, falling back to the localised word "progress" — so it was named after the widget rather than after the work, not unnamed. The existing transport test asserted the host's aria-label and called it an accessible name, so it was pinning the bug.
This commit is contained in:
@@ -0,0 +1,57 @@
|
|||||||
|
import { test, expect } from '../support/fixtures.js';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Plan 008 phase 3: the two sliders the audit filed as exemplary have
|
||||||
|
* no accessible name.
|
||||||
|
*
|
||||||
|
* `a11y.md` lists `seek-bar` and `volume-control` under **what is
|
||||||
|
* already correct** — "`wa-slider` with `aria-label` and a
|
||||||
|
* `valueFormatter`, so the seek position is announced as `3:42` rather
|
||||||
|
* than `222`". The formatter is real. The name was not: `wa-slider`
|
||||||
|
* puts `role="slider"` on a `<div id="slider" aria-labelledby="label">`
|
||||||
|
* inside its own shadow root, and that IDREF — pointing at an empty
|
||||||
|
* internal `<label>` — outranks whatever `aria-label` the host carries.
|
||||||
|
* Measured with `Accessibility.getFullAXTree` on all eleven views:
|
||||||
|
* name `""`, every time. `volume-control` had no `aria-label` at all.
|
||||||
|
*
|
||||||
|
* It is here rather than only in the component tier for the reason
|
||||||
|
* `dialog-names.spec.ts` gives: **only Playwright computes an
|
||||||
|
* accessible name.** The Vitest tier can assert the internal label
|
||||||
|
* carries the text and the IDREF still resolves to it; it cannot say
|
||||||
|
* whether anything would announce it. `getByRole('slider', { name })`
|
||||||
|
* matched nothing in this app before the fix.
|
||||||
|
*/
|
||||||
|
test.describe('a control says what it controls', () => {
|
||||||
|
test('the seek bar is announced as Seek', async ({ app }) => {
|
||||||
|
await expect(
|
||||||
|
app.getByRole('slider', { name: 'Seek' }),
|
||||||
|
).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('the volume slider is announced as Volume', async ({ app }) => {
|
||||||
|
// The popup renders no slider at all while closed, the same way the
|
||||||
|
// queue panel renders no list — so this has to open it first.
|
||||||
|
await app.getByRole('button', { name: /volume/i }).click();
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
app.getByRole('slider', { name: 'Volume' }),
|
||||||
|
).toBeVisible();
|
||||||
|
|
||||||
|
// Leave the transport as it was found: the specs share one page in
|
||||||
|
// file order, and an open popup covers the buttons beneath it.
|
||||||
|
await app.keyboard.press('Escape');
|
||||||
|
await app.locator('body').click({ position: { x: 5, y: 5 } });
|
||||||
|
});
|
||||||
|
|
||||||
|
test('naming the slider did not move the transport', async ({ app }) => {
|
||||||
|
// `#slider` takes an 8px margin-block-start the moment a label
|
||||||
|
// exists, so the fix that gives it a name also grows it from 6px to
|
||||||
|
// 14px unless the margin is put back. Nothing else in this app
|
||||||
|
// would fail if it did — the bar would simply sit lower.
|
||||||
|
const height = await app
|
||||||
|
.getByRole('slider', { name: 'Seek' })
|
||||||
|
.evaluate((el) => el.getBoundingClientRect().height);
|
||||||
|
|
||||||
|
expect(height).toBeLessThan(10);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -5,6 +5,7 @@ import WaSlider from '@awesome.me/webawesome/dist/components/slider/slider.js';
|
|||||||
import { formatSeconds } from '@utils/time';
|
import { formatSeconds } from '@utils/time';
|
||||||
import { PlayerController } from '@store/controllers/player-controller';
|
import { PlayerController } from '@store/controllers/player-controller';
|
||||||
import { designTokens } from '../../../styles/tokens.css';
|
import { designTokens } from '../../../styles/tokens.css';
|
||||||
|
import { waSliderLabel } from '../../../styles/wa-slider-label.css';
|
||||||
|
|
||||||
const ProgressIntervalMillis = 1000;
|
const ProgressIntervalMillis = 1000;
|
||||||
|
|
||||||
@@ -25,7 +26,7 @@ export class SeekBar extends LitElement {
|
|||||||
@state()
|
@state()
|
||||||
private showRemaining: boolean = true;
|
private showRemaining: boolean = true;
|
||||||
|
|
||||||
static override styles = [designTokens, css`
|
static override styles = [designTokens, waSliderLabel, css`
|
||||||
wa-slider {
|
wa-slider {
|
||||||
--track-size: 6px;
|
--track-size: 6px;
|
||||||
flex: 1;
|
flex: 1;
|
||||||
@@ -210,7 +211,7 @@ export class SeekBar extends LitElement {
|
|||||||
<div id="seek-bar-container">
|
<div id="seek-bar-container">
|
||||||
<small data-testid="elapsed-time">${elapsedTime}</small>
|
<small data-testid="elapsed-time">${elapsedTime}</small>
|
||||||
<wa-slider
|
<wa-slider
|
||||||
aria-label="Seek"
|
label="Seek"
|
||||||
.value="${this.seekValue}"
|
.value="${this.seekValue}"
|
||||||
max="${this.trackLength}"
|
max="${this.trackLength}"
|
||||||
?with-tooltip="${this.hasTrack}"
|
?with-tooltip="${this.hasTrack}"
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import '@awesome.me/webawesome/dist/components/slider/slider.js';
|
|||||||
import type WaSlider from '@awesome.me/webawesome/dist/components/slider/slider.js';
|
import type WaSlider from '@awesome.me/webawesome/dist/components/slider/slider.js';
|
||||||
import { PlayerController } from '@store/controllers/player-controller';
|
import { PlayerController } from '@store/controllers/player-controller';
|
||||||
import { designTokens } from '../../../styles/tokens.css';
|
import { designTokens } from '../../../styles/tokens.css';
|
||||||
|
import { waSliderLabel } from '../../../styles/wa-slider-label.css';
|
||||||
|
|
||||||
/** Volume change (0-100) applied per scroll-wheel tick. */
|
/** Volume change (0-100) applied per scroll-wheel tick. */
|
||||||
const WHEEL_STEP = 5;
|
const WHEEL_STEP = 5;
|
||||||
@@ -28,7 +29,7 @@ export class VolumeControl extends LitElement {
|
|||||||
@state()
|
@state()
|
||||||
private pendingVolume: number | null = null;
|
private pendingVolume: number | null = null;
|
||||||
|
|
||||||
static override styles = [designTokens, css`
|
static override styles = [designTokens, waSliderLabel, css`
|
||||||
:host {
|
:host {
|
||||||
position: relative;
|
position: relative;
|
||||||
display: inline-flex;
|
display: inline-flex;
|
||||||
@@ -209,6 +210,7 @@ export class VolumeControl extends LitElement {
|
|||||||
@click="${this.handlePopupClick}"
|
@click="${this.handlePopupClick}"
|
||||||
>
|
>
|
||||||
<wa-slider
|
<wa-slider
|
||||||
|
label="Volume"
|
||||||
orientation="vertical"
|
orientation="vertical"
|
||||||
min="0"
|
min="0"
|
||||||
max="100"
|
max="100"
|
||||||
|
|||||||
@@ -268,6 +268,7 @@ export class JobRow extends LitElement {
|
|||||||
|
|
||||||
return html`
|
return html`
|
||||||
<wa-progress-bar
|
<wa-progress-bar
|
||||||
|
label=${job.title}
|
||||||
value=${progressPercent(job) ?? 0}
|
value=${progressPercent(job) ?? 0}
|
||||||
></wa-progress-bar>
|
></wa-progress-bar>
|
||||||
`;
|
`;
|
||||||
|
|||||||
@@ -0,0 +1,41 @@
|
|||||||
|
import { css } from 'lit';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A `wa-slider` is named by its `label`, and its `label` is visible.
|
||||||
|
*
|
||||||
|
* The same trap as `utils/name-dialog.ts`, one component over, and this
|
||||||
|
* one the audit filed under "what is already correct": `a11y.md` lists
|
||||||
|
* `seek-bar` and `volume-control` as exemplary because they pass
|
||||||
|
* `aria-label`. The role is not on the host. Web Awesome renders a
|
||||||
|
* `<div id="slider" role="slider" aria-labelledby="label">` inside its
|
||||||
|
* own shadow root, pointing at an internal `<label id="label">` that is
|
||||||
|
* empty unless the `label` property is set — and `aria-labelledby`
|
||||||
|
* outranks the host's `aria-label`, which the AX tree never sees. Both
|
||||||
|
* sliders in this app computed a name of `""`. Measured with
|
||||||
|
* `Accessibility.getFullAXTree` against the running app, on all eleven
|
||||||
|
* views; `volume-control` did not even have the `aria-label` the audit
|
||||||
|
* credits it with.
|
||||||
|
*
|
||||||
|
* So the name comes from `label`, which is the library's own API, and
|
||||||
|
* this hides it. That is preferred over reaching into the shadow root
|
||||||
|
* the way `name-dialog.ts` has to, for the failure mode: if Web Awesome
|
||||||
|
* renames these parts the label becomes *visible* — wrong-looking and
|
||||||
|
* correctly named — rather than silently nameless again.
|
||||||
|
*
|
||||||
|
* The second rule is not decoration. `#slider` takes an 8px
|
||||||
|
* `margin-block-start` as soon as a label exists, so hiding the label
|
||||||
|
* alone still grows the control from 6px to 14px and moves the transport
|
||||||
|
* bar. `display: none` on the label is deliberate and safe: an element
|
||||||
|
* referenced by `aria-labelledby` contributes its text even when it is
|
||||||
|
* hidden, which is exactly the accname rule this relies on (verified —
|
||||||
|
* the slider reports "Seek" with the label displaying nothing).
|
||||||
|
*/
|
||||||
|
export const waSliderLabel = css`
|
||||||
|
wa-slider::part(label) {
|
||||||
|
display: none;
|
||||||
|
}
|
||||||
|
|
||||||
|
wa-slider::part(slider) {
|
||||||
|
margin-block-start: 0;
|
||||||
|
}
|
||||||
|
`;
|
||||||
@@ -375,7 +375,11 @@ describe('<seek-bar>', () => {
|
|||||||
it('carries an accessible name, since it is otherwise an unlabelled slider', async () => {
|
it('carries an accessible name, since it is otherwise an unlabelled slider', async () => {
|
||||||
const el = await fixture('seek-bar');
|
const el = await fixture('seek-bar');
|
||||||
|
|
||||||
expect(shadow(el, 'wa-slider')?.getAttribute('aria-label')).toBe('Seek');
|
// This asserted `aria-label` on the host for six phases, and the
|
||||||
|
// host is not what carries `role="slider"` — the name never
|
||||||
|
// reached the accessibility tree. `wa-control-names.test.ts` is
|
||||||
|
// the whole story; the name now comes from `label`.
|
||||||
|
expect(shadow(el, 'wa-slider')?.getAttribute('label')).toBe('Seek');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('looks the way it did last time', async () => {
|
it('looks the way it did last time', async () => {
|
||||||
|
|||||||
@@ -0,0 +1,122 @@
|
|||||||
|
/**
|
||||||
|
* Two Web Awesome controls put the role somewhere the host's
|
||||||
|
* `aria-label` cannot reach it, and this pins the way out of each.
|
||||||
|
*
|
||||||
|
* `a11y.md` lists `seek-bar` and `volume-control` under **what is
|
||||||
|
* already correct** ("`wa-slider` with `aria-label`"). Measured against
|
||||||
|
* the running app with `Accessibility.getFullAXTree`, both sliders
|
||||||
|
* computed a name of `""` on all eleven views: the role is on a
|
||||||
|
* `<div id="slider" aria-labelledby="label">` inside `wa-slider`'s own
|
||||||
|
* shadow root, and an `aria-labelledby` pointing at an empty internal
|
||||||
|
* `<label>` outranks the host's `aria-label`. `volume-control` did not
|
||||||
|
* have the `aria-label` the audit credits it with in the first place.
|
||||||
|
*
|
||||||
|
* `a11y.25` is the same family, and the fix is the library's own API in
|
||||||
|
* both cases — `label` — which for a progress bar is invisible and for a
|
||||||
|
* slider is not, hence `styles/wa-slider-label.css.ts`.
|
||||||
|
*
|
||||||
|
* What this tier checks is the *wiring*: that the internal label carries
|
||||||
|
* the text, that the IDREF still points at it, and that hiding it does
|
||||||
|
* not move the control. Computing an accessible name is Playwright's
|
||||||
|
* job, and `e2e/specs/control-names.spec.ts` does it there.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import '@awesome.me/webawesome/dist/components/slider/slider.js';
|
||||||
|
import '@awesome.me/webawesome/dist/components/progress-bar/progress-bar.js';
|
||||||
|
import '@components/audio-player/seekbar/seek-bar';
|
||||||
|
import '@components/audio-player/volume-control/volume-control';
|
||||||
|
import '@components/jobs/job-row';
|
||||||
|
import { fixture, shadow } from '@test/support/render';
|
||||||
|
|
||||||
|
/** The element Web Awesome puts `role="slider"` on, and its name source. */
|
||||||
|
function sliderName(wa: Element | null): string | null {
|
||||||
|
const inner = wa?.shadowRoot?.querySelector('[role="slider"]');
|
||||||
|
const id = inner?.getAttribute('aria-labelledby');
|
||||||
|
|
||||||
|
if (!id) return null;
|
||||||
|
|
||||||
|
return wa?.shadowRoot?.getElementById(id)?.textContent?.trim() ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('naming a wa-slider', () => {
|
||||||
|
it('names the seek bar through the internal label, not the host', async () => {
|
||||||
|
const el = await fixture('seek-bar');
|
||||||
|
const wa = shadow(el, 'wa-slider');
|
||||||
|
|
||||||
|
await (wa as HTMLElement & { updateComplete: Promise<unknown> })
|
||||||
|
.updateComplete;
|
||||||
|
|
||||||
|
// The positive case: the name a screen reader would compute is
|
||||||
|
// reachable from the element that carries the role.
|
||||||
|
expect(sliderName(wa)).toBe('Seek');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves the seek bar the height it was without a label', async () => {
|
||||||
|
const el = await fixture('seek-bar');
|
||||||
|
const wa = shadow(el, 'wa-slider') as HTMLElement;
|
||||||
|
|
||||||
|
await (wa as HTMLElement & { updateComplete: Promise<unknown> })
|
||||||
|
.updateComplete;
|
||||||
|
|
||||||
|
// `#slider` takes an 8px margin-block-start as soon as a label
|
||||||
|
// exists, so hiding the label alone still grows the control from
|
||||||
|
// 6px to 14px and moves the transport bar with it.
|
||||||
|
const label = wa.shadowRoot?.querySelector('[part~="label"]') as HTMLElement;
|
||||||
|
|
||||||
|
expect(getComputedStyle(label).display).toBe('none');
|
||||||
|
expect(wa.getBoundingClientRect().height).toBeLessThan(10);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('names the volume slider, which had no name of any kind', async () => {
|
||||||
|
const el = await fixture('volume-control');
|
||||||
|
const trigger = shadow<HTMLElement>(el, 'button');
|
||||||
|
|
||||||
|
trigger?.click();
|
||||||
|
await (el as HTMLElement & { updateComplete: Promise<unknown> })
|
||||||
|
.updateComplete;
|
||||||
|
|
||||||
|
const wa = shadow(el, 'wa-slider');
|
||||||
|
|
||||||
|
await (wa as HTMLElement & { updateComplete: Promise<unknown> })
|
||||||
|
.updateComplete;
|
||||||
|
|
||||||
|
expect(sliderName(wa)).toBe('Volume');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('naming a wa-progress-bar', () => {
|
||||||
|
it('says what is progressing rather than "progress"', async () => {
|
||||||
|
const el = await fixture('job-row', {
|
||||||
|
job: {
|
||||||
|
id: 'j1',
|
||||||
|
title: 'Scanning Music',
|
||||||
|
kind: 'library-scan',
|
||||||
|
state: 'running',
|
||||||
|
current: 45,
|
||||||
|
total: 100,
|
||||||
|
caps: { pausable: false, cancellable: false },
|
||||||
|
stages: [],
|
||||||
|
stats: [],
|
||||||
|
startedAt: 0,
|
||||||
|
updatedAt: 0,
|
||||||
|
logCount: 0,
|
||||||
|
warnCount: 0,
|
||||||
|
errorCount: 0,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const bar = shadow(el, 'wa-progress-bar');
|
||||||
|
|
||||||
|
await (bar as HTMLElement & { updateComplete: Promise<unknown> })
|
||||||
|
.updateComplete;
|
||||||
|
|
||||||
|
// Web Awesome maps `label` onto the inner role="progressbar"'s
|
||||||
|
// aria-label, falling back to the localised word "progress" — so
|
||||||
|
// this was never *unnamed*, it was named after the widget instead
|
||||||
|
// of after the work.
|
||||||
|
const inner = bar?.shadowRoot?.querySelector('[role="progressbar"]');
|
||||||
|
|
||||||
|
expect(inner?.getAttribute('aria-label')).toBe('Scanning Music');
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user