fix(a11y): let a clipped value be read, and name a row's own buttons
a11y.24: `text-overflow: ellipsis` in 40+ places, and the four highest-density lists were the ones with no `title` — the queue panel (whose width is user-resizable down to MIN_WIDTH), track-info, every track-list cell, and the playlist sidebar. In track-list the attribute is on the *cell*, not on what is inside it: the value may be a link, a highlighted search match or plain text, and a tooltip is inherited by descendants either way. One binding rather than three, and the same value the accessor already computed. a11y.32: every queue row's remove button was named "Remove from queue", so a list whose entire purpose is which track is where had four identically named controls.
This commit is contained in:
@@ -1786,6 +1786,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
|||||||
: html`
|
: html`
|
||||||
<span
|
<span
|
||||||
class="playlist-name"
|
class="playlist-name"
|
||||||
|
title=${entry.summary.Name}
|
||||||
>
|
>
|
||||||
${entry.summary
|
${entry.summary
|
||||||
.Name}
|
.Name}
|
||||||
|
|||||||
@@ -1595,6 +1595,13 @@ export class QueuePanel
|
|||||||
|
|
||||||
const artUrl = track.coverArtPath || '';
|
const artUrl = track.coverArtPath || '';
|
||||||
|
|
||||||
|
// The panel's width is user-resizable down to MIN_WIDTH, so
|
||||||
|
// both of these are routinely clipped (a11y.24) — and the
|
||||||
|
// remove button is one of every row, named identically
|
||||||
|
// (a11y.32).
|
||||||
|
const title = this.getDisplayTitle(track);
|
||||||
|
const artist = track.artist || 'Unknown Artist';
|
||||||
|
|
||||||
// No inline closures — all events delegated via data-index
|
// No inline closures — all events delegated via data-index
|
||||||
// on the virtualizer element (see firstUpdated).
|
// on the virtualizer element (see firstUpdated).
|
||||||
return html`
|
return html`
|
||||||
@@ -1620,16 +1627,17 @@ export class QueuePanel
|
|||||||
</span>
|
</span>
|
||||||
${artUrl ? html`<div class="track-art"><img src="${artUrl}" alt="" loading="lazy" /></div>` : nothing}
|
${artUrl ? html`<div class="track-art"><img src="${artUrl}" alt="" loading="lazy" /></div>` : nothing}
|
||||||
<div class="track-details">
|
<div class="track-details">
|
||||||
<span class="track-title">
|
<span class="track-title" title=${title}>
|
||||||
${trackLink(this.getDisplayTitle(track), track.album, track.releaseGroupMbid, track.recordingMbid, undefined, track.artist)}
|
${trackLink(title, track.album, track.releaseGroupMbid, track.recordingMbid, undefined, track.artist)}
|
||||||
</span>
|
</span>
|
||||||
<span class="track-artist">
|
<span class="track-artist" title=${artist}>
|
||||||
${artistLink(track.artist, track.artistMbid) || 'Unknown Artist'}
|
${artistLink(track.artist, track.artistMbid) || 'Unknown Artist'}
|
||||||
</span>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
<button
|
<button
|
||||||
class="remove-button"
|
class="remove-button"
|
||||||
title="Remove from queue"
|
title="Remove from queue"
|
||||||
|
aria-label="Remove ${title} from queue"
|
||||||
>
|
>
|
||||||
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 384 512" width="14" height="14">
|
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 384 512" width="14" height="14">
|
||||||
${svg`<path fill="currentColor" d="M342.6 150.6c12.5-12.5 12.5-32.8 0-45.3s-32.8-12.5-45.3 0L192 210.7 86.6 105.4c-12.5-12.5-32.8-12.5-45.3 0s-12.5 32.8 0 45.3L146.7 256 41.4 361.4c-12.5 12.5-12.5 32.8 0 45.3s32.8 12.5 45.3 0L192 301.3 297.4 406.6c12.5 12.5 32.8 12.5 45.3 0s12.5-32.8 0-45.3L237.3 256 342.6 150.6z"/>`}
|
${svg`<path fill="currentColor" d="M342.6 150.6c12.5-12.5 12.5-32.8 0-45.3s-32.8-12.5-45.3 0L192 210.7 86.6 105.4c-12.5-12.5-32.8-12.5-45.3 0s-12.5 32.8 0 45.3L146.7 256 41.4 361.4c-12.5 12.5-12.5 32.8 0 45.3s32.8 12.5 45.3 0L192 301.3 297.4 406.6c12.5 12.5 32.8 12.5 45.3 0s12.5-32.8 0-45.3L237.3 256 342.6 150.6z"/>`}
|
||||||
|
|||||||
@@ -118,10 +118,12 @@ export class TrackInfo extends LitElement {
|
|||||||
${showCover ? this.renderCoverArt() : nothing}
|
${showCover ? this.renderCoverArt() : nothing}
|
||||||
<div class="text">
|
<div class="text">
|
||||||
${displayTitle
|
${displayTitle
|
||||||
? html`<span class="title">${displayTitle}</span>`
|
? html`<span class="title" title=${displayTitle}
|
||||||
|
>${displayTitle}</span
|
||||||
|
>`
|
||||||
: nothing}
|
: nothing}
|
||||||
${secondaryParts
|
${secondaryParts
|
||||||
? html`<span class="secondary"
|
? html`<span class="secondary" title=${secondaryParts}
|
||||||
>${secondaryParts}</span
|
>${secondaryParts}</span
|
||||||
>`
|
>`
|
||||||
: nothing}
|
: nothing}
|
||||||
|
|||||||
@@ -1839,8 +1839,12 @@ export class TrackList
|
|||||||
display = albumLink(track.Album, track.ReleaseGroupMBID, display as any, track.ArtistName);
|
display = albumLink(track.Album, track.ReleaseGroupMBID, display as any, track.ArtistName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// `title` on the cell rather than on whatever is inside
|
||||||
|
// it: the value may be a link, a highlighted match or
|
||||||
|
// plain text, and a tooltip is inherited by descendants
|
||||||
|
// either way (a11y.24).
|
||||||
return html`
|
return html`
|
||||||
<div role="gridcell" class=${classMap({
|
<div role="gridcell" title=${val} class=${classMap({
|
||||||
cell: true,
|
cell: true,
|
||||||
'cell-center': centered,
|
'cell-center': centered,
|
||||||
'cell-right': !centered && col.align === 'right',
|
'cell-right': !centered && col.align === 'right',
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import type { LitElement } from 'lit';
|
|||||||
import '@components/track-list/track-list';
|
import '@components/track-list/track-list';
|
||||||
import '@components/artists-view/artists-view';
|
import '@components/artists-view/artists-view';
|
||||||
import '@components/genres-view/genres-view';
|
import '@components/genres-view/genres-view';
|
||||||
|
import '@components/track-info/track-info';
|
||||||
import { emit, stub, flush, resetHarness } from '@test/support/harness';
|
import { emit, stub, flush, resetHarness } from '@test/support/harness';
|
||||||
import { Events } from '../../src/events';
|
import { Events } from '../../src/events';
|
||||||
import { fixture, shadow, shadowAll } from '@test/support/render';
|
import { fixture, shadow, shadowAll } from '@test/support/render';
|
||||||
@@ -191,3 +192,46 @@ describe('a selectable grid is a listbox, not a row of buttons', () => {
|
|||||||
expect(list!.getAttribute('aria-multiselectable')).toBe('true');
|
expect(list!.getAttribute('aria-multiselectable')).toBe('true');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('a clipped value is readable somewhere', () => {
|
||||||
|
beforeEach(async () => {
|
||||||
|
resetHarness();
|
||||||
|
searchStore.setTerm('');
|
||||||
|
stub('library.Library.GetAllTracks', TRACKS);
|
||||||
|
stub('library.Library.GetAllAlbums', []);
|
||||||
|
emit(Events.LibraryScanComplete);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('gives every track-list cell the value it may be clipping', async () => {
|
||||||
|
const el = await fixture<LitElement>('track-list');
|
||||||
|
|
||||||
|
sized(el);
|
||||||
|
await settle(el);
|
||||||
|
|
||||||
|
const titles = shadowAll(el, '.track-row [role="gridcell"].cell').map((c) =>
|
||||||
|
c.getAttribute('title'),
|
||||||
|
);
|
||||||
|
|
||||||
|
// `a11y.24`: `text-overflow: ellipsis` in 40+ places, and the
|
||||||
|
// highest-density lists were the ones without a `title`. The
|
||||||
|
// attribute is on the cell rather than on what is inside it,
|
||||||
|
// because the value may be a link or a highlighted match and a
|
||||||
|
// tooltip is inherited by descendants either way.
|
||||||
|
expect(titles.length).toBeGreaterThan(0);
|
||||||
|
expect(titles).toContain('Departure');
|
||||||
|
expect(titles.every((t) => t !== null && t !== '')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('gives track-info its own title and secondary line', async () => {
|
||||||
|
const el = await fixture<LitElement>('track-info', {
|
||||||
|
trackTitle: 'An Exhaustively Overlong Track Name',
|
||||||
|
artist: 'Aurora Fields',
|
||||||
|
});
|
||||||
|
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(shadow(el, '.title')?.getAttribute('title'))
|
||||||
|
.toBe('An Exhaustively Overlong Track Name');
|
||||||
|
expect(shadow(el, '.secondary')?.getAttribute('title')).toBeTruthy();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -19,7 +19,7 @@ import { describe, expect, it, beforeEach } from 'vitest';
|
|||||||
import '@components/queue-panel/queue-panel';
|
import '@components/queue-panel/queue-panel';
|
||||||
import type { QueuePanel } from '@components/queue-panel/queue-panel';
|
import type { QueuePanel } from '@components/queue-panel/queue-panel';
|
||||||
import { Events } from '../../src/events';
|
import { Events } from '../../src/events';
|
||||||
import { emit, calls, flush, lastArgs } from '@test/support/harness';
|
import { emit, calls, flush, lastArgs, resetHarness } from '@test/support/harness';
|
||||||
import { fixture, shadow, shadowAll } from '@test/support/render';
|
import { fixture, shadow, shadowAll } from '@test/support/render';
|
||||||
import type { QueueTrack } from '@store/queue-store';
|
import type { QueueTrack } from '@store/queue-store';
|
||||||
|
|
||||||
@@ -170,3 +170,36 @@ describe('<queue-panel> keyboard reorder', () => {
|
|||||||
expect(calls().some((c) => c.path.includes('Move'))).toBe(false);
|
expect(calls().some((c) => c.path.includes('Move'))).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('a queue row says which track its controls act on', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('names each remove button after its own track', async () => {
|
||||||
|
const el = await panelWithQueue();
|
||||||
|
|
||||||
|
const labels = shadowAll(el, '.remove-button').map((b) =>
|
||||||
|
b.getAttribute('aria-label'),
|
||||||
|
);
|
||||||
|
|
||||||
|
// `a11y.32`: `title="Remove from queue"` on every row is a name
|
||||||
|
// that never identifies which track — four identical buttons in a
|
||||||
|
// list whose whole purpose is the order.
|
||||||
|
expect(labels.slice(0, 4)).toEqual([
|
||||||
|
'Remove First from queue',
|
||||||
|
'Remove Second from queue',
|
||||||
|
'Remove Third from queue',
|
||||||
|
'Remove Fourth from queue',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('gives the title and artist a tooltip, since the panel is resizable', async () => {
|
||||||
|
const el = await panelWithQueue();
|
||||||
|
|
||||||
|
// `a11y.24` calls this one acute: MIN_WIDTH is narrow enough that
|
||||||
|
// both lines clip routinely, and nothing else can show the value.
|
||||||
|
expect(shadow(el, '.track-title')?.getAttribute('title')).toBe('First');
|
||||||
|
expect(shadow(el, '.track-artist')?.getAttribute('title')).toBe('Artist');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user