Compare commits

..
Author SHA1 Message Date
logan 266e7032dd fix(explore): stop labelling the album tracklist "TRACKLIST"
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m31s
CI / e2e (pull_request) Successful in 6m9s
A list of numbered titles with durations, under the album's cover, was
the one thing on the page carrying a word above it saying what it is.

What goes is the ink and not the element: the section is a landmark and
the page's heading structure runs through it, so the h3 stays and is
clipped the way sr-only clips -- never display:none, which would take
it out of the accessibility tree along with the layout.

Refs #9
2026-08-18 11:06:14 -04:00
6 changed files with 106 additions and 105 deletions
-66
View File
@@ -1,66 +0,0 @@
import { test, expect } from '../support/fixtures.js';
/**
* The queue button says whether the queue is open.
*
* It used to look identical in both states, so the only way to tell
* what pressing it would do was to look at the other side of the window
* and infer it — and for anyone not looking at all there was nothing to
* infer from: no `aria-expanded`, no `aria-controls`, no pressed state.
*
* The state is reflected *from the panel*, not kept beside the click,
* because the button is not the only thing that opens the queue —
* `now-playing-view` sets the same attribute, since it hides the bar
* this button lives in. A flag maintained by the click handler would be
* right until something else opened the panel and then quietly wrong,
* which is the second test here.
*/
test.describe('the queue toggle', () => {
test('reports open and closed, and names what it controls', async ({
app,
}) => {
const toggle = app.locator('#queue-button');
await expect(toggle).toHaveAttribute('aria-controls', 'queue-panel');
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
await toggle.click();
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
// The state is not only in the accessibility tree: a control that
// announces a state it does not draw is half a fix.
//
// Background rather than colour, because the pointer is still on
// the button after the click and `:hover` paints it the same accent
// the open state does -- so a colour comparison here passes on the
// broken build and proves nothing.
const [open, closed] = await toggle.evaluate((el) => {
const now = getComputedStyle(el).backgroundColor;
el.setAttribute('aria-expanded', 'false');
const shut = getComputedStyle(el).backgroundColor;
el.setAttribute('aria-expanded', 'true');
return [now, shut];
});
expect(open).not.toBe(closed);
await toggle.click();
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
});
test('follows the panel when something else opens it', async ({ app }) => {
const toggle = app.locator('#queue-button');
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
// Exactly what `now-playing-view`'s queue button does.
await app.evaluate(() =>
document.getElementById('queue-panel')?.setAttribute('open', ''),
);
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
});
});
-11
View File
@@ -207,17 +207,6 @@ body div.sidebar {
color: var(--yj-accent, #ffd43b);
}
/* An open queue is a state this button can be in, and it used to
look exactly like the closed one -- so the only way to tell what
pressing it would do was to look at the other side of the window
and infer it. `aria-expanded` is the same fact for anyone not
looking at all, and it points at the panel it controls. */
#queue-button[aria-expanded='true'] {
color: var(--yj-accent, #ffd43b);
background: var(--yj-bg-overlay, #404040);
border-radius: 4px;
}
#queue-button.drag-over {
color: var(--yj-accent, #ffd43b);
outline: 2px dashed var(--yj-accent, #ffd43b);
+1 -2
View File
@@ -37,8 +37,7 @@
<footer class="bottom-bar">
<now-playing></now-playing>
<audio-player></audio-player>
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
id="queue-button">
<button aria-label="Toggle queue" id="queue-button">
<wa-icon name="list"></wa-icon>
</button>
</footer>
-22
View File
@@ -521,28 +521,6 @@ if (queueButton && queuePanel) {
}
});
// The button says whether the panel is open, and it learns that
// from the panel rather than from its own click handler.
//
// It is not the only thing that opens the queue -- `now-playing-view`
// sets the same attribute, because it hides the bar this button
// lives in -- so a state kept beside the click would be right until
// something else opened the panel and then quietly wrong. The panel's
// `open` attribute is the one fact; this reflects it.
const reflectQueueState = () => {
queueButton.setAttribute(
'aria-expanded',
String(queuePanel.hasAttribute('open')),
);
};
new MutationObserver(reflectQueueState).observe(queuePanel, {
attributes: true,
attributeFilter: ['open'],
});
reflectQueueState();
// ---------------------------------------------------------------
// Queue button as drop target (when queue panel is closed)
// ---------------------------------------------------------------
@@ -2,6 +2,7 @@ import { LitElement, html, css, nothing } from 'lit';
import { customElement, property, state, query } from 'lit/decorators.js';
import { classMap } from 'lit/directives/class-map.js';
import { designTokens } from '../../styles/tokens.css';
import { srOnly } from '../../styles/sr-only.css';
import {
LookupReleaseGroup,
BrowseReleases,
@@ -289,6 +290,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
designTokens,
exploreLinkStyles,
contextMenuStyles,
srOnly,
css`
:host {
display: flex;
@@ -3033,11 +3035,21 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
/* ── Tracklist ── */
/**
* The heading is there and is not drawn.
*
* A list of numbered titles with durations under an album's cover
* does not need a word above it saying what it is — it was the
* only thing on this page labelling something already obvious. But
* the section is a landmark and the page's heading structure runs
* through it, so what goes is the *ink*, not the element: a reader
* jumping by heading still finds the tracklist.
*/
private renderTracklist() {
if (this.loadingReleases) {
return html`
<section>
<h3 class="section-header">Tracklist</h3>
<h3 class="sr-only">Tracklist</h3>
<div class="section-loading">Loading tracks\u2026</div>
</section>
`;
@@ -3050,7 +3062,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
if (!current) {
return html`
<section>
<h3 class="section-header">Tracklist</h3>
<h3 class="sr-only">Tracklist</h3>
<div class="section-error">
<wa-icon name="triangle-exclamation"></wa-icon>
No release data available.
@@ -3063,7 +3075,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
if (tracks.length === 0) {
return html`
<section>
<h3 class="section-header">Tracklist</h3>
<h3 class="sr-only">Tracklist</h3>
<div
style="color: var(--yj-text-tertiary, #888); font-size: var(--yj-text-md)"
>
@@ -3079,7 +3091,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
return html`
<section>
<h3 class="section-header">Tracklist</h3>
<h3 class="sr-only">Tracklist</h3>
<div class="tracklist">
${discNumbers.map((discNum) => {
const discTracks = discMap.get(discNum) ?? [];
@@ -0,0 +1,89 @@
/**
* The tracklist's own heading.
*
* A list of numbered titles with durations, under the album's cover, is
* the one thing on this page that did not need a word above it saying
* what it was — "TRACKLIST" labelled the only thing already obvious.
*
* What goes is the *ink*, not the element. The section is a landmark
* and the page's heading structure runs through it, so a reader moving
* by heading still has to be able to find it, and it is hidden the way
* `sr-only` hides things: `clip-path`, never `display: none`, which
* would take it out of the accessibility tree along with the layout.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/explore-album-details/explore-album-details';
import { stub, flush, resetHarness } from '@test/support/harness';
import { fixture, shadowAll } from '@test/support/render';
function track(n: number) {
return {
position: n,
discNumber: 1,
title: `Track ${n}`,
length: 200000,
mbid: `mbid-${n}`,
inLibrary: true,
};
}
async function albumPage(): Promise<LitElement> {
const el = await fixture<LitElement>('explore-album-details', {
albumName: 'Glass Harbour',
releaseGroupMBID: 'rg-1',
});
Object.assign(el, {
versionEntries: [
{
key: 'v1',
label: '2019',
sublabel: '2 tracks',
tracks: [track(1), track(2)],
},
],
selectedVersionKey: 'v1',
loadingReleases: false,
loadingInfo: false,
});
el.requestUpdate();
await flush();
await el.updateComplete;
return el;
}
const tracklistHeading = (el: LitElement) =>
shadowAll(el, 'h3').find((h) => h.textContent?.trim() === 'Tracklist');
describe('the album tracklist heading', () => {
beforeEach(() => {
resetHarness();
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
stub('library.Library.GetFilePathsByAlbums', {});
stub('library.Library.GetAlbumTracks', []);
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
stub('download.Service.ProviderKinds', []);
stub('download.Service.ListProviders', []);
stub('download.Service.ListDownloads', []);
stub('download.Service.ListRequests', []);
});
it('is still in the tree', async () => {
expect(tracklistHeading(await albumPage())).toBeTruthy();
});
it('takes up no room on the page', async () => {
const heading = tracklistHeading(await albumPage())!;
const box = heading.getBoundingClientRect();
expect(box.width).toBeLessThanOrEqual(1);
expect(box.height).toBeLessThanOrEqual(1);
// Hidden by clipping, not by removal: display:none and
// visibility:hidden both take it out of the accessibility tree.
expect(getComputedStyle(heading).display).not.toBe('none');
expect(getComputedStyle(heading).visibility).not.toBe('hidden');
});
});