From 702408d6a4d0482d3f758d2250bb3448321b4ff0 Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 22 Jul 2026 15:34:38 +0100 Subject: [PATCH] docs(compare): implementation plan for InfoPopover metric help Co-Authored-By: Claude Opus 4.8 --- .../plans/2026-07-22-info-popover.md | 739 ++++++++++++++++++ 1 file changed, 739 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-22-info-popover.md diff --git a/docs/superpowers/plans/2026-07-22-info-popover.md b/docs/superpowers/plans/2026-07-22-info-popover.md new file mode 100644 index 0000000..1706b28 --- /dev/null +++ b/docs/superpowers/plans/2026-07-22-info-popover.md @@ -0,0 +1,739 @@ +# Info Popover Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the two broken metric-help affordances (the off-viewport `MetricTooltip` bubble and the native-`title` compare `?` help) with one `@floating-ui/react`-backed `InfoPopover` that stays on-screen in any viewport and shows a custom (non-native) tooltip on desktop. + +**Architecture:** One shared engine (`InfoPopover`) owns all Floating UI positioning and interactions. Two existing components become thin adapters over it: `MetricTooltip` keeps its public API (so ~24 detail-page call sites are untouched), and `RowLabel` in `sectionShared.tsx` swaps its native-`title` span for `InfoPopover` (so ~14 compare `tip=` call sites are untouched). + +**Tech Stack:** Next.js 16, React 19, TypeScript, `@floating-ui/react`, CSS Modules, Jest + jsdom + Testing Library, Playwright (e2e). + +## Global Constraints + +- All work happens in `nextjs-app/` unless a path says otherwise. Commands below assume the working directory is `nextjs-app/`. +- Path alias: `@/` → `nextjs-app/` root (jest `moduleNameMapper` and tsconfig). +- Jest test locations: files under `__tests__/**` or named `*.test.tsx`. Env is `jest-environment-jsdom`. +- Trigger glyph is the circled **?** everywhere (replaces ⓘ on detail pages). +- Trigger is 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. +```