diff --git a/frontend/.storybook/preview.tsx b/frontend/.storybook/preview.tsx index 173ba6e2..cef9b679 100644 --- a/frontend/.storybook/preview.tsx +++ b/frontend/.storybook/preview.tsx @@ -20,12 +20,11 @@ const preview: Preview = { ), - // Global (rather than living on Tooltip.stories.tsx's meta.decorators) - // so it also covers the autodocs-generated Docs page, which renders the - // component through its own preview pathway and doesn't reliably pick up - // meta-level decorators - without this, Tooltip's Radix primitives throw - // "must be used within TooltipProvider" there even though every actual - // story works fine. + // TooltipProvider is a no-op passthrough since #583 (Tamagui's Tooltip + // needs no ambient ancestor), kept here purely so Tooltip.stories.tsx + // and its autodocs-generated Docs page - which renders the component + // through its own preview pathway and doesn't reliably pick up + // meta-level decorators - don't need their own wrapping. (Story) => ( diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2dc44083..fc9c68b7 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -13,7 +13,6 @@ "@radix-ui/react-dropdown-menu": "^2.1.19", "@radix-ui/react-popover": "^1.1.18", "@radix-ui/react-tabs": "^1.1.16", - "@radix-ui/react-tooltip": "^1.2.8", "@tamagui/config": "^2.7.6", "@tamagui/core": "^2.7.6", "@tamagui/vite-plugin": "^2.7.6", @@ -2670,41 +2669,6 @@ } } }, - "node_modules/@radix-ui/react-tooltip": { - "version": "1.2.16", - "resolved": "https://registry.npmjs.org/@radix-ui/react-tooltip/-/react-tooltip-1.2.16.tgz", - "integrity": "sha512-6EamKFRRnlpdadndbZ6LMwycfwkwPte1B42hs6QA0gYhjaOKqW4PZ4pjaW9UrlDX5eVt/OjncE7BFTPL5nmZhg==", - "license": "MIT", - "dependencies": { - "@radix-ui/primitive": "1.1.7", - "@radix-ui/react-compose-refs": "1.1.5", - "@radix-ui/react-context": "1.2.2", - "@radix-ui/react-dismissable-layer": "1.1.19", - "@radix-ui/react-id": "1.1.4", - "@radix-ui/react-popper": "1.3.7", - "@radix-ui/react-portal": "1.1.17", - "@radix-ui/react-presence": "1.1.10", - "@radix-ui/react-primitive": "2.1.10", - "@radix-ui/react-slot": "1.3.3", - "@radix-ui/react-use-controllable-state": "1.2.6", - "@radix-ui/react-use-layout-effect": "1.1.4", - "@radix-ui/react-visually-hidden": "1.2.11" - }, - "peerDependencies": { - "@types/react": "*", - "@types/react-dom": "*", - "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", - "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" - }, - "peerDependenciesMeta": { - "@types/react": { - "optional": true - }, - "@types/react-dom": { - "optional": true - } - } - }, "node_modules/@radix-ui/react-use-callback-ref": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@radix-ui/react-use-callback-ref/-/react-use-callback-ref-1.1.4.tgz", @@ -2824,29 +2788,6 @@ } } }, - "node_modules/@radix-ui/react-visually-hidden": { - "version": "1.2.11", - "resolved": "https://registry.npmjs.org/@radix-ui/react-visually-hidden/-/react-visually-hidden-1.2.11.tgz", - "integrity": "sha512-NFS86RYYZb4/exihaESBGOpMJFz8MGLAfu3mOBSGByVnVPC9JPASfYubxd/8KbkQK0sYAv8lVQDEQukDX/qXvQ==", - "license": "MIT", - "dependencies": { - "@radix-ui/react-primitive": "2.1.10" - }, - "peerDependencies": { - "@types/react": "*", - "@types/react-dom": "*", - "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", - "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" - }, - "peerDependenciesMeta": { - "@types/react": { - "optional": true - }, - "@types/react-dom": { - "optional": true - } - } - }, "node_modules/@radix-ui/rect": { "version": "1.1.3", "resolved": "https://registry.npmjs.org/@radix-ui/rect/-/rect-1.1.3.tgz", diff --git a/frontend/package.json b/frontend/package.json index 80c580fa..4023f6a3 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -28,7 +28,6 @@ "@radix-ui/react-dropdown-menu": "^2.1.19", "@radix-ui/react-popover": "^1.1.18", "@radix-ui/react-tabs": "^1.1.16", - "@radix-ui/react-tooltip": "^1.2.8", "@tamagui/config": "^2.7.6", "@tamagui/core": "^2.7.6", "@tamagui/vite-plugin": "^2.7.6", diff --git a/frontend/src/components/Tooltip/Tooltip.module.scss b/frontend/src/components/Tooltip/Tooltip.module.scss index 47875489..e5115446 100644 --- a/frontend/src/components/Tooltip/Tooltip.module.scss +++ b/frontend/src/components/Tooltip/Tooltip.module.scss @@ -31,30 +31,37 @@ box-shadow: 0 12px 28px rgba(0, 0, 0, 0.18); font-size: 0.875rem; line-height: 1.4; - transform-origin: var(--radix-tooltip-content-transform-origin); + // Tamagui sets transform-origin inline itself (computed from the actual + // flip/shift middleware result), unlike Radix's CSS-var handoff. animation-duration: v.$duration-fast; animation-timing-function: v.$easing-standard; animation-fill-mode: both; - &[data-side='top'] { + // Tamagui's `data-placement` is the full placement ("top", "top-start", + // "bottom-end", ...), not just the side Radix's `data-side` gave us - + // prefix-match to key the slide direction off the side alone. + &[data-placement^='top'] { animation-name: tooltip-slide-up; } - &[data-side='bottom'] { + &[data-placement^='bottom'] { animation-name: tooltip-slide-down; } - &[data-side='left'] { + &[data-placement^='left'] { animation-name: tooltip-slide-left; } - &[data-side='right'] { + &[data-placement^='right'] { animation-name: tooltip-slide-right; } } .arrow { - fill: rgba(c.$color-bg-invert, 0.96); + // Tamagui's Arrow is a rotated box (border/background), not an SVG + // polygon like Radix's - background-color is what actually paints it. + background-color: rgba(c.$color-bg-invert, 0.96); + border-width: 0; } @keyframes tooltip-slide-up { diff --git a/frontend/src/components/Tooltip/Tooltip.stories.tsx b/frontend/src/components/Tooltip/Tooltip.stories.tsx index da8c07a6..a044e201 100644 --- a/frontend/src/components/Tooltip/Tooltip.stories.tsx +++ b/frontend/src/components/Tooltip/Tooltip.stories.tsx @@ -5,10 +5,10 @@ import Button from '../Button/Button'; /** * `Tooltip` wraps a single focusable trigger element and shows supplementary - * content on click/tap or focus via Radix — hover never opens it, on either - * desktop or mobile (see #568). Clicking/tapping the trigger again, clicking - * anywhere else, or pressing Escape closes it. It must be nested under a - * `TooltipProvider` (mounted once near the app root, and globally in + * content on click/tap or focus via Tamagui — hover never opens it, on + * either desktop or mobile (see #568). Clicking/tapping the trigger again, + * clicking anywhere else, or pressing Escape closes it. It must be nested + * under a `TamaguiProvider` (mounted once near the app root, and globally in * .storybook/preview.tsx). Use tooltips only for supplementary context - * essential information must remain available without clicking or focusing. */ @@ -35,7 +35,7 @@ export const Default: Story = { const trigger = canvas.getByRole('button', { name: 'Click or focus me' }); await userEvent.tab(); await expect(trigger).toHaveFocus(); - // Tooltip content renders via a Radix Portal into document.body. + // Tooltip content renders via a Tamagui Portal into document.body. const tooltip = await within(canvasElement.ownerDocument.body).findByRole('tooltip'); await expect(tooltip).toHaveTextContent('Additional context shown on click/tap or focus'); }, diff --git a/frontend/src/components/Tooltip/Tooltip.test.tsx b/frontend/src/components/Tooltip/Tooltip.test.tsx index fb88aad5..5f8645f4 100644 --- a/frontend/src/components/Tooltip/Tooltip.test.tsx +++ b/frontend/src/components/Tooltip/Tooltip.test.tsx @@ -1,7 +1,24 @@ import { describe, it, expect } from 'vitest'; -import { render, screen, waitFor } from '@testing-library/react'; +import { render as rtlRender, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { TamaguiProvider } from 'tamagui'; import Tooltip, { TooltipProvider } from './Tooltip'; +import tamaguiConfig from '../../../tamagui.config'; + +// Tamagui's Tooltip (#583) needs a TamaguiProvider ancestor - unlike Radix's +// Tooltip.Root, it isn't usable standalone. The app root (src/main.tsx) +// provides this in production; tests need their own. +function render(...args: Parameters) { + const [ui, options] = args; + return rtlRender(ui, { + wrapper: ({ children }) => ( + + {children} + + ), + ...options, + }); +} function renderTooltip() { render( diff --git a/frontend/src/components/Tooltip/Tooltip.tsx b/frontend/src/components/Tooltip/Tooltip.tsx index 40e1715e..c899dcff 100644 --- a/frontend/src/components/Tooltip/Tooltip.tsx +++ b/frontend/src/components/Tooltip/Tooltip.tsx @@ -1,6 +1,6 @@ import React from 'react'; import classNames from 'classnames'; -import * as TooltipPrimitive from '@radix-ui/react-tooltip'; +import { Tooltip as TooltipPrimitive, type TamaguiElement } from 'tamagui'; import styles from './Tooltip.module.scss'; type TooltipPlacement = 'top' | 'bottom' | 'left' | 'right'; @@ -23,21 +23,72 @@ interface TooltipProps { disabled?: boolean; } -export function TooltipProvider({ - children, - delayDuration = 250, - skipDelayDuration = 100, - disableHoverableContent = true, -}: TooltipProviderProps): React.ReactElement { - return ( - - {children} - - ); +/** + * Radix's `Provider` grouped sibling tooltips under a shared hover-delay + * timer. Tamagui's `Tooltip` needs no such ambient ancestor - each one is + * self-contained - and #568 already made hover a no-op for opening (see + * below), so there's no delay behaviour left to group. Kept as a passthrough, + * accepting the same props, purely so call sites and tests don't need to + * change (#582-style API-compatibility). + */ +export function TooltipProvider({ children }: TooltipProviderProps): React.ReactElement { + return <>{children}; +} + +function toTamaguiPlacement(placement: TooltipPlacement, align: TooltipAlign) { + return align === 'center' ? placement : (`${placement}-${align}` as const); +} + +// The DOM events React derives its synthetic onMouseEnter/onMouseLeave/ +// onMouseMove from. React delegates a *single* listener per type at the +// app root rather than attaching one per element, and for the two +// non-bubbling ones (mouseenter/mouseleave) it doesn't listen for those +// directly at all - it listens for the bubbling mouseover/mouseout and +// derives enter/leave transitions itself. Pointer variants are included +// since React treats them as an equivalent source on pointer-capable +// browsers. +const HOVER_SOURCE_EVENT_TYPES = [ + 'mouseover', + 'mouseout', + 'mousemove', + 'pointerover', + 'pointerout', + 'pointermove', +]; + +/** + * Tamagui's Tooltip hardcodes `hoverable: true` in its floating-ui + * interaction context, with no prop to turn it off - `useHover` (from + * `@tamagui/floating`) exposes an `onMouseMove` *React prop* via + * `getReferenceProps()`, which `asChild` merges onto the trigger's real + * element (winning over any same-named prop we pass, the same "parent + * wins" merge #582 hit with Dialog.Title's aria-label). That prop is + * dispatched through React's own delegated root listener, not a listener + * on the trigger itself, so a same-named prop can't pre-empt it. React + * doesn't listen for 'mouseenter'/'mouseleave' directly either (they don't + * bubble) - it derives its synthetic enter/leave from the bubbling + * 'mouseover'/'mouseout' instead. Capturing those (plus 'mousemove' and the + * pointer equivalents, since React treats pointer events as an equivalent + * hover source) at `document` - an ancestor of React's root container - + * pre-empts all of it: capture-phase listeners on an ancestor always run + * before React's own listener on the root container ever sees the event. + * Verified manually against a real browser (this interaction falls outside + * what `npm run test:storybook`/`test:a11y` can run in this sandbox - see + * #583's PR description). + */ +function useSuppressHoverOpen(triggerRef: React.RefObject) { + React.useEffect(() => { + const isInTrigger = (event: Event) => + event.target instanceof Node && + ((triggerRef.current as HTMLElement | null)?.contains(event.target) ?? false); + const stop = (event: Event) => { + if (isInTrigger(event)) event.stopPropagation(); + }; + for (const type of HOVER_SOURCE_EVENT_TYPES) document.addEventListener(type, stop, true); + return () => { + for (const type of HOVER_SOURCE_EVENT_TYPES) document.removeEventListener(type, stop, true); + }; + }, [triggerRef]); } /** @@ -59,25 +110,48 @@ export default function Tooltip({ disabled = false, }: TooltipProps): React.ReactElement { const [open, setOpen] = React.useState(false); + const triggerRef = React.useRef(null); + const contentRef = React.useRef(null); // Tracks whether the trigger's current focus came from our own - // pointerdown handler below, so that handler and handleFocus don't both - // try to decide the open state for the same interaction. + // pointerdown handler below, so that handler and handleFocusCapture don't + // both try to decide the open state for the same interaction. const pointerDownRef = React.useRef(false); + useSuppressHoverOpen(triggerRef); + + // Closing over outside-click and Escape ourselves, driven by real DOM + // listeners, is more reliable here than leaning on Tamagui's own dismiss + // wiring (untested/unclear for Tooltip content, unlike the Dismissable + // layer #582 could rely on for Modal/AlertDialog) - and #568 wants a click + // anywhere else, including elements with no tooltip of their own, to + // close it, which is exactly what a document-level listener gives for + // free. + React.useEffect(() => { + if (!open) return; + const handlePointerDownOutside = (event: PointerEvent) => { + const target = event.target as Node | null; + if (!target) return; + if ((triggerRef.current as HTMLElement | null)?.contains(target)) return; + if ((contentRef.current as HTMLElement | null)?.contains(target)) return; + setOpen(false); + }; + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') setOpen(false); + }; + document.addEventListener('pointerdown', handlePointerDownOutside, true); + document.addEventListener('keydown', handleKeyDown); + return () => { + document.removeEventListener('pointerdown', handlePointerDownOutside, true); + document.removeEventListener('keydown', handleKeyDown); + }; + }, [open]); + if (disabled || content === null || content === undefined || content === false) { return <>{children}; } - // Radix's Trigger opens on hover and unconditionally closes on click, - // which fights a click/tap-to-toggle model on both counts. We take the - // trigger's open/close decisions over entirely: hover is inert (handled - // below), and pointerdown is the sole place we ever *open* it - never - // close, because a pointerdown on an already-open trigger is also seen by - // the open tooltip's own dismissable layer as an "outside" press (the - // trigger isn't part of the portaled content), which closes it for us. - // Deciding to close here too would race that, since our own pointerdown - // handler runs in the bubble phase, after the layer's capture-phase - // dismissal has already fired. + // The sole place we ever *open* the tooltip via pointer interaction - a + // pointerdown on an already-open trigger toggles it closed directly. const handlePointerDown = (event: React.PointerEvent) => { event.preventDefault(); pointerDownRef.current = true; @@ -88,54 +162,57 @@ export default function Tooltip({ }, { once: true } ); - if (!open) setOpen(true); + setOpen((wasOpen) => !wasOpen); }; // preventDefault() on pointerdown doesn't stop the resulting click event // for mouse pointers (only for touch, where it suppresses the - // compatibility click outright) - so on desktop, Radix's own "click - // always closes" trigger handler would still fire right after the - // pointerdown above opens it. Taking the click event over too stops that. + // compatibility click outright), so left unhandled this would otherwise + // reach any onClick a consumer's own child sets. const handleClick = (event: React.MouseEvent) => { event.preventDefault(); }; - // Suppress Radix's hover-driven open/close entirely - hover is cursor - // affordance only now, never a way to open or close the tooltip. - const suppressHover = (event: React.PointerEvent) => { - event.preventDefault(); - }; - - const handleFocus = (event: React.FocusEvent) => { - event.preventDefault(); + // Capture-phase - unlike onMouseEnter/onMouseLeave above, React (and thus + // the DOM node Trigger's `asChild` clones onto) does accept an + // onFocusCapture prop, even though Tamagui's own Trigger prop types don't + // list it, so this one can stay a normal prop rather than needing a raw + // DOM listener; running before Tamagui's own (broken - see + // #583/tamagui/tamagui#4152) bubble-phase onFocus keeps our decision + // authoritative either way. + const handleFocusCapture = (event: React.FocusEvent) => { + event.stopPropagation(); if (!pointerDownRef.current) setOpen(true); }; + const focusCaptureProp = { onFocusCapture: handleFocusCapture }; return ( - + {children} - - - {content} - - - - + + {content} + + + ); } diff --git a/frontend/src/layout/Infobar/AchievementBadges.test.tsx b/frontend/src/layout/Infobar/AchievementBadges.test.tsx index 815c93cc..0e7b3a79 100644 --- a/frontend/src/layout/Infobar/AchievementBadges.test.tsx +++ b/frontend/src/layout/Infobar/AchievementBadges.test.tsx @@ -1,9 +1,26 @@ import { describe, expect, it } from "vitest"; -import { render, screen } from "@testing-library/react"; +import { render as rtlRender, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { TamaguiProvider } from "tamagui"; import { TooltipProvider } from "../../components/Tooltip/Tooltip"; import AchievementBadges from "./AchievementBadges"; +import tamaguiConfig from "../../../tamagui.config"; + +// AchievementBadges renders Tooltip (#583), which needs a TamaguiProvider +// ancestor - unlike Radix's Tooltip.Root, it isn't usable standalone. The +// app root (src/main.tsx) provides this in production; tests need their own. +function render(...args: Parameters) { + const [ui, options] = args; + return rtlRender(ui, { + wrapper: ({ children }) => ( + + {children} + + ), + ...options, + }); +} const achievements = [ { @@ -40,8 +57,8 @@ function renderBadges(props: Partial { it("renders nothing when there are no achievements", () => { - const { container } = renderBadges({ achievements: [] }); - expect(container).toBeEmptyDOMElement(); + renderBadges({ achievements: [] }); + expect(screen.queryByRole("button")).not.toBeInTheDocument(); }); it("renders an accessible badge per achievement with an aria-label summarizing state", () => {