From 104cec7eb1fdd0d114cdd5b0d8a61dde07e8a8c7 Mon Sep 17 00:00:00 2001 From: Ronja Date: Tue, 10 Nov 2026 20:53:28 +0100 Subject: [PATCH] fix(a11y): use a more helpful text for the element drawings The element page uses the description of the element as the text of the drawing, and the cards of the results list use an empty text, because the description is in the card just above the drawing. The rule is written in docs/ACCESSIBILITY.md. Closes #173 --- docs/ACCESSIBILITY.md | 6 ++++++ docs/AI_USAGE.md | 7 +++++++ src/components/elements/ElementDiagram.tsx | 10 +++++++++- src/components/elements/ResultsTable.tsx | 6 ++++-- src/tests/components/elements/ResultsTable.test.tsx | 8 +++----- src/tests/pages/ElementDetailPage.test.tsx | 4 ++++ src/tests/pages/ElementsPage.test.tsx | 3 ++- 7 files changed, 35 insertions(+), 9 deletions(-) diff --git a/docs/ACCESSIBILITY.md b/docs/ACCESSIBILITY.md index 447bc97..9eda2f3 100644 --- a/docs/ACCESSIBILITY.md +++ b/docs/ACCESSIBILITY.md @@ -31,6 +31,12 @@ Dynamic updates such as loading, result counts, and action feedback use status s **Where:** [ElementsPage.tsx](../src/pages/ElementsPage.tsx), [ElementActions.tsx](../src/components/elements/ElementActions.tsx), [ElementDetailPage.tsx](../src/pages/ElementDetailPage.tsx), and [YourGymnastsPage.tsx](../src/pages/YourGymnastsPage.tsx). +### Text for the drawings + +The drawing of an element has one rule: its text for a screen reader (`alt`) says what the drawing shows, and is never repeated where the same words are already next to it. On the element page it is the description of the element ("Clear hip circle to hstd"), because the drawing is the main content there. In the cards of the results list it is empty (`alt=""`), because the description is in the card just above the drawing, and saying it again for every card would only be noise. + +**Where:** [ElementDiagram.tsx](../src/components/elements/ElementDiagram.tsx) and [ResultsTable.tsx](../src/components/elements/ResultsTable.tsx). + ### Themes, contrast, and reduced motion The interface supports light and dark themes with separate color tokens. Interactive controls and links include visible focus styling. Animations and transitions use reduced-motion variants so they can be suppressed when the user requests reduced motion. diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 3e7719c..9d9a0e6 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -589,3 +589,10 @@ - Changed outside the Gymnast page: the element page (colors, the badge, "Try in a routine", "Add to gymnast"), "Your gymnasts" (the wrapping of the title and the list), `variables.css` (`--card-hover`), and two shared parts that other pages use too: `Button` (the `icon` prop, which changes nothing when it is not given) and `DifficultyBadge` (its colors now follow the theme, also in the results of the Elements page). - Not done on purpose: moving the error messages of the other pages (the elements results, the element page, the login, "Your gymnasts") to the three layers, because those files belong to other pull requests and the change would conflict with them. The menus of the Elements page still have their own copies of the menu classes that `menuStyles.ts` now holds. - **Limits:** The end-to-end tests need the database and the backend, and are run by hand: the course git server has no runners. A screen reader was not used: that has to be done by a person. Lighthouse was started on a production build in the last round, but the run did not finish (the computer was low on memory), so there are no Lighthouse scores from that round. "Your gymnasts" was fixed after the last check in a browser and has not been measured again, and the results table of the Elements page, which uses the changed badge, has not been looked at in dark mode. The test gymnast was made with the test login in the local database and deleted again. + +## The text for the element drawings (`src/components/elements/`) + +- **Tool:** Claude Code +- **What the team did:** Took the peer feedback that "Illustration for 1.00" is of little use to a screen reader user, and wrote issue #173. +- **What Claude did:** Found that there are only two places with a drawing (the element page and the cards of the results list) and that both have the description of the element right next to the drawing. Chose one rule: the description as the text on the element page, where the drawing is the main content, and an empty text in the cards, where it would be repeated for every card. Wrote the rule in `docs/ACCESSIBILITY.md` and changed the three tests that read the old text. +- **Limits:** An empty text hides the drawing from a screen reader. That is correct only because the description is next to it; it was not tested with a screen reader. The peer reviewer suggested using the description everywhere, and the team can choose that instead. diff --git a/src/components/elements/ElementDiagram.tsx b/src/components/elements/ElementDiagram.tsx index 5bd9bc3..8336037 100644 --- a/src/components/elements/ElementDiagram.tsx +++ b/src/components/elements/ElementDiagram.tsx @@ -2,6 +2,11 @@ import { Card, CardContent } from '@/components/ui/card' import { displayCode, type ElementDetail } from '@/lib/catalog' import { imageUrlForApp } from '@/lib/imageUrl' +/** + * The drawing of the element. Its text for a screen reader is the description of the element + * ("Clear hip circle to hstd"), which says what the drawing shows. It is not "Illustration for + * 1.00", which only repeats the code. + */ export default function ElementDiagram({ element, }: { @@ -14,7 +19,10 @@ export default function ElementDiagram({ {`Illustration 0)}${element.name ? `: ${element.name}` : ''}`} + alt={ + element.description || + `Drawing of ${displayCode(element, element.variants.length > 0)}` + } className="block h-auto max-h-[75svh] w-full object-contain" /> diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 8ff38d8..b5730dd 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -81,10 +81,12 @@ function ResultCards({ rows }: { rows: ElementListRow[] }) { )} {/* The drawing is below the text. If it is missing, nothing is shown, - and not a broken image. */} + and not a broken image. It has no text for a screen reader (alt=""), because the + description of the element is in the card just above it, and saying it again for + every card of the list would only be noise. */} {`Illustration { diff --git a/src/tests/components/elements/ResultsTable.test.tsx b/src/tests/components/elements/ResultsTable.test.tsx index 2e71d7a..905f856 100644 --- a/src/tests/components/elements/ResultsTable.test.tsx +++ b/src/tests/components/elements/ResultsTable.test.tsx @@ -227,7 +227,7 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { expect(card).not.toContain(plain.elementGroupName) }) - it('show the drawing below the text, always, with a text for a screen reader, loaded when it is near', () => { + it('show the drawing below the text, always, without a text for a screen reader (the description is above it), loaded when it is near', () => { const plainCard = cardsOf(render([plain])) const namedCard = cardsOf(render([named])) @@ -235,10 +235,8 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { expect(image).toContain(`src="${plain.imageUrl}"`) expect(image).toContain('loading="lazy"') expect(image.split(/[ "]/)).not.toContain('hidden') - expect(plainCard).toContain(`alt="Illustration for ${plain.code}"`) - expect(namedCard).toContain( - `alt="Illustration for ${named.code}: ${named.name}"`, - ) + expect(image).toContain('alt=""') + expect(/]*>/.exec(namedCard)?.[0]).toContain('alt=""') // Not beside the text: there is no column for it. expect(plainCard).not.toContain('@container') }) diff --git a/src/tests/pages/ElementDetailPage.test.tsx b/src/tests/pages/ElementDetailPage.test.tsx index 7432d55..578f947 100644 --- a/src/tests/pages/ElementDetailPage.test.tsx +++ b/src/tests/pages/ElementDetailPage.test.tsx @@ -149,6 +149,10 @@ describe('ElementDetailPage', () => { expect(screen.getByRole('img').getAttribute('src')).toBe( 'http://images.test/UB/UB-2.305-a.webp', ) + // The text for a screen reader is the description of the element, not "Illustration for ...". + expect(screen.getByRole('img').getAttribute('alt')).toBe( + 'Description of UB-2.305-a', + ) expect( screen.getByRole('link', { name: /2\.305-b/ }).getAttribute('href'), ).toBe('/elements/UB-2.305-b') diff --git a/src/tests/pages/ElementsPage.test.tsx b/src/tests/pages/ElementsPage.test.tsx index c75b6df..2f19cf5 100644 --- a/src/tests/pages/ElementsPage.test.tsx +++ b/src/tests/pages/ElementsPage.test.tsx @@ -1074,7 +1074,8 @@ describe('ElementsPage: the results from the API', () => { ).toBeGreaterThan(0) expect(screen.queryByText(vault.elementGroupName)).toBeNull() // The drawing is what the API says (imageUrl). - const images = screen.getAllByRole('img') + // (A drawing in a card has no text for a screen reader, so it has no role.) + const images = [...document.querySelectorAll('img')] expect(images.map((image) => image.src)).toContain(vault.imageUrl) // "More variants" only where the API lists variants (a card and a row). expect(withVariants.hasVariants).toBe(true)