Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/ACCESSIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions docs/AI_USAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,13 @@
- 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.

## A shorter README (`README.md`, `docs/`)

- **Tool:** Claude Code
Expand Down
10 changes: 9 additions & 1 deletion src/components/elements/ElementDiagram.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}: {
Expand All @@ -14,7 +19,10 @@ export default function ElementDiagram({
<CardContent className="p-0">
<img
src={imageUrlForApp(element.imageUrl)}
alt={`Illustration for ${displayCode(element, element.variants.length > 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"
/>
</CardContent>
Expand Down
6 changes: 4 additions & 2 deletions src/components/elements/ResultsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,12 @@ function ResultCards({ rows }: { rows: ElementListRow[] }) {
</span>
)}
{/* 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. */}
<img
src={imageUrlForApp(row.imageUrl)}
alt={`Illustration for ${row.code}${row.name ? `: ${row.name}` : ''}`}
alt=""
loading="lazy"
decoding="async"
onError={(event) => {
Expand Down
8 changes: 3 additions & 5 deletions src/tests/components/elements/ResultsTable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -227,18 +227,16 @@ 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]))

const image = /<img[^>]*>/.exec(plainCard)?.[0] ?? ''
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(/<img[^>]*>/.exec(namedCard)?.[0]).toContain('alt=""')
// Not beside the text: there is no column for it.
expect(plainCard).not.toContain('@container')
})
Expand Down
4 changes: 4 additions & 0 deletions src/tests/pages/ElementDetailPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
3 changes: 2 additions & 1 deletion src/tests/pages/ElementsPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLImageElement>('img')
// (A drawing in a card has no text for a screen reader, so it has no role.)
const images = [...document.querySelectorAll<HTMLImageElement>('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)
Expand Down