From dcb49e96871c28eeea762630d6855e506f9cd0a9 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 21:05:27 -0400 Subject: [PATCH] fix(web): a11y on DiscoverResultCard + drop test-only inline styles aria-labels now include the card title on all three button variants so SR users navigating a grid can tell which card a button belongs to: - "Request " / "<title> is already in library" / "<title> already requested" The Kept pill gets role="status" so SR users hear the badge when it appears (without aria-live's announce-on-mount noise). The reserved-slot CSS (.badge-row { min-height: 22px } and .actions { margin-top: auto }) was already in the scoped <style> block; we drop the duplicate inline style="" attributes that existed purely to satisfy jsdom's getComputedStyle. Tried stylesheet introspection (document.styleSheets) as a replacement assertion, but vitest's @testing-library/svelte renderer doesn't inject the scoped <style> tag into jsdom (styleSheets.length === 0), so the two CSS assertions are dropped with an explanatory comment. The layout discipline is enforced visually at the consumer page rather than as a "CSS exists in CSS" unit test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --- .../lib/components/DiscoverResultCard.svelte | 13 +++--- .../lib/components/DiscoverResultCard.test.ts | 46 ++++++++----------- 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/web/src/lib/components/DiscoverResultCard.svelte b/web/src/lib/components/DiscoverResultCard.svelte index e6ae477d..345e316f 100644 --- a/web/src/lib/components/DiscoverResultCard.svelte +++ b/web/src/lib/components/DiscoverResultCard.svelte @@ -55,21 +55,18 @@ {#if subtitle} <div class="subtitle text-sm text-text-secondary">{subtitle}</div> {/if} - <div - class="badge-row" - data-testid="badge-row" - style="min-height: 22px;" - > + <div class="badge-row" data-testid="badge-row"> {#if state === 'kept'} - <span class="kept-pill">Kept</span> + <span class="kept-pill" role="status">Kept</span> {/if} </div> </div> - <div class="actions pt-3" data-testid="actions" style="margin-top: auto;"> + <div class="actions pt-3" data-testid="actions"> {#if state === 'requestable'} <button type="button" + aria-label={`Request ${title}`} class="flex items-center gap-1 rounded-md bg-action-primary px-3 py-1.5 text-sm text-text-primary" onclick={handleRequest} > @@ -79,6 +76,7 @@ <button type="button" disabled + aria-label={`${title} is already in library`} onclick={handleRequest} class="rounded-md border border-border px-3 py-1.5 text-sm text-text-muted" > @@ -88,6 +86,7 @@ <button type="button" disabled + aria-label={`${title} already requested`} onclick={handleRequest} class="rounded-md border border-border px-3 py-1.5 text-sm text-text-muted" > diff --git a/web/src/lib/components/DiscoverResultCard.test.ts b/web/src/lib/components/DiscoverResultCard.test.ts index 6672349c..acffc80f 100644 --- a/web/src/lib/components/DiscoverResultCard.test.ts +++ b/web/src/lib/components/DiscoverResultCard.test.ts @@ -15,13 +15,17 @@ describe('DiscoverResultCard', () => { onRequest, }, }); - const btn = screen.getByRole('button', { name: /request/i }); + // aria-label includes the title, so the accessible name is + // "Request Boards of Canada". + const btn = screen.getByRole('button', { + name: /request boards of canada/i, + }); expect(btn).not.toBeDisabled(); await fireEvent.click(btn); expect(onRequest).toHaveBeenCalledOnce(); }); - test('kept state renders disabled "In library" button + Kept pill', () => { + test('kept state renders disabled "In library" button + Kept pill with role=status', () => { render(DiscoverResultCard, { props: { kind: 'album', @@ -30,7 +34,10 @@ describe('DiscoverResultCard', () => { }, }); expect(screen.getByRole('button', { name: /in library/i })).toBeDisabled(); - expect(screen.getByText(/kept/i)).toBeInTheDocument(); + // Kept pill is exposed as a status region so SR users hear it when it appears. + const status = screen.getByRole('status'); + expect(status).toBeInTheDocument(); + expect(status.textContent).toMatch(/kept/i); }); test('requested state renders disabled "Requested" button', () => { @@ -61,30 +68,15 @@ describe('DiscoverResultCard', () => { expect(onRequest).not.toHaveBeenCalled(); }); - test('badge row reserves min-height: 22px even when empty', () => { - render(DiscoverResultCard, { - props: { - kind: 'artist', - title: 'Aphex Twin', - state: 'requestable', - }, - }); - const row = screen.getByTestId('badge-row'); - const cs = getComputedStyle(row); - expect(cs.minHeight).toBe('22px'); - }); - - test('actions block is anchored to bottom (margin-top: auto)', () => { - render(DiscoverResultCard, { - props: { - kind: 'artist', - title: 'Aphex Twin', - state: 'requestable', - }, - }); - const actions = screen.getByTestId('actions'); - expect(getComputedStyle(actions).marginTop).toBe('auto'); - }); + // NOTE: previous versions of this file asserted .badge-row { min-height: 22px } + // and .actions { margin-top: auto } via getComputedStyle, which only worked + // because the component shipped inline style="" attributes — a layering + // violation. Stylesheet introspection (document.styleSheets) was tried as a + // replacement, but vitest's @testing-library/svelte renderer does not inject + // the component's scoped <style> block into jsdom (styleSheets.length === 0). + // Asserting "CSS exists in CSS" has weak value anyway, so the layout + // discipline (reserved badge slot + bottom-anchored actions) is enforced + // visually at the consumer page rather than as a unit test here. test('renders <img> when imageUrl is set', () => { const { container } = render(DiscoverResultCard, {