feat(ui): viewport-aware InfoPopover replaces broken metric tooltips #81
@@ -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 `<button>` with a min 24px tap target (WCAG 2.5.8) and an accessible name.
|
||||
- `InfoPopover` renders `null` when it has no `plain` content (preserves current `MetricTooltip` behaviour).
|
||||
- Positioning/interaction is delegated to Floating UI — do not hand-roll positioning, outside-click, or Escape handling.
|
||||
- Run `npx tsc --noEmit` and `npx jest` green before every commit that changes code.
|
||||
- Never push to `main`; this plan's branch is `feat/info-popover-tooltip`.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Add the `@floating-ui/react` dependency
|
||||
|
||||
**Files:**
|
||||
- Modify: `nextjs-app/package.json` (dependencies)
|
||||
- Modify: `nextjs-app/package-lock.json` (generated)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: nothing.
|
||||
- Produces: `@floating-ui/react` importable — the hooks `useFloating`, `autoUpdate`, `offset`, `flip`, `shift`, `arrow`, `useHover`, `useFocus`, `useClick`, `useDismiss`, `useRole`, `useInteractions`, and the component `FloatingPortal`, `FloatingArrow`.
|
||||
|
||||
- [ ] **Step 1: Install the package**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
npm install @floating-ui/react@^0.27.0
|
||||
```
|
||||
Expected: `package.json` gains `"@floating-ui/react": "^0.27.0"` under `dependencies`; `package-lock.json` updates; exit 0.
|
||||
|
||||
- [ ] **Step 2: Verify it resolves**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
node -e "const f=require('@floating-ui/react'); console.log(typeof f.useFloating, typeof f.FloatingPortal, typeof f.useInteractions)"
|
||||
```
|
||||
Expected: `function function function`
|
||||
|
||||
- [ ] **Step 3: Typecheck still passes**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
npx tsc --noEmit
|
||||
```
|
||||
Expected: no output (exit 0).
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add package.json package-lock.json
|
||||
git commit -m "build(deps): add @floating-ui/react for the info popover"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Build `InfoPopover` (the shared engine) with unit tests
|
||||
|
||||
**Files:**
|
||||
- Create: `nextjs-app/components/InfoPopover.tsx`
|
||||
- Create: `nextjs-app/components/InfoPopover.module.css`
|
||||
- Test: `nextjs-app/__tests__/components/InfoPopover.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `@floating-ui/react` (Task 1).
|
||||
- Produces:
|
||||
```ts
|
||||
export interface InfoPopoverProps {
|
||||
label?: string; // bold heading
|
||||
plain?: string; // body text (primary explanation)
|
||||
detail?: string; // muted supplementary line
|
||||
ariaLabel?: string; // accessible name for the trigger button
|
||||
}
|
||||
export function InfoPopover(props: InfoPopoverProps): JSX.Element | null;
|
||||
```
|
||||
Behaviour: renders `null` if `plain` is falsy. Otherwise renders a `<button>`
|
||||
(the circled `?`) with `aria-label={ariaLabel ?? 'More information'}` and
|
||||
`aria-expanded`. When open, a portalled `role="tooltip"` container shows
|
||||
`label` (if any), `plain`, and `detail` (if any). Opens on hover (100ms
|
||||
open / 0ms close) and focus on desktop, on click for touch; closes on
|
||||
outside-press and Escape.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Create `nextjs-app/__tests__/components/InfoPopover.test.tsx`:
|
||||
```tsx
|
||||
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(<InfoPopover label="X" />);
|
||||
expect(container).toBeEmptyDOMElement();
|
||||
});
|
||||
|
||||
it('renders a labelled, collapsed trigger button', () => {
|
||||
render(<InfoPopover plain="what it means" ariaLabel="Reading score" />);
|
||||
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(
|
||||
<InfoPopover
|
||||
label="Reading, Writing & Maths"
|
||||
plain="% reaching the expected standard"
|
||||
detail="National average ~60%"
|
||||
ariaLabel="RWM"
|
||||
/>,
|
||||
);
|
||||
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(<InfoPopover plain="body" ariaLabel="Info" />);
|
||||
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(<InfoPopover plain="body" ariaLabel="Info" />);
|
||||
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(<InfoPopover plain="body" />);
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'More information' }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
npx jest __tests__/components/InfoPopover.test.tsx
|
||||
```
|
||||
Expected: FAIL — `Cannot find module '@/components/InfoPopover'`.
|
||||
|
||||
- [ ] **Step 3: Write the component**
|
||||
|
||||
Create `nextjs-app/components/InfoPopover.tsx`:
|
||||
```tsx
|
||||
'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<SVGSVGElement>(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 (
|
||||
<>
|
||||
<button
|
||||
type="button"
|
||||
ref={refs.setReference}
|
||||
className={styles.icon}
|
||||
aria-label={ariaLabel ?? 'More information'}
|
||||
aria-expanded={open}
|
||||
{...getReferenceProps()}
|
||||
>
|
||||
?
|
||||
</button>
|
||||
{open && (
|
||||
<FloatingPortal>
|
||||
<div
|
||||
ref={refs.setFloating}
|
||||
className={styles.tooltip}
|
||||
style={floatingStyles}
|
||||
{...getFloatingProps()}
|
||||
>
|
||||
<FloatingArrow ref={arrowRef} context={context} className={styles.arrow} />
|
||||
{label && <span className={styles.label}>{label}</span>}
|
||||
<span className={styles.plain}>{plain}</span>
|
||||
{detail && <span className={styles.detail}>{detail}</span>}
|
||||
</div>
|
||||
</FloatingPortal>
|
||||
)}
|
||||
</>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **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(<MetricTooltip metricKey={key} />);
|
||||
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(<MetricTooltip metricKey="__nope__" />);
|
||||
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(<MetricTooltip metricKey={key} plain="custom text" />);
|
||||
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 (
|
||||
<InfoPopover
|
||||
label={resolvedLabel}
|
||||
plain={plain ?? explanation?.plain}
|
||||
detail={detail ?? explanation?.detail}
|
||||
ariaLabel={resolvedLabel ? `What does ${resolvedLabel} mean?` : undefined}
|
||||
/>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **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(<RowLabel>Attainment 8</RowLabel>);
|
||||
expect(screen.getByText('Attainment 8')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows no help affordance when no tip is given', () => {
|
||||
render(<RowLabel>Attainment 8</RowLabel>);
|
||||
expect(screen.queryByRole('button')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('opens the tip in a popover on click', async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<RowLabel tip="Average GCSE score across 8 subjects">Attainment 8</RowLabel>);
|
||||
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 `<span title=...>` (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 (
|
||||
<div className={styles.rowLabel}>
|
||||
{children}
|
||||
{tip && <InfoPopover plain={tip} />}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **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.
|
||||
```
|
||||
Reference in New Issue
Block a user