From 09d94e513e0d5fbaee33ae0e7c0b29ff64a76e1a Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 22 Jul 2026 15:30:40 +0100 Subject: [PATCH 1/7] docs(compare): design for unified InfoPopover metric-help affordance Co-Authored-By: Claude Opus 4.8 --- .../specs/2026-07-22-info-popover-design.md | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-22-info-popover-design.md diff --git a/docs/superpowers/specs/2026-07-22-info-popover-design.md b/docs/superpowers/specs/2026-07-22-info-popover-design.md new file mode 100644 index 0000000..5222ec2 --- /dev/null +++ b/docs/superpowers/specs/2026-07-22-info-popover-design.md @@ -0,0 +1,125 @@ +# Info Popover — unified metric-help affordance + +**Date:** 2026-07-22 +**Status:** Approved (design) + +## Problem + +Two different "info affordance" patterns explain metrics across the app, and both are broken: + +1. **`MetricTooltip` (ⓘ)** — used on the two detail pages (`SchoolDetailView`, + `SecondarySchoolDetailView`). Its bubble is `position: absolute` with a fixed + `220px` width and no viewport-collision detection. Near a screen edge on + mobile the bubble renders **partly or wholly off-viewport with no way to + scroll to it** — effectively unusable. A `left: -12px` mobile hack only + shifts the problem, it doesn't solve it. + +2. **Compare-section `?` help** — every compare section funnels through + `RowLabel` in `components/compare/sectionShared.tsx`, which uses the **native + `title=` attribute**. On desktop the native tooltip has a long, unconfigurable + hover delay; on touch it barely surfaces at all. + +We want a single component that positions itself correctly in any viewport and +shows a custom (non-native) tooltip on desktop. + +## Decisions + +- **Positioning: `@floating-ui/react`** (industry standard, ~10KB gzipped, React + 19 compatible). Chosen over a hand-rolled portal + JS positioning because the + current hand-rolled approach is exactly what failed, and Floating UI already + solves flip/shift/portal/interactions/ARIA. +- **Mobile presentation: repositioning popover** (not a bottom-sheet). Same small + bubble as desktop; Floating UI's `shift`/`flip` keep it fully on-screen. One + presentation to build and maintain, consistent across platforms. +- **Glyph: standardise on the circled `?`** everywhere (replaces ⓘ on the detail + pages). Matches the common "help" convention. +- **Bundle:** adding `@floating-ui/react` as a runtime dependency is accepted. + +## Architecture + +One shared engine, two thin adapters — **no call-site churn**. + +### `InfoPopover` (new — `components/InfoPopover.tsx`) + +Owns all behaviour via `@floating-ui/react`. + +- **Trigger:** a real ` + {open && ( + +
+ + {label && {label}} + {plain} + {detail && {detail}} +
+ + )} + + ); +} +``` + +- [ ] **Step 4: Write the styles** + +Create `nextjs-app/components/InfoPopover.module.css`: +```css +.icon { + display: inline-flex; + align-items: center; + justify-content: center; + min-width: 24px; + min-height: 24px; + margin: -6px 0; + padding: 0; + border: none; + background: none; + /* font-size:0 hides the button's own "?" text node; the ::before glyph + below carries the visible circled "?" at its own explicit size. */ + font-size: 0; + color: var(--text-muted, #8a7a72); + cursor: help; + line-height: 1; + user-select: none; + transition: color 0.15s ease; +} + +/* The visible affordance: a small circled "?" centred in the 24px target. */ +.icon::before { + content: '?'; + display: inline-flex; + align-items: center; + justify-content: center; + width: 15px; + height: 15px; + border-radius: 50%; + border: 1px solid currentColor; + font-size: 0.65rem; +} + +.icon:hover, +.icon[aria-expanded='true'], +.icon:focus-visible { + color: var(--accent-coral-dark, #b04a2e); +} + +.tooltip { + z-index: 9999; + width: max-content; + max-width: min(260px, calc(100vw - 24px)); + background: var(--bg-primary, #faf7f2); + border: 1px solid var(--border-color, #e8ddd4); + border-radius: 10px; + box-shadow: 0 4px 16px rgba(44, 36, 32, 0.15); + padding: 0.6rem 0.75rem; + display: flex; + flex-direction: column; + gap: 0.3rem; +} + +.arrow { + fill: var(--bg-primary, #faf7f2); + stroke: var(--border-color, #e8ddd4); + stroke-width: 1px; +} + +.label { + font-weight: 600; + font-size: 0.75rem; + color: var(--text-primary, #2c2420); +} + +.plain { + font-size: 0.75rem; + color: var(--text-secondary, #5a4a44); + line-height: 1.4; +} + +.detail { + font-size: 0.7rem; + color: var(--text-muted, #8a7a72); + line-height: 1.4; + margin-top: 0.1rem; +} +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: +```bash +npx jest __tests__/components/InfoPopover.test.tsx +``` +Expected: PASS (6 tests). + +- [ ] **Step 6: Typecheck** + +Run: +```bash +npx tsc --noEmit +``` +Expected: no output (exit 0). + +- [ ] **Step 7: Commit** + +```bash +git add components/InfoPopover.tsx components/InfoPopover.module.css __tests__/components/InfoPopover.test.tsx +git commit -m "feat(ui): add InfoPopover — viewport-aware metric help via Floating UI" +``` + +--- + +### Task 3: Reduce `MetricTooltip` to a thin adapter over `InfoPopover` + +**Files:** +- Modify: `nextjs-app/components/MetricTooltip.tsx` (full rewrite of body) +- Delete: `nextjs-app/components/MetricTooltip.module.css` +- Test: `nextjs-app/__tests__/components/MetricTooltip.test.tsx` (create) + +**Interfaces:** +- Consumes: `InfoPopover` (Task 2), `METRIC_EXPLANATIONS` from `@/lib/metrics`. +- Produces: `MetricTooltip` with unchanged public props + `{ metricKey?: string; label?: string; plain?: string; detail?: string }`. + Resolves `metricKey` → `METRIC_EXPLANATIONS[metricKey]`, with explicit + `label`/`plain`/`detail` props overriding the looked-up values. Passes the + resolved label as `InfoPopover`'s `ariaLabel`. + +- [ ] **Step 1: Write the failing test** + +Create `nextjs-app/__tests__/components/MetricTooltip.test.tsx`: +```tsx +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MetricTooltip } from '@/components/MetricTooltip'; +import { METRIC_EXPLANATIONS } from '@/lib/metrics'; + +describe('MetricTooltip', () => { + it('resolves content from a metricKey', async () => { + const key = Object.keys(METRIC_EXPLANATIONS)[0]; + const exp = METRIC_EXPLANATIONS[key]; + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button', { name: exp.label })); + const tip = await screen.findByRole('tooltip'); + expect(tip).toHaveTextContent(exp.plain); + }); + + it('renders nothing for an unknown metricKey with no explicit content', () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + + it('lets explicit props override the looked-up explanation', async () => { + const key = Object.keys(METRIC_EXPLANATIONS)[0]; + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button')); + expect(await screen.findByRole('tooltip')).toHaveTextContent('custom text'); + }); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: +```bash +npx jest __tests__/components/MetricTooltip.test.tsx +``` +Expected: FAIL — the old `MetricTooltip` renders a `.tooltip` span even when +collapsed (no `role="tooltip"` gating on open) / different structure, so +`findByRole('tooltip')` after click and the empty-render assertion diverge from +the new contract. (If any assertion happens to pass against the old component, +it still must fail overall before Step 3.) + +- [ ] **Step 3: Rewrite the component** + +Replace the entire contents of `nextjs-app/components/MetricTooltip.tsx` with: +```tsx +'use client'; + +import { METRIC_EXPLANATIONS } from '@/lib/metrics'; +import { InfoPopover } from './InfoPopover'; + +interface MetricTooltipProps { + metricKey?: string; + label?: string; + plain?: string; + detail?: string; +} + +export function MetricTooltip({ metricKey, label, plain, detail }: MetricTooltipProps) { + const explanation = metricKey ? METRIC_EXPLANATIONS[metricKey] : undefined; + const resolvedLabel = label ?? explanation?.label; + return ( + + ); +} +``` + +- [ ] **Step 4: Delete the now-unused stylesheet** + +Run: +```bash +git rm components/MetricTooltip.module.css +``` +Expected: file staged for deletion. (The old component was its only importer; +`InfoPopover.module.css` supersedes it.) + +- [ ] **Step 5: Verify nothing else imports the deleted CSS** + +Run: +```bash +grep -rn "MetricTooltip.module.css" components app lib || echo "no importers" +``` +Expected: `no importers`. + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: +```bash +npx jest __tests__/components/MetricTooltip.test.tsx +``` +Expected: PASS (3 tests). + +- [ ] **Step 7: Typecheck** + +Run: +```bash +npx tsc --noEmit +``` +Expected: no output (exit 0). + +- [ ] **Step 8: Commit** + +```bash +git add components/MetricTooltip.tsx __tests__/components/MetricTooltip.test.tsx +git commit -m "refactor(ui): MetricTooltip delegates to InfoPopover (circled ? glyph)" +``` + +--- + +### Task 4: Swap the compare `?` help (`RowLabel`) to `InfoPopover` + +**Files:** +- Modify: `nextjs-app/components/compare/sectionShared.tsx` (`RowLabel`) +- Modify: `nextjs-app/components/compare/compareSections.module.css` (remove `.help`) +- Test: `nextjs-app/__tests__/components/sectionShared.test.tsx` (create) + +**Interfaces:** +- Consumes: `InfoPopover` (Task 2). +- Produces: `RowLabel({ children, tip })` renders the label text plus, when + `tip` is set, an `InfoPopover` with `plain={tip}`. `ariaLabel` is omitted, so + the trigger uses `InfoPopover`'s default accessible name `"More information"` + (the row label is arbitrary `ReactNode`, so there is no clean string to derive + a per-row name from). `Measure` and all compare `tip=` call sites are + unchanged. + +- [ ] **Step 1: Write the failing test** + +Create `nextjs-app/__tests__/components/sectionShared.test.tsx`: +```tsx +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { RowLabel } from '@/components/compare/sectionShared'; + +describe('RowLabel', () => { + it('renders its label text', () => { + render(Attainment 8); + expect(screen.getByText('Attainment 8')).toBeInTheDocument(); + }); + + it('shows no help affordance when no tip is given', () => { + render(Attainment 8); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + + it('opens the tip in a popover on click', async () => { + const user = userEvent.setup(); + render(Attainment 8); + await user.click(screen.getByRole('button')); + expect(await screen.findByRole('tooltip')).toHaveTextContent( + 'Average GCSE score across 8 subjects', + ); + }); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: +```bash +npx jest __tests__/components/sectionShared.test.tsx +``` +Expected: FAIL — the current `RowLabel` renders a `` (not a +`button`/`role="tooltip"`), so the click test fails. + +- [ ] **Step 3: Update `RowLabel`** + +In `nextjs-app/components/compare/sectionShared.tsx`, add the import near the +other imports: +```tsx +import { InfoPopover } from '@/components/InfoPopover'; +``` +Then replace the `RowLabel` function: +```tsx +export function RowLabel({ children, tip }: { children: ReactNode; tip?: string }) { + return ( +
+ {children} + {tip && } +
+ ); +} +``` + +- [ ] **Step 4: Remove the dead `.help` style** + +In `nextjs-app/components/compare/compareSections.module.css`, delete the entire +`.help { … }` rule (the `display: inline-flex; width: 15px; … flex: none;` +block — the circled-`?` styling now lives in `InfoPopover.module.css`). + +- [ ] **Step 5: Confirm `.help` is unreferenced** + +Run: +```bash +grep -rn "styles.help\|\.help\b" components/compare || echo "no references" +``` +Expected: `no references`. + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: +```bash +npx jest __tests__/components/sectionShared.test.tsx +``` +Expected: PASS (3 tests). + +- [ ] **Step 7: Full unit suite + typecheck** + +Run: +```bash +npx tsc --noEmit && npx jest +``` +Expected: typecheck clean; all suites pass (existing + the 3 new suites). + +- [ ] **Step 8: Commit** + +```bash +git add components/compare/sectionShared.tsx components/compare/compareSections.module.css __tests__/components/sectionShared.test.tsx +git commit -m "refactor(compare): row-label help uses InfoPopover, not native title" +``` + +--- + +### Task 5: E2E regression guard — popover stays within the mobile viewport + +**Files:** +- Modify: `nextjs-app/../e2e/tests/journeys.spec.ts` (add one test) + +**Interfaces:** +- Consumes: the running app (staging/local baseURL), the `twoPrimaryUrns` + helper already defined in `journeys.spec.ts`. +- Produces: a Playwright test asserting an opened compare help popover's + bounding box is fully within the viewport on a narrow screen. + +- [ ] **Step 1: Add the failing-guard test** + +Append to `nextjs-app/../e2e/tests/journeys.spec.ts` (i.e. `e2e/tests/journeys.spec.ts`): +```ts +test('compare metric-help popover stays within the mobile viewport', async ({ page }) => { + await page.setViewportSize({ width: 390, height: 844 }); + const [urn0, urn1] = await twoPrimaryUrns(page); + await page.goto(`/compare?urns=${urn0},${urn1}`); + + await expect( + page.getByRole('heading', { name: 'At a glance' }), + ).toBeVisible({ timeout: 15_000 }); + + // The metric-help triggers are the circled-"?" buttons in the row labels. + // "More information" is InfoPopover's default accessible name. + const help = page.getByRole('button', { name: 'More information' }).first(); + await expect(help).toBeVisible({ timeout: 15_000 }); + await help.click(); + + const tip = page.getByRole('tooltip'); + await expect(tip).toBeVisible(); + + // The whole bubble must sit inside the viewport — the original bug pushed it + // off the right edge with no way to scroll to it. + const box = await tip.boundingBox(); + const width = page.viewportSize()!.width; + expect(box).not.toBeNull(); + expect(box!.x).toBeGreaterThanOrEqual(0); + expect(box!.x + box!.width).toBeLessThanOrEqual(width); + + // And the page must not have gained a horizontal scrollbar from the bubble. + const bodyOverflowsX = await page + .locator('body') + .evaluate((el) => el.scrollWidth > el.clientWidth + 1); + expect(bodyOverflowsX).toBe(false); +}); +``` + +- [ ] **Step 2: Lint/typecheck the e2e file** + +Run (from `e2e/`): +```bash +cd ../e2e && npx tsc --noEmit -p . 2>/dev/null || npx tsc --noEmit journeys 2>/dev/null; cd ../nextjs-app +``` +Expected: no type errors reported for `journeys.spec.ts`. (If the e2e package +has no standalone tsconfig, this is a no-op; the CI Playwright run type-checks +on execution.) + +- [ ] **Step 3: Note on running e2e** + +The e2e journeys run against a deployed environment (staging) in CI, per +`CLAUDE.md`; they are not run locally here (no local server). This test will +execute in the staging gate after merge. Do not attempt to start a local server. + +- [ ] **Step 4: Commit** + +```bash +git add ../e2e/tests/journeys.spec.ts +git commit -m "test(e2e): compare help popover stays within the mobile viewport" +``` + +--- + +### Task 6: Final verification and PR + +**Files:** none (verification + PR). + +- [ ] **Step 1: Full typecheck + unit suite** + +Run (from `nextjs-app/`): +```bash +npx tsc --noEmit && npx jest 2>&1 | tail -8 +``` +Expected: typecheck clean; all suites pass. + +- [ ] **Step 2: Production build sanity (catches client/server boundary issues)** + +Run: +```bash +npx next build 2>&1 | tail -20 +``` +Expected: build completes without errors. (`InfoPopover` is a client +component — `'use client'` — so this confirms the portal usage compiles.) + +- [ ] **Step 3: Confirm no stragglers reference removed APIs** + +Run: +```bash +grep -rn "MetricTooltip.module.css\|styles.help\|title={tip}" components app || echo "clean" +``` +Expected: `clean`. + +- [ ] **Step 4: Push and open the PR** + +```bash +git push -u origin feat/info-popover-tooltip +``` +Then open a Gitea PR (base `main`, head `feat/info-popover-tooltip`) via the +credential-helper + API pattern used in this repo, summarising: the two bugs +fixed (off-viewport mobile bubble; slow native-`title` desktop hover), the +unified `InfoPopover` approach, the circled-`?` standardisation, the new +`@floating-ui/react` dependency, and the mobile-viewport e2e guard. +``` From ddb42badb630b0f00d9493dd9536bb2cec67ad60 Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 22 Jul 2026 15:35:21 +0100 Subject: [PATCH 3/7] build(deps): add @floating-ui/react for the info popover Co-Authored-By: Claude Opus 4.8 --- nextjs-app/package-lock.json | 60 ++++++++++++++++++++++++++++++++++++ nextjs-app/package.json | 1 + 2 files changed, 61 insertions(+) diff --git a/nextjs-app/package-lock.json b/nextjs-app/package-lock.json index ef88bb3..4ce1fb4 100644 --- a/nextjs-app/package-lock.json +++ b/nextjs-app/package-lock.json @@ -8,6 +8,7 @@ "name": "nextjs-app", "version": "0.1.0", "dependencies": { + "@floating-ui/react": "^0.27.20", "@types/node": "^25.2.0", "@types/react": "^19.2.10", "@types/react-dom": "^19.2.3", @@ -832,6 +833,59 @@ "node": "^18.18.0 || ^20.9.0 || >=21.1.0" } }, + "node_modules/@floating-ui/core": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@floating-ui/core/-/core-1.8.0.tgz", + "integrity": "sha512-0CIZ5itps/8x7BG8dEIhs53BvCUH2PCoogtakwRTut+Arm58sJooJ0AuZhLw2HJYIR5cMLNPBSS728sPho2khQ==", + "license": "MIT", + "dependencies": { + "@floating-ui/utils": "^0.2.12" + } + }, + "node_modules/@floating-ui/dom": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@floating-ui/dom/-/dom-1.8.0.tgz", + "integrity": "sha512-yXSrzeHZBTZadLOlfyhCkJHNeLJnHRnRInwdZ40L7ZiaAtrBwoYlsDrX3v5zB1Utk7CLfzcOVnVVWoXEky7Ceg==", + "license": "MIT", + "dependencies": { + "@floating-ui/core": "^1.8.0", + "@floating-ui/utils": "^0.2.12" + } + }, + "node_modules/@floating-ui/react": { + "version": "0.27.20", + "resolved": "https://registry.npmjs.org/@floating-ui/react/-/react-0.27.20.tgz", + "integrity": "sha512-CMqMy7OaXl9W0eq1Uy7L7i2Y/anPvHmFmESd2CEw0t5YvZhcVCeo4MBevAmswRllX7Y2dEidA4ozGPunLSTQpw==", + "license": "MIT", + "dependencies": { + "@floating-ui/react-dom": "^2.1.9", + "@floating-ui/utils": "^0.2.12", + "tabbable": "^6.0.0" + }, + "peerDependencies": { + "react": ">=17.0.0", + "react-dom": ">=17.0.0" + } + }, + "node_modules/@floating-ui/react-dom": { + "version": "2.1.9", + "resolved": "https://registry.npmjs.org/@floating-ui/react-dom/-/react-dom-2.1.9.tgz", + "integrity": "sha512-JDjEFGCpImxDCA7JJKviA0M9+RtmJdj0m/NVU5IMgBK+AmZouAQQ7/+2GLH0GXXY0YMw9oXPB8hKdbPYg5QLYg==", + "license": "MIT", + "dependencies": { + "@floating-ui/dom": "^1.8.0" + }, + "peerDependencies": { + "react": ">=16.8.0", + "react-dom": ">=16.8.0" + } + }, + "node_modules/@floating-ui/utils": { + "version": "0.2.12", + "resolved": "https://registry.npmjs.org/@floating-ui/utils/-/utils-0.2.12.tgz", + "integrity": "sha512-HpCo8tmWzLVad5s2d19EhAz5zqrrQ6s69qd6moPMQvkOuSwDT1YgRfWSVuc4ennqrgv3OHppiOGMQ7oC13yIww==", + "license": "MIT" + }, "node_modules/@humanfs/core": { "version": "0.19.1", "resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.1.tgz", @@ -9136,6 +9190,12 @@ "url": "https://opencollective.com/synckit" } }, + "node_modules/tabbable": { + "version": "6.5.0", + "resolved": "https://registry.npmjs.org/tabbable/-/tabbable-6.5.0.tgz", + "integrity": "sha512-wieBHXygIm7OyQOu5hQlkk62/WyCFYGlWg7L6/ZCUZwx0o398Zkn4pVmMyfYhfMG8kGrj/Krt8eIk6UKC6VzwA==", + "license": "MIT" + }, "node_modules/test-exclude": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/test-exclude/-/test-exclude-6.0.0.tgz", diff --git a/nextjs-app/package.json b/nextjs-app/package.json index 742c9b6..d38b908 100644 --- a/nextjs-app/package.json +++ b/nextjs-app/package.json @@ -13,6 +13,7 @@ "test:coverage": "jest --coverage" }, "dependencies": { + "@floating-ui/react": "^0.27.20", "@types/node": "^25.2.0", "@types/react": "^19.2.10", "@types/react-dom": "^19.2.3", From 1d9d2eb5aefc971f898da5074697ac7fcde2068b Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 22 Jul 2026 15:36:23 +0100 Subject: [PATCH 4/7] =?UTF-8?q?feat(ui):=20add=20InfoPopover=20=E2=80=94?= =?UTF-8?q?=20viewport-aware=20metric=20help=20via=20Floating=20UI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 --- .../__tests__/components/InfoPopover.test.tsx | 64 +++++++++++++ nextjs-app/components/InfoPopover.module.css | 77 +++++++++++++++ nextjs-app/components/InfoPopover.tsx | 93 +++++++++++++++++++ 3 files changed, 234 insertions(+) create mode 100644 nextjs-app/__tests__/components/InfoPopover.test.tsx create mode 100644 nextjs-app/components/InfoPopover.module.css create mode 100644 nextjs-app/components/InfoPopover.tsx diff --git a/nextjs-app/__tests__/components/InfoPopover.test.tsx b/nextjs-app/__tests__/components/InfoPopover.test.tsx new file mode 100644 index 0000000..526b736 --- /dev/null +++ b/nextjs-app/__tests__/components/InfoPopover.test.tsx @@ -0,0 +1,64 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { InfoPopover } from '@/components/InfoPopover'; + +describe('InfoPopover', () => { + it('renders nothing when there is no plain content', () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + + it('renders a labelled, collapsed trigger button', () => { + render(); + const btn = screen.getByRole('button', { name: 'Reading score' }); + expect(btn).toHaveAttribute('aria-expanded', 'false'); + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); + }); + + it('opens on click and shows label, plain and detail', async () => { + const user = userEvent.setup(); + render( + , + ); + await user.click(screen.getByRole('button', { name: 'RWM' })); + const tip = await screen.findByRole('tooltip'); + expect(tip).toHaveTextContent('Reading, Writing & Maths'); + expect(tip).toHaveTextContent('% reaching the expected standard'); + expect(tip).toHaveTextContent('National average ~60%'); + expect(screen.getByRole('button', { name: 'RWM' })).toHaveAttribute( + 'aria-expanded', + 'true', + ); + }); + + it('closes again on a second click', async () => { + const user = userEvent.setup(); + render(); + const btn = screen.getByRole('button', { name: 'Info' }); + await user.click(btn); + expect(await screen.findByRole('tooltip')).toBeInTheDocument(); + await user.click(btn); + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); + }); + + it('closes on Escape', async () => { + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button', { name: 'Info' })); + expect(await screen.findByRole('tooltip')).toBeInTheDocument(); + await user.keyboard('{Escape}'); + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); + }); + + it('defaults the accessible name when no ariaLabel is given', () => { + render(); + expect( + screen.getByRole('button', { name: 'More information' }), + ).toBeInTheDocument(); + }); +}); diff --git a/nextjs-app/components/InfoPopover.module.css b/nextjs-app/components/InfoPopover.module.css new file mode 100644 index 0000000..297a184 --- /dev/null +++ b/nextjs-app/components/InfoPopover.module.css @@ -0,0 +1,77 @@ +.icon { + display: inline-flex; + align-items: center; + justify-content: center; + min-width: 24px; + min-height: 24px; + margin: -6px 0; + padding: 0; + border: none; + background: none; + /* font-size:0 hides the button's own "?" text node; the ::before glyph + below carries the visible circled "?" at its own explicit size. */ + font-size: 0; + color: var(--text-muted, #8a7a72); + cursor: help; + line-height: 1; + user-select: none; + transition: color 0.15s ease; +} + +/* The visible affordance: a small circled "?" centred in the 24px target. */ +.icon::before { + content: '?'; + display: inline-flex; + align-items: center; + justify-content: center; + width: 15px; + height: 15px; + border-radius: 50%; + border: 1px solid currentColor; + font-size: 0.65rem; +} + +.icon:hover, +.icon[aria-expanded='true'], +.icon:focus-visible { + color: var(--accent-coral-dark, #b04a2e); +} + +.tooltip { + z-index: 9999; + width: max-content; + max-width: min(260px, calc(100vw - 24px)); + background: var(--bg-primary, #faf7f2); + border: 1px solid var(--border-color, #e8ddd4); + border-radius: 10px; + box-shadow: 0 4px 16px rgba(44, 36, 32, 0.15); + padding: 0.6rem 0.75rem; + display: flex; + flex-direction: column; + gap: 0.3rem; +} + +.arrow { + fill: var(--bg-primary, #faf7f2); + stroke: var(--border-color, #e8ddd4); + stroke-width: 1px; +} + +.label { + font-weight: 600; + font-size: 0.75rem; + color: var(--text-primary, #2c2420); +} + +.plain { + font-size: 0.75rem; + color: var(--text-secondary, #5a4a44); + line-height: 1.4; +} + +.detail { + font-size: 0.7rem; + color: var(--text-muted, #8a7a72); + line-height: 1.4; + margin-top: 0.1rem; +} diff --git a/nextjs-app/components/InfoPopover.tsx b/nextjs-app/components/InfoPopover.tsx new file mode 100644 index 0000000..4c5e812 --- /dev/null +++ b/nextjs-app/components/InfoPopover.tsx @@ -0,0 +1,93 @@ +'use client'; + +import { useRef, useState } from 'react'; +import { + useFloating, + autoUpdate, + offset, + flip, + shift, + arrow, + useHover, + useFocus, + useClick, + useDismiss, + useRole, + useInteractions, + FloatingPortal, + FloatingArrow, +} from '@floating-ui/react'; +import styles from './InfoPopover.module.css'; + +export interface InfoPopoverProps { + label?: string; + plain?: string; + detail?: string; + ariaLabel?: string; +} + +export function InfoPopover({ label, plain, detail, ariaLabel }: InfoPopoverProps) { + const [open, setOpen] = useState(false); + const arrowRef = useRef(null); + + const { refs, floatingStyles, context } = useFloating({ + open, + onOpenChange: setOpen, + placement: 'top', + whileElementsMounted: autoUpdate, + middleware: [ + offset(8), + flip({ fallbackAxisSideDirection: 'start' }), + shift({ padding: 8 }), + arrow({ element: arrowRef, padding: 8 }), + ], + }); + + // Hover (desktop) with a short open delay, keyboard focus, tap (touch), + // outside-press + Escape to dismiss. Floating UI disables hover on touch, + // so tap and hover never double-fire. + const hover = useHover(context, { delay: { open: 100, close: 0 } }); + const focus = useFocus(context); + const click = useClick(context); + const dismiss = useDismiss(context); + const role = useRole(context, { role: 'tooltip' }); + const { getReferenceProps, getFloatingProps } = useInteractions([ + hover, + focus, + click, + dismiss, + role, + ]); + + if (!plain) return null; + + return ( + <> + + {open && ( + +
+ + {label && {label}} + {plain} + {detail && {detail}} +
+ + )} + + ); +} From 15b7493b85548f2daef33fa80f31e1db77f9f59e Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 22 Jul 2026 15:37:33 +0100 Subject: [PATCH 5/7] refactor(ui): MetricTooltip delegates to InfoPopover (circled ? glyph) Co-Authored-By: Claude Opus 4.8 --- .../components/MetricTooltip.test.tsx | 32 +++++ .../components/MetricTooltip.module.css | 114 ------------------ nextjs-app/components/MetricTooltip.tsx | 55 ++------- 3 files changed, 40 insertions(+), 161 deletions(-) create mode 100644 nextjs-app/__tests__/components/MetricTooltip.test.tsx delete mode 100644 nextjs-app/components/MetricTooltip.module.css diff --git a/nextjs-app/__tests__/components/MetricTooltip.test.tsx b/nextjs-app/__tests__/components/MetricTooltip.test.tsx new file mode 100644 index 0000000..7c6c191 --- /dev/null +++ b/nextjs-app/__tests__/components/MetricTooltip.test.tsx @@ -0,0 +1,32 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MetricTooltip } from '@/components/MetricTooltip'; +import { METRIC_EXPLANATIONS } from '@/lib/metrics'; + +describe('MetricTooltip', () => { + it('resolves content from a metricKey', async () => { + const key = Object.keys(METRIC_EXPLANATIONS)[0]; + const exp = METRIC_EXPLANATIONS[key]; + const user = userEvent.setup(); + render(); + // Collapsed by default — the popover content is not in the DOM until opened + // (the old component left an always-present role="tooltip" span behind). + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: `What does ${exp.label} mean?` })); + const tip = await screen.findByRole('tooltip'); + expect(tip).toHaveTextContent(exp.plain); + }); + + it('renders nothing for an unknown metricKey with no explicit content', () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + + it('lets explicit props override the looked-up explanation', async () => { + const key = Object.keys(METRIC_EXPLANATIONS)[0]; + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole('button')); + expect(await screen.findByRole('tooltip')).toHaveTextContent('custom text'); + }); +}); diff --git a/nextjs-app/components/MetricTooltip.module.css b/nextjs-app/components/MetricTooltip.module.css deleted file mode 100644 index ccf0801..0000000 --- a/nextjs-app/components/MetricTooltip.module.css +++ /dev/null @@ -1,114 +0,0 @@ -.wrapper { - position: relative; - display: inline-flex; - align-items: center; - margin-left: 0.3em; -} - -.icon { - /* A real button: 24px tap target (WCAG 2.5.8) drawn as the small glyph. */ - display: inline-flex; - align-items: center; - justify-content: center; - min-width: 24px; - min-height: 24px; - margin: -6px 0; - padding: 0; - border: none; - background: none; - font-size: 0.9em; - color: var(--text-muted, #8a7a72); - cursor: help; - line-height: 1; - user-select: none; - transition: color 0.15s ease; -} - -.wrapper:hover .icon, -.icon[aria-expanded="true"] { - color: var(--accent-coral-dark, #b04a2e); -} - -.tooltip { - visibility: hidden; - opacity: 0; - position: absolute; - bottom: calc(100% + 6px); - left: 50%; - transform: translateX(-50%); - z-index: 9999; - width: 220px; - background: var(--bg-primary, #faf7f2); - border: 1px solid var(--border-color, #e8ddd4); - border-radius: 10px; - box-shadow: 0 4px 16px rgba(44, 36, 32, 0.15); - padding: 0.6rem 0.75rem; - display: flex; - flex-direction: column; - gap: 0.3rem; - pointer-events: none; - transition: opacity 0.15s ease, visibility 0.15s ease; -} - -/* Reveal on hover (desktop), keyboard focus, or explicit tap/click toggle. */ -.wrapper:hover .tooltip, -.wrapper:focus-within .tooltip, -.tooltipOpen { - visibility: visible; - opacity: 1; -} - -.tooltipOpen { - pointer-events: auto; -} - -/* Small arrow pointing down */ -.tooltip::after { - content: ''; - position: absolute; - top: 100%; - left: 50%; - transform: translateX(-50%); - border: 5px solid transparent; - border-top-color: var(--border-color, #e8ddd4); -} - -.tooltipLabel { - font-weight: 600; - font-size: 0.75rem; - color: var(--text-primary, #2c2420); -} - -.tooltipPlain { - font-size: 0.75rem; - color: var(--text-secondary, #5a4a44); - line-height: 1.4; -} - -.tooltipDetail { - font-size: 0.7rem; - color: var(--text-muted, #8a7a72); - line-height: 1.4; - margin-top: 0.1rem; -} - -@media (max-width: 480px) { - .tooltip { - width: 180px; - } -} - -/* Anchor the bubble to open rightward on phones — icons follow their labels, - which start at the left edge, so centring pushed the bubble off-screen. */ -@media (max-width: 640px) { - .tooltip { - left: -12px; - right: auto; - transform: none; - } - - .tooltip::after { - left: 16px; - transform: none; - } -} diff --git a/nextjs-app/components/MetricTooltip.tsx b/nextjs-app/components/MetricTooltip.tsx index 6bef6f4..d27fc30 100644 --- a/nextjs-app/components/MetricTooltip.tsx +++ b/nextjs-app/components/MetricTooltip.tsx @@ -1,8 +1,7 @@ 'use client'; -import { useEffect, useRef, useState } from 'react'; import { METRIC_EXPLANATIONS } from '@/lib/metrics'; -import styles from './MetricTooltip.module.css'; +import { InfoPopover } from './InfoPopover'; interface MetricTooltipProps { metricKey?: string; @@ -13,51 +12,13 @@ interface MetricTooltipProps { export function MetricTooltip({ metricKey, label, plain, detail }: MetricTooltipProps) { const explanation = metricKey ? METRIC_EXPLANATIONS[metricKey] : undefined; - const tooltipLabel = label ?? explanation?.label; - const tooltipPlain = plain ?? explanation?.plain; - const tooltipDetail = detail ?? explanation?.detail; - - // Tap/click/keyboard toggle so the definition is reachable on touch devices - // and by keyboard, not just mouse hover (hover still works on desktop). - const [open, setOpen] = useState(false); - const wrapperRef = useRef(null); - - useEffect(() => { - if (!open) return; - const dismiss = (e: Event) => { - if (wrapperRef.current && e.target instanceof Node && !wrapperRef.current.contains(e.target)) { - setOpen(false); - } - }; - const onKey = (e: KeyboardEvent) => { - if (e.key === 'Escape') setOpen(false); - }; - document.addEventListener('click', dismiss); - document.addEventListener('keydown', onKey); - return () => { - document.removeEventListener('click', dismiss); - document.removeEventListener('keydown', onKey); - }; - }, [open]); - - if (!tooltipPlain) return null; - + const resolvedLabel = label ?? explanation?.label; return ( - - - - {tooltipLabel && {tooltipLabel}} - {tooltipPlain} - {tooltipDetail && {tooltipDetail}} - - + ); } From 84bca53c7e0c8ccedebc2da2797285d55282e00f Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 22 Jul 2026 15:38:25 +0100 Subject: [PATCH 6/7] refactor(compare): row-label help uses InfoPopover, not native title Co-Authored-By: Claude Opus 4.8 --- .../components/sectionShared.test.tsx | 24 +++++++++++++++++++ .../compare/compareSections.module.css | 14 ----------- .../components/compare/sectionShared.tsx | 7 ++---- 3 files changed, 26 insertions(+), 19 deletions(-) create mode 100644 nextjs-app/__tests__/components/sectionShared.test.tsx diff --git a/nextjs-app/__tests__/components/sectionShared.test.tsx b/nextjs-app/__tests__/components/sectionShared.test.tsx new file mode 100644 index 0000000..e29da9f --- /dev/null +++ b/nextjs-app/__tests__/components/sectionShared.test.tsx @@ -0,0 +1,24 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { RowLabel } from '@/components/compare/sectionShared'; + +describe('RowLabel', () => { + it('renders its label text', () => { + render(Attainment 8); + expect(screen.getByText('Attainment 8')).toBeInTheDocument(); + }); + + it('shows no help affordance when no tip is given', () => { + render(Attainment 8); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + + it('opens the tip in a popover on click', async () => { + const user = userEvent.setup(); + render(Attainment 8); + await user.click(screen.getByRole('button')); + expect(await screen.findByRole('tooltip')).toHaveTextContent( + 'Average GCSE score across 8 subjects', + ); + }); +}); diff --git a/nextjs-app/components/compare/compareSections.module.css b/nextjs-app/components/compare/compareSections.module.css index f3c5131..af7712f 100644 --- a/nextjs-app/components/compare/compareSections.module.css +++ b/nextjs-app/components/compare/compareSections.module.css @@ -132,20 +132,6 @@ color: var(--text-secondary); } -.help { - display: inline-flex; - width: 15px; - height: 15px; - border-radius: 50%; - border: 1px solid var(--text-muted); - color: var(--text-muted); - font-size: 0.65rem; - align-items: center; - justify-content: center; - cursor: help; - flex: none; -} - .badge { display: inline-block; font-weight: 700; diff --git a/nextjs-app/components/compare/sectionShared.tsx b/nextjs-app/components/compare/sectionShared.tsx index 0c27ea5..74211c9 100644 --- a/nextjs-app/components/compare/sectionShared.tsx +++ b/nextjs-app/components/compare/sectionShared.tsx @@ -11,6 +11,7 @@ import type { CSSProperties, ReactNode } from 'react'; import type { School } from '@/lib/types'; import { CHART_COLORS, CHART_TEXT_COLORS, shortName } from '@/lib/utils'; +import { InfoPopover } from '@/components/InfoPopover'; import styles from './compareSections.module.css'; export function Section({ @@ -52,11 +53,7 @@ export function RowLabel({ children, tip }: { children: ReactNode; tip?: string return (
{children} - {tip && ( - - ? - - )} + {tip && }
); } From 8e763e39d17a4964cf558e51c03f044186371f6d Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 22 Jul 2026 15:38:56 +0100 Subject: [PATCH 7/7] test(e2e): compare help popover stays within the mobile viewport Co-Authored-By: Claude Opus 4.8 --- e2e/tests/journeys.spec.ts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index f290eae..4ac1f72 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -478,3 +478,36 @@ test('rankings stay populated after picking a specific year', async ({ page }) = await expect(rows.first()).toBeVisible({ timeout: 15_000 }); expect(await rows.count()).toBeGreaterThan(5); }); + +test('compare metric-help popover stays within the mobile viewport', async ({ page }) => { + await page.setViewportSize({ width: 390, height: 844 }); + const [urn0, urn1] = await twoPrimaryUrns(page); + await page.goto(`/compare?urns=${urn0},${urn1}`); + + await expect( + page.getByRole('heading', { name: 'At a glance' }), + ).toBeVisible({ timeout: 15_000 }); + + // The metric-help triggers are the circled-"?" buttons in the row labels. + // "More information" is InfoPopover's default accessible name. + const help = page.getByRole('button', { name: 'More information' }).first(); + await expect(help).toBeVisible({ timeout: 15_000 }); + await help.click(); + + const tip = page.getByRole('tooltip'); + await expect(tip).toBeVisible(); + + // The whole bubble must sit inside the viewport — the original bug pushed it + // off the right edge with no way to scroll to it. + const box = await tip.boundingBox(); + const width = page.viewportSize()!.width; + expect(box).not.toBeNull(); + expect(box!.x).toBeGreaterThanOrEqual(0); + expect(box!.x + box!.width).toBeLessThanOrEqual(width); + + // And the page must not have gained a horizontal scrollbar from the bubble. + const bodyOverflowsX = await page + .locator('body') + .evaluate((el) => el.scrollWidth > el.clientWidth + 1); + expect(bodyOverflowsX).toBe(false); +});