fix(a11y): make Settings and the Downloads tabs keyboard-reachable

a11y.1 is the audit's last Critical and reproduced exactly: seven
config-section headers, seven bare `<div @click>`s with no tabindex,
no role and no aria-expanded, and every section collapsed by default —
so every setting in the app was behind a control that could not be
tabbed to. a11y.2 is the same bug in Downloads' two `<div class=tab>`s.

Both now follow patterns the app already had: a real
`<button aria-expanded aria-controls>` (explore-artist-details has five),
and a role=tablist/tab/tabpanel with a roving tab stop and
Left/Right/Home/End. The section body renders unconditionally and is
toggled with `hidden`, because aria-controls has to name an element
that exists and the slot's light-DOM children exist either way.

H-22's reorder ships with them: Libraries is first and the only
expanded section, Search Index — configured once, if ever — is second
to last. The Playback/Audio section H-22 also asks for is deliberately
not here: there is no output-device, gapless, crossfade or replay-gain
setting in backend/config to expose, and a section of controls that do
nothing is worse than admitting it does not exist.

Settings also stops advertising `tracklist.delete`, which was bound to
Delete and configurable in the UI while nothing listened for the event
it dispatched.
This commit is contained in:
2026-08-12 12:12:33 -04:00
parent a150b24e71
commit 24887d6840
6 changed files with 240 additions and 42 deletions
+9 -3
View File
@@ -32,9 +32,15 @@ func DefaultBindings() map[string]string {
// App actions (Global scope, Ctrl modifier) // App actions (Global scope, Ctrl modifier)
"app.selectAll": "Ctrl+A", "app.selectAll": "Ctrl+A",
// Panel-specific (track list) // Panel-specific (track list). There is no `tracklist.delete`:
"tracklist.play": "Enter", // it was bound to Delete and advertised in Settings as
"tracklist.delete": "Delete", // configurable while nothing listened for it, because "remove
// from library" does not exist and it is not clear what it would
// remove — the row (which the next scan puts back unless the path
// is also excluded) or the file (a delete-your-music button one
// keystroke from a focused row). Advertise it again when it does
// something.
"tracklist.play": "Enter",
// Panel-specific (autotag review). These are the keys the // Panel-specific (autotag review). These are the keys the
// autotag page used to bind on its own document listener, which // autotag page used to bind on its own document listener, which
@@ -163,12 +163,6 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
scope: 'panel:track-list', scope: 'panel:track-list',
defaultKey: 'Enter', defaultKey: 'Enter',
}, },
'tracklist.delete': {
label: 'Remove Selected',
category: 'Navigation',
scope: 'panel:track-list',
defaultKey: 'Delete',
},
'autotag.apply': { 'autotag.apply': {
label: 'Apply Match', label: 'Apply Match',
category: 'Autotag', category: 'Autotag',
@@ -1500,14 +1494,20 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
return html` return html`
<h2>Settings</h2> <h2>Settings</h2>
${this.renderSearchSection()} <!--
Ordered by how often a setting is *reached*, not by how
the sections were written (H-22). Libraries was last and
below the fold while Search Index — which is configured
once, if ever — was first and the only expanded one.
-->
${this.renderLibrarySection()}
${this.renderNowPlayingSection()} ${this.renderNowPlayingSection()}
${this.renderThemeSection()} ${this.renderThemeSection()}
${this.renderFavoritesSection()}
${this.renderTrackListSection()} ${this.renderTrackListSection()}
${this.renderFavoritesSection()}
${this.renderShortcutsSection()} ${this.renderShortcutsSection()}
${this.renderSearchSection()}
<download-clients></download-clients> <download-clients></download-clients>
${this.renderLibrarySection()}
`; `;
} }
@@ -1520,7 +1520,6 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
<config-section <config-section
heading="Search Index" heading="Search Index"
description="The explore search index is built from the MusicBrainz/ListenBrainz data dumps — popular artists, albums, and tracks with listen counts — for fast offline search." description="The explore search index is built from the MusicBrainz/ListenBrainz data dumps — popular artists, albums, and tracks with listen counts — for fast offline search."
.open=${true}
> >
<div class="index-status"> <div class="index-status">
${s ${s
@@ -2035,6 +2034,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
heading="Libraries" heading="Libraries"
description="Manage your music library folders. Scanning and its description="Manage your music library folders. Scanning and its
progress live in the Jobs panel." progress live in the Jobs panel."
.open=${true}
> >
<div class="scan-actions"> <div class="scan-actions">
<button <button
@@ -5,6 +5,18 @@ import { customElement, property, state } from 'lit/decorators.js';
* A visual grouping wrapper for config fields. * A visual grouping wrapper for config fields.
* Renders a collapsible heading with optional description, * Renders a collapsible heading with optional description,
* and a slot for fields. Starts collapsed by default. * and a slot for fields. Starts collapsed by default.
*
* The header is a real `<button aria-expanded aria-controls>`, which
* it was not: it was a bare `<div @click>` with no tabindex and no
* role, and every section defaults to collapsed — so every setting in
* the app sat behind a control that could not be tabbed to (a11y.1).
* `explore-artist-details` has had the correct pattern in five places
* the whole time.
*
* The body renders unconditionally and is toggled with `hidden`,
* rather than being added and removed. `aria-controls` has to name an
* element that exists, and the slot's light-DOM children exist either
* way — a conditional `<slot>` only stops projecting them.
*/ */
@customElement('config-section') @customElement('config-section')
export class ConfigSection extends LitElement { export class ConfigSection extends LitElement {
@@ -24,9 +36,15 @@ export class ConfigSection extends LitElement {
display: flex; display: flex;
align-items: flex-start; align-items: flex-start;
gap: 0.5em; gap: 0.5em;
width: 100%;
padding: 1.25em; padding: 1.25em;
cursor: pointer; cursor: pointer;
user-select: none; user-select: none;
background: none;
border: none;
font: inherit;
color: inherit;
text-align: left;
} }
.header:hover { .header:hover {
@@ -34,6 +52,12 @@ export class ConfigSection extends LitElement {
border-radius: 6px; border-radius: 6px;
} }
.header:focus-visible {
outline: 2px solid var(--yj-accent, #ffc107);
outline-offset: -2px;
border-radius: 6px;
}
.chevron { .chevron {
flex-shrink: 0; flex-shrink: 0;
width: 16px; width: 16px;
@@ -69,6 +93,10 @@ export class ConfigSection extends LitElement {
padding: 0 1.25em 1.25em; padding: 0 1.25em 1.25em;
} }
.body[hidden] {
display: none;
}
.fields { .fields {
display: flex; display: flex;
flex-direction: column; flex-direction: column;
@@ -99,8 +127,11 @@ export class ConfigSection extends LitElement {
override render() { override render() {
return html` return html`
<div class="section"> <div class="section">
<div <button
type="button"
class="header" class="header"
aria-expanded=${this.expanded ? 'true' : 'false'}
aria-controls="section-body"
@click=${this.toggle} @click=${this.toggle}
> >
<svg <svg
@@ -120,16 +151,12 @@ export class ConfigSection extends LitElement {
</p>` </p>`
: nothing} : nothing}
</div> </div>
</button>
<div class="body" id="section-body" ?hidden=${!this.expanded}>
<div class="fields">
<slot></slot>
</div>
</div> </div>
${this.expanded
? html`
<div class="body">
<div class="fields">
<slot></slot>
</div>
</div>
`
: nothing}
</div> </div>
`; `;
} }
@@ -98,10 +98,18 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
font-weight: 600; font-weight: 600;
color: var(--yj-text-secondary, #b3b3b3); color: var(--yj-text-secondary, #b3b3b3);
cursor: pointer; cursor: pointer;
border: none;
border-bottom: 2px solid transparent; border-bottom: 2px solid transparent;
background: none;
font-family: inherit;
user-select: none; user-select: none;
} }
.tab:focus-visible {
outline: 2px solid var(--yj-accent, #ffd43b);
outline-offset: -2px;
}
.tab:hover { .tab:hover {
color: var(--yj-text-primary, #fff); color: var(--yj-text-primary, #fff);
} }
@@ -261,25 +269,79 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
everything on the list immediately. everything on the list immediately.
</p> </p>
<div class="tabs"> <div
<div class="tabs"
class="tab ${this.tab === 'requests' ? 'active' : ''}" role="tablist"
@click=${() => (this.tab = 'requests')} aria-label="Downloads sections"
> @keydown=${this.onTabKeydown}
Requests >
</div> ${DownloadsView.TABS.map(
<div ([id, label]) => html`
class="tab ${this.tab === 'downloads' ? 'active' : ''}" <button
@click=${() => (this.tab = 'downloads')} type="button"
> role="tab"
Downloads id=${`tab-${id}`}
</div> class="tab ${this.tab === id ? 'active' : ''}"
aria-selected=${this.tab === id ? 'true' : 'false'}
aria-controls=${`panel-${id}`}
tabindex=${this.tab === id ? 0 : -1}
@click=${() => (this.tab = id)}
>
${label}
</button>
`,
)}
</div> </div>
${this.tab === 'requests' ? this.renderRequests() : this.renderDownloads()} <div
role="tabpanel"
id=${`panel-${this.tab}`}
aria-labelledby=${`tab-${this.tab}`}
>
${this.tab === 'requests' ? this.renderRequests() : this.renderDownloads()}
</div>
`; `;
} }
/**
* The tabs, in order, so the markup and the keyboard model read
* the same list rather than each spelling it out (a11y.2: these
* were two `<div @click>`s with no roles, no tabindex and no
* keyboard path at all, which made the Downloads half of the
* Downloads view mouse-only).
*/
private static readonly TABS: ReadonlyArray<readonly [Tab, string]> = [
['requests', 'Requests'],
['downloads', 'Downloads'],
];
/**
* A tablist moves with Left/Right/Home/End and activates as it
* moves — the panel is already rendered, so there is nothing to
* defer. Focus follows, which is what makes the roving tabindex
* mean anything.
*/
private onTabKeydown = (e: KeyboardEvent): void => {
const ids = DownloadsView.TABS.map(([id]) => id);
const at = ids.indexOf(this.tab);
let next: number | null = null;
if (e.key === 'ArrowRight') next = (at + 1) % ids.length;
else if (e.key === 'ArrowLeft') next = (at - 1 + ids.length) % ids.length;
else if (e.key === 'Home') next = 0;
else if (e.key === 'End') next = ids.length - 1;
if (next === null) return;
e.preventDefault();
this.tab = ids[next]!;
void this.updateComplete.then(() => {
this.shadowRoot
?.querySelector<HTMLButtonElement>(`#tab-${this.tab}`)
?.focus();
});
};
// ----------------------------------------------------------------- // -----------------------------------------------------------------
// Requests tab // Requests tab
// ----------------------------------------------------------------- // -----------------------------------------------------------------
@@ -411,11 +411,9 @@ async function dispatch(action: string): Promise<void> {
); );
break; break;
case 'tracklist.delete': // No `tracklist.delete`: it dispatched an event nothing
document.dispatchEvent( // listened for, from a binding Settings advertised as
new CustomEvent('shortcut:tracklist-delete'), // configurable. See backend/shortcuts/config.go.
);
break;
// Panel-specific: autotag review. The view listens for these // Panel-specific: autotag review. The view listens for these
// while it is the view on screen, and for nothing while it is // while it is the view on screen, and for nothing while it is
@@ -0,0 +1,105 @@
/**
* Settings is reachable, and Downloads' tabs are tabs.
*
* `a11y.1` is the last Critical in the accessibility audit: every
* `config-section` header was a bare `<div @click>` with no tabindex,
* no role and no `aria-expanded`, and every section defaults to
* collapsed — so every setting in the app sat behind a control that
* could not be tabbed to. `a11y.2` is the same bug one page over, in
* Downloads' two `<div class="tab">`s.
*
* Reproduced in the running app before either was fixed: seven
* sections, seven `DIV`s, `tabindex` and `role` null on all of them.
*/
import { describe, expect, it } from 'vitest';
import '@components/config-page/config-section';
import '@components/downloads-view/downloads-view';
import { fixture, shadow, shadowAll, update } from '@test/support/render';
describe('<config-section> disclosure', () => {
it('is a button that reports its state', async () => {
const el = await fixture('config-section', { heading: 'Libraries' });
const header = shadow(el, '.header');
expect(header?.tagName).toBe('BUTTON');
expect(header?.getAttribute('aria-expanded')).toBe('false');
});
it('points aria-controls at a body that exists while collapsed', async () => {
const el = await fixture('config-section', { heading: 'Theme' });
const id = shadow(el, '.header')?.getAttribute('aria-controls');
const body = shadow(el, `#${id}`);
// The body renders unconditionally and is toggled with `hidden`:
// aria-controls has to name an element that is in the DOM, and the
// slot's light-DOM children exist either way.
expect(body).toBeTruthy();
expect((body as HTMLElement).hidden).toBe(true);
});
it('expands on activation and says so', async () => {
const el = await fixture('config-section', { heading: 'Theme' });
shadow<HTMLButtonElement>(el, '.header')?.click();
await update(el, {});
expect(shadow(el, '.header')?.getAttribute('aria-expanded')).toBe('true');
expect((shadow(el, '.body') as HTMLElement).hidden).toBe(false);
});
it('starts expanded when the host asks it to', async () => {
const el = await fixture('config-section', { heading: 'Libraries', open: true });
expect(shadow(el, '.header')?.getAttribute('aria-expanded')).toBe('true');
});
});
describe('<downloads-view> tabs', () => {
it('is a tablist of tabs owning a panel', async () => {
const el = await fixture('downloads-view');
const tabs = shadowAll<HTMLButtonElement>(el, '[role="tab"]');
const panel = shadow(el, '[role="tabpanel"]');
expect(shadow(el, '[role="tablist"]')).toBeTruthy();
expect(tabs).toHaveLength(2);
expect(tabs.map((t) => t.getAttribute('aria-selected'))).toEqual([
'true',
'false',
]);
expect(tabs[0]!.getAttribute('aria-controls')).toBe(panel?.id);
});
it('carries a roving tab stop, not two', async () => {
const el = await fixture('downloads-view');
const tabs = shadowAll<HTMLButtonElement>(el, '[role="tab"]');
expect(tabs.map((t) => t.tabIndex)).toEqual([0, -1]);
});
it('moves and activates on ArrowRight, wrapping', async () => {
const el = await fixture('downloads-view');
const tablist = shadow<HTMLElement>(el, '[role="tablist"]')!;
tablist.dispatchEvent(
new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true }),
);
await update(el, {});
expect(
shadowAll(el, '[role="tab"]').map((t) => t.getAttribute('aria-selected')),
).toEqual(['false', 'true']);
expect(shadow(el, '[role="tabpanel"]')?.id).toBe('panel-downloads');
tablist.dispatchEvent(
new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true }),
);
await update(el, {});
expect(shadow(el, '[role="tabpanel"]')?.id).toBe('panel-requests');
});
});