diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 44e324f..0fe9254 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -596,3 +596,19 @@ - **What the team did:** Took the peer feedback that the README (about 59 kB) is too long to find your way in, wrote issue #182, and asked for the reasoning to be kept. - **What Claude did:** Moved four long sections unchanged to their own documents (`ELEMENTS_PAGE.md`, `DATABASE_SETUP.md`, `CATALOG_SEARCH.md`, `E2E_TESTS.md`) with a short summary and a link in the README (now about half the size). Added a "Where is what" table that points from each feature to the file where it starts, and a list of contents at the top of `docs/API.md`. A script checked that every relative link and anchor in the changed files works. - **Limits:** The text of the moved sections was not shortened or checked again, so what was outdated in them is still outdated (that is issue #148). A first run of the move emptied `README.md` because a formatting command was cut off in a pipe; it was restored from git and run again. + +## The hover and pressed state of the buttons, the menus and the rows (`src/pages/ElementsPage.tsx`, `src/components/elements/`, `src/components/ui/menuStyles.ts`, `src/styles/variables.css`) + +- **Tool:** Claude Code +- **What the team did:** Took the peer feedback that the hover effect of the buttons is hard to see, that the thin text shadow of the result rows is blended differently by each browser, and that "Clear filters" could have a danger color (issue #171). Ronja then tried the result by hand in the browser, in both themes and on a phone, and asked for a fix for each thing that was not right. The prompts that had to be repeated or corrected are listed below. +- **What Claude did:** Measured the hover states in a real browser first (the button color changed by only 1.09:1 in light mode). Gave the buttons a 2 px edge in the text color on hover (12.8:1 in light and 7.4:1 in dark against the page), made "Clear filters" turn to `--danger`, replaced the text shadow of the rows by a 4 px edge on the first cell, and wrote an end-to-end test (`e2e/hover.e2e.ts`) that measures the states in both themes. +- **What was not right the first time, and was changed after Ronja's prompts:** + - An open menu button had white text on the tan button of dark mode (2.4:1). It now has the colors of a highlighted choice. The same colors are used while a button is pressed, so the state can be seen. + - The pressed "Clear filters" turned navy, because a color shared by all the buttons overrode the danger color. It is red when pressed, as it is on hover. + - A highlighted choice in a menu was light text on light tan in dark mode (1.5:1) and almost invisible in light mode (1.09:1 against the menu). It is now navy with light text in light mode (15.9:1) and light tan with dark text in dark mode (9.0:1, 6.6:1 against the menu). + - A first fix gave the highlighted choice a 2 px edge for every pointer hover. Ronja asked to remove it. The edge is only for the keyboard, and only where the fill is not used (a choice that cannot be chosen but can be reached, to read why), because that is where WCAG 1.4.11 needs it. + - The menus could not be told from the page. They have an edge in the text color, first 2 px, and 1 px as the search field has, after Ronja's prompt. + - A filter that cannot be used, and the choices that cannot be chosen, reacted to the pointer (an edge, a color, a smaller size). They do not react now. + - The sort button was alone in a row on a phone. Below 600 px it is in the row of the last filter when there is room for it. +- **Lesson:** The first version was measured for the states Claude had been asked about (hover on a button and on a row). The states it had not thought of (open, pressed, disabled, the menu itself, the keyboard) had to be found by hand, and each of them had a contrast problem or a state that did not make sense. For the next change of a component, every state is listed first, and each one is measured in both themes. +- **Limits:** Measured on one machine in Chromium only, so how other browsers draw the edges is not checked. The rows still have no color change in light mode (the design has none); only the edge and the height show the hover there. diff --git a/e2e/hover.e2e.ts b/e2e/hover.e2e.ts new file mode 100644 index 0000000..6b43ec2 --- /dev/null +++ b/e2e/hover.e2e.ts @@ -0,0 +1,135 @@ +// The hover state of the buttons and of the result rows, measured in a real browser in both +// themes: the hovered state must be easy to tell from the resting one (an edge of at least 3:1 +// against the page, WCAG 1.4.11), and "Clear filters" turns to the danger color. +import { expect, test, type Page } from '@playwright/test' +import { chooseTheme, type Theme } from './gymnast-helpers.ts' +import { choose, openMenu, resultsLoaded } from './helpers.ts' + +/** The contrast ratio of two CSS colors, computed in the page (so every color syntax works). */ +async function contrast(page: Page, a: string, b: string) { + return page.evaluate( + ([x, y]) => { + const rgb = (color: string) => { + const context = document.createElement('canvas').getContext('2d')! + context.fillStyle = color + context.fillRect(0, 0, 1, 1) + return [...context.getImageData(0, 0, 1, 1).data].slice(0, 3) + } + const luminance = (color: string) => { + const [r, g, b] = rgb(color).map((v) => { + const c = v! / 255 + return c <= 0.03928 ? c / 12.92 : ((c + 0.055) / 1.055) ** 2.4 + }) + return 0.2126 * r! + 0.7152 * g! + 0.0722 * b! + } + const [hi, lo] = [luminance(x!), luminance(y!)].sort((p, q) => q - p) + return (hi! + 0.05) / (lo! + 0.05) + }, + [a, b], + ) +} + +for (const theme of ['light', 'dark'] as Theme[]) { + test(`the hovered button has an edge of 3:1 and Clear filters turns red (${theme})`, async ({ + page, + }) => { + await chooseTheme(page, theme) + await page.goto('elements') + await resultsLoaded(page) + const pageBackground = await page + .locator('main') + .evaluate((e) => getComputedStyle(e).backgroundColor) + + // A filter that cannot be used (a group needs an apparatus) does not react to the pointer. + const unavailable = page.getByRole('button', { name: /Element group/ }) + await expect(unavailable).toHaveAttribute('aria-disabled', 'true') + const before = await unavailable.evaluate( + (e) => getComputedStyle(e).boxShadow, + ) + await unavailable.hover() + await page.waitForTimeout(400) + expect( + await unavailable.evaluate((e) => getComputedStyle(e).boxShadow), + ).toBe(before) + + // A choice that cannot be chosen (sorting by difficulty needs an apparatus) is not + // highlighted by the pointer. + await openMenu(page, /Code:/) + const blocked = page.getByRole('menuitemradio', { + name: 'Difficulty: low to high', + }) + await expect(blocked).toHaveAttribute('data-disabled', '') + await blocked.hover() + await expect(blocked).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)') + await page.keyboard.press('Escape') + + const button = page.getByRole('button', { name: /Apparatus/ }).first() + await page.mouse.move(0, 0) + const resting = await button.evaluate((e) => getComputedStyle(e).boxShadow) + await button.hover() + // The edge is a 2 px ring in the text color. It is not there at rest, and the + // transition has to be over before it is measured. + const RING = /(rgba?[(][^)]*[)]|color[(][^)]*[)]) 0px 0px 0px 2px/ + await expect + .poll(async () => + RING.exec(await button.evaluate((e) => getComputedStyle(e).boxShadow)), + ) + .not.toBeNull() + expect(resting).not.toMatch(RING) + const ring = RING.exec( + await button.evaluate((e) => getComputedStyle(e).boxShadow), + ) + expect(await contrast(page, ring![1]!, pageBackground)).toBeGreaterThan(3) + + // While its menu is open the button keeps its own text color (it was white, 2.4:1 on + // the tan button of dark mode). + const difficulty = page.getByRole('button', { name: /Difficulty/ }) + await openMenu(page, /Difficulty/) + const open = await difficulty.evaluate((e) => { + const style = getComputedStyle(e) + return [style.color, style.backgroundColor] as const + }) + expect(await contrast(page, open[0], open[1])).toBeGreaterThan(4.5) + // The menu has an edge that can be told from the page (1.4.11). + const edge = await page.getByRole('menu').evaluate((e) => { + const style = getComputedStyle(e) + return [style.borderTopWidth, style.borderTopColor] as const + }) + expect(parseFloat(edge[0])).toBeGreaterThanOrEqual(1) + expect(await contrast(page, edge[1], pageBackground)).toBeGreaterThan(3) + await page.keyboard.press('Escape') + + await openMenu(page, /Apparatus/) + // The choice under the pointer must be readable (it was light on light tan in dark mode). + const choice = page.getByRole('menuitemradio', { + name: 'Vault', + exact: true, + }) + await choice.hover() + await expect(choice).toHaveAttribute('data-highlighted', '') + const highlighted = await choice.evaluate((e) => { + const style = getComputedStyle(e) + return [style.color, style.backgroundColor] as const + }) + expect( + await contrast(page, highlighted[0], highlighted[1]), + ).toBeGreaterThan(4.5) + await choose(page, 'menuitemradio', 'Vault') + await page.keyboard.press('Escape') + const clear = page.getByRole('button', { name: 'Clear filters' }) + await expect(clear).toBeVisible() + // The danger color of the theme, as the browser writes it. + const danger = await page.evaluate(() => { + const probe = document.createElement('span') + probe.style.color = 'var(--danger)' + document.body.append(probe) + const color = getComputedStyle(probe).color + probe.remove() + return color + }) + await clear.hover() + await expect + .poll(() => clear.evaluate((e) => getComputedStyle(e).backgroundColor)) + .toBe(danger) + }) +} diff --git a/src/components/elements/FilterMenu.tsx b/src/components/elements/FilterMenu.tsx index c170ce0..67017ee 100644 --- a/src/components/elements/FilterMenu.tsx +++ b/src/components/elements/FilterMenu.tsx @@ -5,10 +5,10 @@ import { Check, ChevronDown } from 'lucide-react' import { Button } from '@/components/ui/button' const ITEM = - 'flex min-h-10 cursor-default items-center gap-2 rounded-md px-3 text-sm font-medium text-(--text-color) outline-none select-none data-[highlighted]:bg-(--button-hover-bg)' + 'flex min-h-10 cursor-default items-center gap-2 rounded-md px-3 text-sm font-medium text-(--text-color) outline-none select-none data-[highlighted]:bg-(--menu-highlight-bg) data-[highlighted]:text-(--menu-highlight-text) data-[disabled]:cursor-not-allowed data-[disabled]:data-[highlighted]:bg-transparent data-[disabled]:data-[highlighted]:text-(--text-color) data-[disabled]:focus-visible:ring-2 data-[disabled]:focus-visible:ring-(--text-color) data-[disabled]:focus-visible:ring-inset' const POPUP = - 'z-50 max-h-(--available-height) min-w-48 max-w-[min(32rem,90vw)] overflow-y-auto rounded-lg bg-(--surface) p-1 shadow-(--box-shadow) outline-none' + 'z-50 max-h-(--available-height) min-w-48 max-w-[min(32rem,90vw)] overflow-y-auto rounded-lg border border-(--text-color) bg-(--surface) p-1 shadow-(--box-shadow) outline-none' type Option = { value: string; label: string } diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 8ff38d8..8667df9 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -113,12 +113,14 @@ const COLUMN_HEADER = // The whole row is one link, so the whole row answers when the pointer or the // keyboard is on it: it gets taller (5 px above and below, in front of the next // rows) and, in dark mode, a little lighter. Only the height changes, so nothing -// moves and the table does not get wider. A shadow does it. The text looks only -// a little bolder (a very thin shadow in the color of the text): real bold makes -// the words wider and moves the line breaks, and more than a little makes the -// text harder to read. (The classes are written out, so Tailwind can find them.) +// moves and the table does not get wider. A shadow does it. The first cell also gets a +// 4 px edge in the text color on its left side, so the row can be told from the others in +// light mode too, where its color does not change. The text itself is not changed: a thin +// shadow around the letters (to make them look bolder) is blended differently by each +// browser, and real bold makes the words wider and moves the line breaks. (The classes +// are written out, so Tailwind can find them.) const ROW_EFFECT = - 'group relative cursor-pointer [--row-bg:var(--surface)] even:[--row-bg:var(--table-row-alt)] [--row-hover:color-mix(in_srgb,var(--row-bg),var(--text-color)_var(--row-hover-amount))] hover:*:bg-(--row-hover) focus-within:*:bg-(--row-hover) not-last:hover:bg-(--row-hover) not-last:focus-within:bg-(--row-hover) hover:z-10 focus-within:z-10 hover:*:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] focus-within:*:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] hover:[text-shadow:0.15px_0_0_currentColor,-0.15px_0_0_currentColor] focus-within:[text-shadow:0.15px_0_0_currentColor,-0.15px_0_0_currentColor] *:transition-[box-shadow,background-color] *:duration-150 motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' + 'group relative cursor-pointer [--row-bg:var(--surface)] even:[--row-bg:var(--table-row-alt)] [--row-hover:color-mix(in_srgb,var(--row-bg),var(--text-color)_var(--row-hover-amount))] hover:*:bg-(--row-hover) focus-within:*:bg-(--row-hover) not-last:hover:bg-(--row-hover) not-last:focus-within:bg-(--row-hover) hover:z-10 focus-within:z-10 hover:*:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] focus-within:*:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] hover:*:first:shadow-[inset_4px_0_0_0_var(--text-color),0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] focus-within:*:first:shadow-[inset_4px_0_0_0_var(--text-color),0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] *:transition-[box-shadow,background-color] *:duration-150 motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' // The colors are on the cells (so the corners can be round). const ROW_LOOK = diff --git a/src/components/elements/SortMenu.tsx b/src/components/elements/SortMenu.tsx index 05b2e88..e914c17 100644 --- a/src/components/elements/SortMenu.tsx +++ b/src/components/elements/SortMenu.tsx @@ -11,7 +11,7 @@ import { } from '@/lib/searchOperations' const ITEM = - 'flex min-h-10 cursor-default items-center gap-2 rounded-md px-3 text-sm font-medium text-(--text-color) outline-none select-none data-[highlighted]:bg-(--button-hover-bg) data-[disabled]:opacity-60' + 'flex min-h-10 cursor-default items-center gap-2 rounded-md px-3 text-sm font-medium text-(--text-color) outline-none select-none data-[highlighted]:bg-(--menu-highlight-bg) data-[highlighted]:text-(--menu-highlight-text) data-[disabled]:cursor-not-allowed data-[disabled]:data-[highlighted]:bg-transparent data-[disabled]:data-[highlighted]:text-(--text-color) data-[disabled]:focus-visible:ring-2 data-[disabled]:focus-visible:ring-(--text-color) data-[disabled]:focus-visible:ring-inset data-[disabled]:opacity-60' const LABEL = 'px-3 pt-2 pb-1 text-xs font-bold text-(--text-color)' @@ -62,7 +62,7 @@ export default function SortMenu({ - + Sort by
-
+ {/* Below 600 px the group is "contents", so the sort button is in the same row as the + last filter when there is room for it, and below it when there is not. */} +
{ changeFilters(() => NO_FILTERS) // The button goes away when the filters are cleared, and the focus must not diff --git a/src/styles/variables.css b/src/styles/variables.css index 0f78d26..12f3813 100644 --- a/src/styles/variables.css +++ b/src/styles/variables.css @@ -51,6 +51,10 @@ --button-bg: var(--surface); --button-hover-bg: var(--primary-color); --button-text: var(--text-color); + /* The choice under the pointer or the keyboard in a menu: the navy of the table head and the + badges, with the light text on it (15.9:1, and 15.9:1 against the menu). */ + --menu-highlight-bg: var(--text-color); + --menu-highlight-text: var(--background-color-element); /* The search field: as the design in light mode. In dark mode it is see-through with a tan line, so it can be seen without a solid color. */ --search-bg: var(--surface); @@ -141,6 +145,9 @@ --button-bg: var(--pill-bg); --button-hover-bg: color-mix(in srgb, var(--pill-bg), white 25%); --button-text: var(--pill-text); + /* In dark mode the light tan with dark text (9.0:1, and 6.6:1 against the menu). */ + --menu-highlight-bg: var(--button-hover-bg); + --menu-highlight-text: var(--pill-text); --search-bg: color-mix(in srgb, var(--text-color) 10%, transparent); --search-hover-bg: color-mix(in srgb, var(--text-color) 16%, transparent); --search-border: var(--pill-bg); diff --git a/src/tests/components/elements/ResultsTable.test.tsx b/src/tests/components/elements/ResultsTable.test.tsx index 2e71d7a..686ba4c 100644 --- a/src/tests/components/elements/ResultsTable.test.tsx +++ b/src/tests/components/elements/ResultsTable.test.tsx @@ -67,13 +67,15 @@ describe('ResultsTable: the table (a wide screen)', () => { expect(table).toContain( 'focus-within:*:shadow-[0_-5px_0_0_var(--row-hover)', ) - // The text only gets a very thin shadow in its own color (a little bolder). More - // than that makes it harder to read. - expect(table).toContain('hover:[text-shadow:0.15px_0_0_currentColor,') + // The first cell gets an edge in the text color. The letters get no text shadow, + // because each browser blends it differently. expect(table).toContain( - 'focus-within:[text-shadow:0.15px_0_0_currentColor,', + 'hover:*:first:shadow-[inset_4px_0_0_0_var(--text-color)', ) - expect(table).not.toContain('0.35px') + expect(table).toContain( + 'focus-within:*:first:shadow-[inset_4px_0_0_0_var(--text-color)', + ) + expect(table).not.toContain('text-shadow') expect(table).not.toContain('hover:font-') expect(table).not.toContain('scale-') expect(table).not.toContain('underline')