Mobile: tapping a metric's help icon showed a definition bubble that rendered partly/wholly off-viewport with no way to scroll to it — unusable. Root cause: MetricTooltip's bubble was position: absolute, fixed 220px, with no viewport-collision detection (a left: -12px hack only moved the problem).
Desktop: the compare-section ? help used the native title= attribute, so it took the browser's long, unconfigurable hover delay to appear.
The fix
One shared, viewport-aware component — InfoPopover — built on @floating-ui/react, replacing both affordances:
Positioning:offset + flip + shift({ padding: 8 }) keep the bubble fully on-screen in any viewport (fixes the mobile bug); rendered in a FloatingPortal so it escapes the compare grid's overflow/transform clipping; an arrow tracks the trigger.
Interactions (one code path): hover with a 100ms open delay + keyboard focus on desktop (fixes the slow native-title hover), tap on touch, outside-press + Escape to dismiss, proper ARIA. Floating UI disables hover on touch so tap/hover never double-fire.
Glyph standardised on the circled ? everywhere (detail pages previously used ⓘ).
Thin adapters — no call-site churn
MetricTooltip keeps its public API and just delegates to InfoPopover — all ~24 detail-page call sites untouched. Its hand-rolled open/dismiss useEffect and absolute-position CSS are deleted.
RowLabel (compare sections) swaps its native-title span for InfoPopover — all ~14 compare tip= call sites untouched. Dead .help CSS removed.
Tests
Unit (jest):InfoPopover (6), MetricTooltip (3), RowLabel (3) — open/close, ARIA, content, Escape, null-render, metricKey resolution + override. Full suite 124/124 green, typecheck clean, next build succeeds.
E2E (Playwright, 390px viewport): the regression guard — opens a compare help popover and asserts its bounding box sits fully within the viewport and adds no horizontal scroll. This is the check that would have caught the original bug. Runs in the staging gate.
Out of scope (flagged for your call)
DotStrip still uses a native title= hover hint on its chart label/dots (not a ?/ⓘ help affordance). The approved spec scoped native-title hints out, so I left it. Happy to migrate it too if you'd like consistency.
Dependency
Adds @floating-ui/react (~10KB gzipped), as agreed.
## The bugs
1. **Mobile:** tapping a metric's help icon showed a definition bubble that rendered **partly/wholly off-viewport with no way to scroll to it** — unusable. Root cause: `MetricTooltip`'s bubble was `position: absolute`, fixed 220px, with no viewport-collision detection (a `left: -12px` hack only moved the problem).
2. **Desktop:** the compare-section `?` help used the native `title=` attribute, so it took the browser's long, unconfigurable hover delay to appear.
## The fix
One shared, viewport-aware component — **`InfoPopover`** — built on `@floating-ui/react`, replacing both affordances:
- **Positioning:** `offset` + `flip` + `shift({ padding: 8 })` keep the bubble fully on-screen in any viewport (fixes the mobile bug); rendered in a `FloatingPortal` so it escapes the compare grid's overflow/transform clipping; an arrow tracks the trigger.
- **Interactions (one code path):** hover with a **100ms open delay** + keyboard focus on desktop (fixes the slow native-`title` hover), tap on touch, outside-press + Escape to dismiss, proper ARIA. Floating UI disables hover on touch so tap/hover never double-fire.
- **Glyph standardised** on the circled **?** everywhere (detail pages previously used ⓘ).
### Thin adapters — no call-site churn
- `MetricTooltip` keeps its public API and just delegates to `InfoPopover` — all ~24 detail-page call sites untouched. Its hand-rolled open/dismiss `useEffect` and absolute-position CSS are deleted.
- `RowLabel` (compare sections) swaps its native-`title` span for `InfoPopover` — all ~14 compare `tip=` call sites untouched. Dead `.help` CSS removed.
## Tests
- **Unit (jest):** `InfoPopover` (6), `MetricTooltip` (3), `RowLabel` (3) — open/close, ARIA, content, Escape, null-render, metricKey resolution + override. Full suite **124/124 green**, typecheck clean, `next build` succeeds.
- **E2E (Playwright, 390px viewport):** the regression guard — opens a compare help popover and asserts its bounding box sits fully within the viewport and adds no horizontal scroll. This is the check that would have caught the original bug. Runs in the staging gate.
## Out of scope (flagged for your call)
`DotStrip` still uses a native `title=` hover hint on its chart label/dots (not a `?`/ⓘ help affordance). The approved spec scoped native-`title` hints out, so I left it. Happy to migrate it too if you'd like consistency.
## Dependency
Adds `@floating-ui/react` (~10KB gzipped), as agreed.
Design + plan: `docs/superpowers/specs/2026-07-22-info-popover-design.md`, `docs/superpowers/plans/2026-07-22-info-popover.md`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
This PR replaces the off-viewport MetricTooltip bubble and native-title compare help with a single Floating-UI-backed InfoPopover, with adapters preserving existing call-site APIs and new unit/e2e tests. The refactor is clean, well-tested, and touches no backend, pipeline, or deploy config, so it poses no production/data risk.
🟡 Minor
nextjs-app/components/compare/sectionShared.tsx: RowLabel no longer passes a distinguishing ariaLabel to InfoPopover (previously aria-label was the tip text itself), so every compare-section help button now shares the identical accessible name 'More information'. Screen-reader users can no longer tell which metric a given help button explains.
e2e/tests/journeys.spec.ts: The new test grabs the first 'More information' button rather than one positioned near a viewport edge, so it doesn't actually reproduce the original off-viewport bug scenario described in the design doc; it exercises the general flip/shift behavior but is weaker regression coverage than intended.
## 🤖 AI Code Review (Claude Code)
This PR replaces the off-viewport MetricTooltip bubble and native-title compare help with a single Floating-UI-backed InfoPopover, with adapters preserving existing call-site APIs and new unit/e2e tests. The refactor is clean, well-tested, and touches no backend, pipeline, or deploy config, so it poses no production/data risk.
### 🟡 Minor
- **nextjs-app/components/compare/sectionShared.tsx**: RowLabel no longer passes a distinguishing ariaLabel to InfoPopover (previously aria-label was the tip text itself), so every compare-section help button now shares the identical accessible name 'More information'. Screen-reader users can no longer tell which metric a given help button explains.
- **e2e/tests/journeys.spec.ts**: The new test grabs the first 'More information' button rather than one positioned near a viewport edge, so it doesn't actually reproduce the original off-viewport bug scenario described in the design doc; it exercises the general flip/shift behavior but is weaker regression coverage than intended.
tudor
merged commit 59b5f3f163 into main2026-07-22 14:48:13 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The bugs
MetricTooltip's bubble wasposition: absolute, fixed 220px, with no viewport-collision detection (aleft: -12pxhack only moved the problem).?help used the nativetitle=attribute, so it took the browser's long, unconfigurable hover delay to appear.The fix
One shared, viewport-aware component —
InfoPopover— built on@floating-ui/react, replacing both affordances:offset+flip+shift({ padding: 8 })keep the bubble fully on-screen in any viewport (fixes the mobile bug); rendered in aFloatingPortalso it escapes the compare grid's overflow/transform clipping; an arrow tracks the trigger.titlehover), tap on touch, outside-press + Escape to dismiss, proper ARIA. Floating UI disables hover on touch so tap/hover never double-fire.Thin adapters — no call-site churn
MetricTooltipkeeps its public API and just delegates toInfoPopover— all ~24 detail-page call sites untouched. Its hand-rolled open/dismissuseEffectand absolute-position CSS are deleted.RowLabel(compare sections) swaps its native-titlespan forInfoPopover— all ~14 comparetip=call sites untouched. Dead.helpCSS removed.Tests
InfoPopover(6),MetricTooltip(3),RowLabel(3) — open/close, ARIA, content, Escape, null-render, metricKey resolution + override. Full suite 124/124 green, typecheck clean,next buildsucceeds.Out of scope (flagged for your call)
DotStripstill uses a nativetitle=hover hint on its chart label/dots (not a?/ⓘ help affordance). The approved spec scoped native-titlehints out, so I left it. Happy to migrate it too if you'd like consistency.Dependency
Adds
@floating-ui/react(~10KB gzipped), as agreed.Design + plan:
docs/superpowers/specs/2026-07-22-info-popover-design.md,docs/superpowers/plans/2026-07-22-info-popover.md.🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
This PR replaces the off-viewport MetricTooltip bubble and native-title compare help with a single Floating-UI-backed InfoPopover, with adapters preserving existing call-site APIs and new unit/e2e tests. The refactor is clean, well-tested, and touches no backend, pipeline, or deploy config, so it poses no production/data risk.
🟡 Minor