Skip to content

feat: replace Radix Tooltip with Tamagui (#583) - #791

Draft
gaidheal1 wants to merge 1 commit into
claude/toast-tamagui-issue-582-zgkzlffrom
claude/tooltip-tamagui-issue-583-zgkzlf
Draft

feat: replace Radix Tooltip with Tamagui (#583)#791
gaidheal1 wants to merge 1 commit into
claude/toast-tamagui-issue-582-zgkzlffrom
claude/tooltip-tamagui-issue-583-zgkzlf

Conversation

@gaidheal1

Copy link
Copy Markdown
Member

Summary

Implements #583: replaces @radix-ui/react-tooltip in frontend/src/components/Tooltip/Tooltip.tsx with Tamagui's Tooltip primitive, part of epic #578. Branched off #582's Tamagui Modal/AlertDialog branch per the user's explicit instruction.

Since #568 (unify tap/click tooltip behaviour) already merged to development before this branch was cut, the existing Tooltip.tsx was already fully self-controlled: it owns open state directly and decides opens/closes itself via explicit pointerdown/focus handlers, never relying on the underlying primitive's own hover/focus logic. That existing architecture turned out to be exactly what was needed to route around Tamagui's confirmed upstream focus-to-open bug (tamagui/tamagui#4152, documented in .claude/plans/issue-629-tamagui-poc.md): since we never rely on Tamagui's built-in focus={{enabled:true}} wiring at all (our own onFocusCapture opens it directly), that bug simply doesn't apply here — no workaround needed, it's moot by construction.

Key implementation notes

  • Hover suppression required more than blocking mouseenter/mouseleave. Tamagui's Tooltip hardcodes hoverable: true with no prop to disable it, and exposes hover-open via a getReferenceProps()-supplied onMouseMove React prop that asChild merges onto the trigger (winning over any same-named prop we pass — the same "parent wins" merge Replace Radix Dialog and AlertDialog with a shared overlay primitive #582 hit with Dialog.Title's aria-label). That prop is dispatched through React's own delegated root listener, and React derives its synthetic onMouseEnter/onMouseLeave from the bubbling mouseover/mouseout (they don't listen for the non-bubbling names directly). The fix: a document-level capture-phase listener for mouseover/mouseout/mousemove (+ pointer equivalents) — an ancestor's capture-phase listener always runs before React's own listener on its root container ever sees the event. Verified manually in a real Chromium browser (Playwright, pinned executable) since this exact interaction falls outside what test:storybook/test:a11y can run in this sandbox (see below).
  • Outside-click and Escape are handled with the app's own document listeners, not Tamagui's built-in dismiss wiring — same reasoning as Replace Radix Dialog and AlertDialog with a shared overlay primitive #582's choice to use plain Buttons over Dialog.Close/AlertDialog.Cancel: more reliable than leaning on internal composition that's untested for this specific usage.
  • Map.tsx no longer uses the shared Tooltip/TooltipProvider — it grew its own custom MapLibre-driven overlay (role="tooltip" div + its own TamaguiProvider) since Replace Radix Tooltip — coordinate with the tap/click unification in #568 #583 was filed, so the issue's "verify Map.tsx's zero-delay tooltips" acceptance item doesn't apply; confirmed via code read, not assumed.
  • TooltipProvider is now a passthrough (Tamagui needs no ambient ancestor) but keeps its exact prop signature for call-site/test compatibility, consistent with Replace Radix Dialog and AlertDialog with a shared overlay primitive #582's approach for Modal/AlertDialog.
  • data-sidedata-placement (prefix-matched) in the SCSS; Tamagui sets transform-origin inline itself; the arrow is a rotated box, not an SVG polygon, so fillbackground-color.

Acceptance criteria

  • No @radix-ui/react-tooltip import remains (uninstalled from package.json)
  • Public props of Tooltip/TooltipProvider unchanged; all consumers (TasksPanel, ActivitiesPanel, ActivityTimeline, AchievementBadges) work untouched
  • Per-subtree provider overrides — N/A, Map.tsx no longer uses the shared component (see above)
  • Placement/collision-avoidance preserved — verified via real-browser check (all 4 placements render in-viewport) + allowFlip/stayInFrame
  • disabled/empty content still short-circuits to bare children
  • Unify tap/click tooltip behaviour #568 criteria — already met on development before this branch; click/tap-to-toggle preserved and tested
  • Trigger stays focusable with aria-describedby wiring — confirmed via test + real browser
  • Tooltip.test.tsx, TasksPanel.test.tsx, Map.test.tsx pass
  • Verified on a real touch device — not available in this sandbox; touch-action/-webkit-touch-callout CSS carried over unchanged from the pre-Replace Radix Tooltip — coordinate with the tap/click unification in #568 #583 implementation, but real hardware verification is still open

Test plan

  • npm run test — 566/567 passing (1 pre-existing, unrelated flake in UnifiedTimerHome.test.tsx, confirmed identical on the base branch and untouched by this change — doesn't render Tooltip)
  • npm run lint / tsc --noEmit — clean
  • npm run build:production — succeeds
  • Manual verification against a real Chromium browser (Playwright, pinned executable, via Storybook) covering: hover doesn't open, click opens/closes (toggle), click elsewhere closes, Tab focuses + opens, Escape closes, all 4 placements render in-viewport, disabled story doesn't open
  • npm run test:storybook / npm run test:a11y — blocked by a pre-existing Playwright browser version mismatch in this sandbox (chromium_headless_shell-1234 expected vs -1194 available), same limitation documented in Replace Radix Dialog and AlertDialog with a shared overlay primitive #582's PR
  • Real touch device verification — not available in this sandbox

Generated by Claude Code

Preserves the click/tap-to-toggle model #568 already established (open
state fully app-controlled, not driven by Tamagui's built-in interaction
system) - the trigger's own onFocusCapture opens the tooltip directly,
which sidesteps the confirmed upstream focus-to-open bug in
tamagui/tamagui#4152 entirely rather than needing a workaround for it.

Hover suppression required capturing the bubbling mouseover/mouseout/
mousemove (and pointer equivalents) events React derives its synthetic
onMouseEnter/onMouseLeave/onMouseMove from at the document level, since
Tamagui's Tooltip hardcodes hoverable interaction with no prop to disable
it and asChild always lets Tamagui's own same-named props win. Outside-
click and Escape dismissal are handled with the app's own document
listeners for the same reason, rather than relying on Tamagui's dismiss
wiring (unclear/untested for Tooltip content).

Map.tsx no longer uses the shared Tooltip/TooltipProvider (it grew its
own custom overlay since #583 was filed), so the per-subtree provider
override this issue called out doesn't apply.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants