fix(a11y): give every wa-dialog an accessible name
Eleven dialogs passed a `label` that never reached the accessibility
tree: Web Awesome renders it into an <h2 id="title"> in the same shadow
root as the native <dialog> and never points aria-labelledby at it, so
getByRole('dialog', {name}) matched nothing and a screen reader
announced an unnamed dialog. a11y.md lists all of them under "what is
already correct".
utils/name-dialog.ts sets the IDREF, with aria-label as the fallback for
without-header (first-run-wizard), called from each host's updated().
aria-labelledby rather than aria-label because three call sites compute
their label at render time, and the heading re-renders anyway. It waits
for the dialog's own first update: wa-dialog populates its shadow root
in its own update, so a query at the host's firstUpdated names nothing.
Reaching into another library's open shadow root is deliberate and the
failure is bounded — if the structure moves, the query misses and the
dialog is as unnamed as it was.
This commit is contained in:
@@ -25,6 +25,7 @@ import { inlineDiff, normalizeStrict, isCosmeticDiff } from '../../utils/text-di
|
||||
import { libraryStore } from '../../store/library-store';
|
||||
import { notificationStore } from '../../store/notification-store';
|
||||
import { describeError, explainError } from '../../utils/describe-error';
|
||||
import { nameDialogsIn } from '../../utils/name-dialog';
|
||||
import { ViewLifecycleMixin } from '../../utils/view-lifecycle';
|
||||
import { confirmAction } from '../confirm-dialog/confirm-dialog';
|
||||
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||
@@ -3042,6 +3043,15 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) {
|
||||
return html`${this.renderPasteDialog()}${this.renderSearchDialog()}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Web Awesome renders `label` into a heading it never points the
|
||||
* `<dialog>` at, so the dialog has no accessible name until
|
||||
* something sets one. See `utils/name-dialog.ts`.
|
||||
*/
|
||||
override updated() {
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
}
|
||||
|
||||
override render() {
|
||||
// The layout chrome always renders immediately; each pane owns
|
||||
// its own skeleton while its data resolves, so the user never
|
||||
|
||||
@@ -17,6 +17,7 @@ import { customElement, query, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import { nameDialogsIn } from '../../utils/name-dialog';
|
||||
|
||||
export interface ConfirmRequest {
|
||||
title: string;
|
||||
@@ -103,6 +104,15 @@ export class ConfirmDialog extends LitElement {
|
||||
settle?.(ok);
|
||||
}
|
||||
|
||||
/**
|
||||
* Web Awesome renders `label` into a heading it never points the
|
||||
* `<dialog>` at, so the dialog has no accessible name until
|
||||
* something sets one. See `utils/name-dialog.ts`.
|
||||
*/
|
||||
override updated() {
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
}
|
||||
|
||||
override render() {
|
||||
const request = this.request;
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import { downloadStore } from '@store/download-store';
|
||||
import type { download } from '@go/models';
|
||||
import './candidate-row';
|
||||
import { explainError } from '@utils/describe-error';
|
||||
import { nameDialogsIn } from '@utils/name-dialog';
|
||||
|
||||
/**
|
||||
* The "find this album" dialog: searches every enabled download client,
|
||||
@@ -119,6 +120,11 @@ export class DownloadPicker extends LitElement {
|
||||
];
|
||||
|
||||
override updated(changed: Map<string, unknown>) {
|
||||
// Web Awesome renders `label` into a heading it never points
|
||||
// the `<dialog>` at, so the dialog has no accessible name until
|
||||
// something sets one. See `utils/name-dialog.ts`.
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
|
||||
if (changed.has('open') && this.open) {
|
||||
void this.search();
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/switch/switch.js';
|
||||
import { AddTracksToPlaylist } from '@go/playlist/Service';
|
||||
import { formatMilliseconds } from '@utils/time';
|
||||
import { nameDialogsIn } from '@utils/name-dialog';
|
||||
|
||||
interface DuplicateTrack {
|
||||
FilePath: string;
|
||||
@@ -271,6 +272,15 @@ export class DuplicateTracksDialog extends LitElement {
|
||||
// RENDER
|
||||
// =================================================================
|
||||
|
||||
/**
|
||||
* Web Awesome renders `label` into a heading it never points the
|
||||
* `<dialog>` at, so the dialog has no accessible name until
|
||||
* something sets one. See `utils/name-dialog.ts`.
|
||||
*/
|
||||
override updated() {
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
}
|
||||
|
||||
override render() {
|
||||
const current = this.duplicates[this.currentIndex];
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
} from '@go/library/Library';
|
||||
import { DirectoryPicker } from '@go/frontendutil/FrontendUtil';
|
||||
import { describeError, explainError } from '@utils/describe-error';
|
||||
import { nameDialogsIn } from '@utils/name-dialog';
|
||||
|
||||
/**
|
||||
* First-run setup wizard.
|
||||
@@ -164,6 +165,15 @@ export class FirstRunWizard extends LitElement {
|
||||
}
|
||||
`;
|
||||
|
||||
/**
|
||||
* Web Awesome renders `label` into a heading it never points the
|
||||
* `<dialog>` at, so the dialog has no accessible name until
|
||||
* something sets one. See `utils/name-dialog.ts`.
|
||||
*/
|
||||
override updated() {
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
}
|
||||
|
||||
override render() {
|
||||
if (!this.active) return nothing;
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||
|
||||
import { notificationStore } from '@store/notification-store';
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import { nameDialogsIn } from '../../utils/name-dialog';
|
||||
|
||||
import { noticeStyles, renderNotice } from './notice';
|
||||
|
||||
@@ -151,6 +152,15 @@ export class NotificationHost extends LitElement {
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Web Awesome renders `label` into a heading it never points the
|
||||
* `<dialog>` at, so the dialog has no accessible name until
|
||||
* something sets one. See `utils/name-dialog.ts`.
|
||||
*/
|
||||
override updated() {
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
}
|
||||
|
||||
override render() {
|
||||
const stacked = [
|
||||
...notificationStore.byLevel('persistent'),
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
} from '@go/playlist/Service';
|
||||
import type { playlist } from '@go/models';
|
||||
import { formatMilliseconds } from '@utils/time';
|
||||
import { nameDialogsIn } from '@utils/name-dialog';
|
||||
|
||||
const SEARCH_DEBOUNCE_MS = 400;
|
||||
|
||||
@@ -922,6 +923,15 @@ export class PhantomResolver extends LitElement {
|
||||
`,
|
||||
];
|
||||
|
||||
/**
|
||||
* Web Awesome renders `label` into a heading it never points the
|
||||
* `<dialog>` at, so the dialog has no accessible name until
|
||||
* something sets one. See `utils/name-dialog.ts`.
|
||||
*/
|
||||
override updated() {
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
}
|
||||
|
||||
override render() {
|
||||
return html`
|
||||
<wa-dialog
|
||||
|
||||
@@ -21,6 +21,7 @@ import { customElement, query, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import { nameDialogsIn } from '../../utils/name-dialog';
|
||||
import {
|
||||
SHORTCUT_CATEGORIES,
|
||||
SHORTCUT_META,
|
||||
@@ -158,6 +159,15 @@ export class ShortcutsOverlay extends LitElement {
|
||||
this.isOpen = false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Web Awesome renders `label` into a heading it never points the
|
||||
* `<dialog>` at, so the dialog has no accessible name until
|
||||
* something sets one. See `utils/name-dialog.ts`.
|
||||
*/
|
||||
override updated() {
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
}
|
||||
|
||||
override render() {
|
||||
if (!this.isOpen) return nothing;
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
formatFileSize,
|
||||
} from '@utils/format';
|
||||
import { formatMilliseconds } from '@utils/time';
|
||||
import { nameDialogsIn } from '@utils/name-dialog';
|
||||
import { WriteTrackTagsByPath } from '@go/tagwriter/TagWriter';
|
||||
import {
|
||||
BatchWriteTrackTags,
|
||||
@@ -726,6 +727,15 @@ export class TrackDetails extends LitElement {
|
||||
// RENDER
|
||||
// =================================================================
|
||||
|
||||
/**
|
||||
* Web Awesome renders `label` into a heading it never points the
|
||||
* `<dialog>` at, so the dialog has no accessible name until
|
||||
* something sets one. See `utils/name-dialog.ts`.
|
||||
*/
|
||||
override updated() {
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
}
|
||||
|
||||
override render() {
|
||||
const label = this.batchMode
|
||||
? 'Batch Edit'
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
/**
|
||||
* Every `wa-dialog` in this app is an unnamed dialog. This names them.
|
||||
*
|
||||
* `a11y.md` lists all of them under "what is already correct" and notes
|
||||
* that every call site passes a `label` — both true, and the label never
|
||||
* reaches the accessibility tree. Web Awesome renders it into an
|
||||
* `<h2 part="title" id="title">` in the *same shadow root* as the native
|
||||
* `<dialog part="dialog">` and never points `aria-labelledby` at it, so
|
||||
* `getByRole('dialog', {name})` matches nothing and a screen reader
|
||||
* announces an unnamed dialog. Confirmed in the installed source
|
||||
* (`dist/chunks/chunk.ZUIYLL2X.js`, `WaDialog.render`).
|
||||
*
|
||||
* Three decisions are worth stating, because each has a failure mode:
|
||||
*
|
||||
* **This reaches into another library's shadow root**, which is open and
|
||||
* therefore reachable, but is not API. It is acceptable here precisely
|
||||
* because the failure is bounded: if Web Awesome changes the structure,
|
||||
* the query returns null, the helper does nothing, and the dialog is as
|
||||
* unnamed as it is today. There is no state to get wrong and nothing to
|
||||
* throw. The alternative — patching `WaDialog.prototype` — fixes every
|
||||
* call site for free and fails loudly and strangely instead.
|
||||
*
|
||||
* **It prefers `aria-labelledby` over `aria-label`.** Three call sites
|
||||
* have a label that changes after the first render (`track-details`,
|
||||
* `confirm-dialog`, `notification-host`), and an IDREF to the `<h2>` the
|
||||
* dialog re-renders anyway stays correct for free. `aria-label` is the
|
||||
* fallback for `without-header`, which renders no `<h2>` at all
|
||||
* (`first-run-wizard` is the one such caller).
|
||||
*
|
||||
* **And it waits for the dialog's own first update, not its host's.**
|
||||
* `wa-dialog` is a Lit element: its shadow root is populated in *its*
|
||||
* update, which has not run when the host reaches `firstUpdated`. A
|
||||
* synchronous query there finds an element with an empty shadow root
|
||||
* and names nothing — the same trap that made `wa-dropdown-item`'s role
|
||||
* invisible to the menu keyboard model two passes ago.
|
||||
*/
|
||||
|
||||
/** The id Web Awesome gives its title heading, within its own shadow root. */
|
||||
const WA_TITLE_ID = 'title';
|
||||
|
||||
/** A `wa-dialog` element, as much of it as this file needs. */
|
||||
type DialogHost = Element & {
|
||||
label?: string;
|
||||
updateComplete?: Promise<unknown>;
|
||||
};
|
||||
|
||||
/**
|
||||
* Dialogs already pointed at their own heading.
|
||||
*
|
||||
* Hosts call this from `updated()`, which runs on every pass, so the
|
||||
* steady-state cost has to be a lookup rather than an attribute write.
|
||||
* Only the IDREF path is cached: it cannot go stale, because the `<h2>`
|
||||
* it names re-renders with the label. The `aria-label` fallback is
|
||||
* re-applied every call, since a copied string can.
|
||||
*/
|
||||
const named = new WeakSet<Element>();
|
||||
|
||||
/** Set the name, if the shadow root is there to set it on. Returns success. */
|
||||
function applyName(host: DialogHost): boolean {
|
||||
const root = host.shadowRoot;
|
||||
|
||||
if (!root) return false;
|
||||
|
||||
const dialog = root.querySelector('dialog[part~="dialog"]');
|
||||
|
||||
if (!dialog) return false;
|
||||
|
||||
// The header, and so the heading the name comes from, is absent
|
||||
// under `without-header`.
|
||||
if (root.querySelector(`#${WA_TITLE_ID}`)) {
|
||||
dialog.setAttribute('aria-labelledby', WA_TITLE_ID);
|
||||
dialog.removeAttribute('aria-label');
|
||||
named.add(host);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
const label = host.label ?? host.getAttribute('label') ?? '';
|
||||
|
||||
if (label) {
|
||||
dialog.setAttribute('aria-label', label);
|
||||
dialog.removeAttribute('aria-labelledby');
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Point the native `<dialog>` inside a `<wa-dialog>` at its own title.
|
||||
*
|
||||
* Safe to call before the dialog has rendered, and safe to call more
|
||||
* than once — it is a no-op when there is nothing to name. Call it from
|
||||
* the host's `firstUpdated()`, or from `updated()` for a host that
|
||||
* renders its dialog conditionally.
|
||||
*/
|
||||
export function nameDialog(host: DialogHost | null | undefined): void {
|
||||
if (!host || named.has(host)) return;
|
||||
|
||||
if (applyName(host)) return;
|
||||
|
||||
// Not rendered yet: wait for the dialog's own update, then retry
|
||||
// once. Anything still missing after that is a structure this
|
||||
// helper does not recognise, which is the bounded failure above.
|
||||
void host.updateComplete?.then(() => {
|
||||
applyName(host);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Name every `wa-dialog` rendered in `root`.
|
||||
*
|
||||
* The hosts that render two dialogs, or render one conditionally, use
|
||||
* this rather than tracking a `@query` each.
|
||||
*/
|
||||
export function nameDialogsIn(root: ShadowRoot | null | undefined): void {
|
||||
if (!root) return;
|
||||
|
||||
for (const host of root.querySelectorAll('wa-dialog')) {
|
||||
nameDialog(host as DialogHost);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,150 @@
|
||||
/**
|
||||
* Every `wa-dialog` in this app was an unnamed dialog.
|
||||
*
|
||||
* `a11y.md` lists all of them under "what is already correct" and notes
|
||||
* that every call site passes a `label`. Both true, and the label never
|
||||
* reached the accessibility tree: Web Awesome renders it into an
|
||||
* `<h2 id="title">` in the same shadow root as the native `<dialog>` and
|
||||
* never points `aria-labelledby` at it.
|
||||
*
|
||||
* What this tier can and cannot check is worth stating, because the
|
||||
* distinction is the whole reason the e2e spec also exists. It queries
|
||||
* shadow roots directly, so it can assert the *wiring* — the IDREF, and
|
||||
* that it resolves to a heading carrying the label. It cannot compute an
|
||||
* accessible name; `getByRole('dialog', {name})` is Playwright's job,
|
||||
* and `e2e/specs/dialog-names.spec.ts` does it there.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||
import { confirmAction } from '@components/confirm-dialog/confirm-dialog';
|
||||
import '@components/shortcuts-overlay/shortcuts-overlay';
|
||||
import { fixture, shadow } from '@test/support/render';
|
||||
import { nameDialog, nameDialogsIn } from '@utils/name-dialog';
|
||||
|
||||
/** The `<dialog>` Web Awesome renders inside a `<wa-dialog>`. */
|
||||
function native(host: Element | null): HTMLElement | null {
|
||||
return host?.shadowRoot?.querySelector<HTMLElement>('dialog[part~="dialog"]')
|
||||
?? null;
|
||||
}
|
||||
|
||||
/** What an IDREF on that dialog actually resolves to, in its own root. */
|
||||
function labelledByText(host: Element | null): string | null {
|
||||
const dialog = native(host);
|
||||
const id = dialog?.getAttribute('aria-labelledby');
|
||||
|
||||
if (!id) return null;
|
||||
|
||||
return host?.shadowRoot?.getElementById(id)?.textContent?.trim() ?? null;
|
||||
}
|
||||
|
||||
/** Mount a bare `wa-dialog`, name it, and wait for both updates. */
|
||||
async function namedDialog(
|
||||
attrs: Record<string, string>,
|
||||
): Promise<HTMLElement> {
|
||||
const el = document.createElement('wa-dialog');
|
||||
|
||||
for (const [k, v] of Object.entries(attrs)) el.setAttribute(k, v);
|
||||
|
||||
document.body.append(el);
|
||||
await (el as HTMLElement & { updateComplete: Promise<unknown> })
|
||||
.updateComplete;
|
||||
|
||||
nameDialog(el);
|
||||
await (el as HTMLElement & { updateComplete: Promise<unknown> })
|
||||
.updateComplete;
|
||||
|
||||
return el;
|
||||
}
|
||||
|
||||
describe('naming a wa-dialog', () => {
|
||||
it('is unnamed until something names it — which is the bug', async () => {
|
||||
const el = document.createElement('wa-dialog');
|
||||
|
||||
el.setAttribute('label', 'Nobody announces this');
|
||||
document.body.append(el);
|
||||
await (el as HTMLElement & { updateComplete: Promise<unknown> })
|
||||
.updateComplete;
|
||||
|
||||
const dialog = native(el);
|
||||
|
||||
expect(dialog).toBeTruthy();
|
||||
expect(dialog!.getAttribute('aria-labelledby')).toBeNull();
|
||||
expect(dialog!.getAttribute('aria-label')).toBeNull();
|
||||
|
||||
el.remove();
|
||||
});
|
||||
|
||||
it('points the dialog at the heading carrying the label', async () => {
|
||||
const el = await namedDialog({ label: 'Duplicate Tracks Found' });
|
||||
|
||||
expect(labelledByText(el)).toBe('Duplicate Tracks Found');
|
||||
|
||||
el.remove();
|
||||
});
|
||||
|
||||
it('falls back to aria-label when there is no header to point at', async () => {
|
||||
// `first-run-wizard` is the one caller using `without-header`, so
|
||||
// the IDREF path has no `<h2>` to name and the string is copied.
|
||||
const el = await namedDialog({
|
||||
label: 'Welcome to YellowJacket',
|
||||
'without-header': '',
|
||||
});
|
||||
|
||||
expect(native(el)!.getAttribute('aria-labelledby')).toBeNull();
|
||||
expect(native(el)!.getAttribute('aria-label')).toBe(
|
||||
'Welcome to YellowJacket',
|
||||
);
|
||||
|
||||
el.remove();
|
||||
});
|
||||
|
||||
it('is a no-op on anything that is not a wa-dialog', () => {
|
||||
// The bounded failure mode: if Web Awesome moves this structure the
|
||||
// query misses, nothing is written, and the dialog is exactly as
|
||||
// unnamed as it is today.
|
||||
expect(() => nameDialog(document.createElement('div'))).not.toThrow();
|
||||
expect(() => nameDialogsIn(null)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('the dialogs the app actually opens', () => {
|
||||
it('names confirm-dialog from the title the caller passed', async () => {
|
||||
const answer = confirmAction({
|
||||
title: 'Remove “Live Sessions”?',
|
||||
message: 'The library entry is removed; the files are not.',
|
||||
});
|
||||
|
||||
const el = document.querySelector('confirm-dialog')!;
|
||||
|
||||
await (el as HTMLElement & { updateComplete: Promise<unknown> })
|
||||
.updateComplete;
|
||||
|
||||
const dialog = shadow(el, 'wa-dialog');
|
||||
|
||||
await (dialog as HTMLElement & { updateComplete: Promise<unknown> })
|
||||
.updateComplete;
|
||||
|
||||
expect(labelledByText(dialog)).toBe('Remove “Live Sessions”?');
|
||||
|
||||
el.shadowRoot
|
||||
?.querySelector<HTMLButtonElement>('[data-testid="confirm-cancel"]')
|
||||
?.click();
|
||||
await answer;
|
||||
});
|
||||
|
||||
it('names the shortcuts overlay, which renders its dialog on demand', async () => {
|
||||
const el = await fixture('shortcuts-overlay');
|
||||
|
||||
document.dispatchEvent(new CustomEvent('shortcut:app-shortcuts'));
|
||||
await (el as HTMLElement & { updateComplete: Promise<unknown> })
|
||||
.updateComplete;
|
||||
|
||||
const dialog = shadow(el, 'wa-dialog');
|
||||
|
||||
await (dialog as HTMLElement & { updateComplete: Promise<unknown> })
|
||||
.updateComplete;
|
||||
|
||||
expect(labelledByText(dialog)).toBe('Keyboard Shortcuts');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user