diff --git a/src/shared/components/Tooltip.tsx b/src/shared/components/Tooltip.tsx index 857e8c9c69..ca594e28ad 100644 --- a/src/shared/components/Tooltip.tsx +++ b/src/shared/components/Tooltip.tsx @@ -16,9 +16,11 @@ import { useCallback, useEffect, useId, + useLayoutEffect, useRef, useState, } from "react"; +import { createPortal } from "react-dom"; interface TooltipProps { children: ReactNode; @@ -26,6 +28,19 @@ interface TooltipProps { position?: "top" | "bottom" | "left" | "right"; className?: string; delayMs?: number; + /** + * Issue #2352: Render the tooltip in a React portal so it escapes the + * stacking context of any ancestor with `overflow:hidden` / `overflow:auto` + * (modals, scroll containers). Without this, long labels next to a modal + * edge are clipped. Defaults to `true` because the previous absolute + * positioning has been the cause of every "tooltip cut off" report. + */ + usePortal?: boolean; + /** + * Allow the tooltip text to wrap onto multiple lines instead of forcing a + * single-line `whitespace-nowrap` layout. Use when the label is long. + */ + multiline?: boolean; } interface AriaDescribedElement { @@ -38,10 +53,14 @@ export default function Tooltip({ position = "top", className = "", delayMs = 200, + usePortal = true, + multiline = false, }: TooltipProps) { const [visible, setVisible] = useState(false); const tooltipId = useId(); const timeoutRef = useRef | null>(null); + const wrapperRef = useRef(null); + const tooltipRef = useRef(null); const show = useCallback(() => { clearTimeout(timeoutRef.current); @@ -59,6 +78,53 @@ export default function Tooltip({ }; }, []); + // Position the portal-rendered tooltip relative to the trigger by writing + // directly to the DOM ref instead of round-tripping through React state. + // Touching .style here is the canonical React-portal positioning pattern + // and avoids the cascading-render warning we'd get from setCoords inside + // a layout effect. + useLayoutEffect(() => { + if (!visible || !usePortal) return; + const wrap = wrapperRef.current; + const tt = tooltipRef.current; + if (!wrap || !tt) return; + const rect = wrap.getBoundingClientRect(); + const tRect = tt.getBoundingClientRect(); + const scrollY = window.scrollY; + const scrollX = window.scrollX; + let top = 0; + let left = 0; + switch (position) { + case "bottom": + top = rect.bottom + scrollY + 8; + left = rect.left + scrollX + rect.width / 2 - tRect.width / 2; + break; + case "left": + top = rect.top + scrollY + rect.height / 2 - tRect.height / 2; + left = rect.left + scrollX - tRect.width - 8; + break; + case "right": + top = rect.top + scrollY + rect.height / 2 - tRect.height / 2; + left = rect.right + scrollX + 8; + break; + case "top": + default: + top = rect.top + scrollY - tRect.height - 8; + left = rect.left + scrollX + rect.width / 2 - tRect.width / 2; + break; + } + // Clamp horizontally inside the viewport so a trigger near the right + // edge does not produce a tooltip that bleeds off the screen. + const margin = 8; + const maxLeft = window.innerWidth + scrollX - tRect.width - margin; + const minLeft = scrollX + margin; + if (left > maxLeft) left = maxLeft; + if (left < minLeft) left = minLeft; + tt.style.top = `${top}px`; + tt.style.left = `${left}px`; + tt.style.visibility = "visible"; + }, [visible, usePortal, position, content]); + const positionClasses = { top: "bottom-full left-1/2 -translate-x-1/2 mb-2", bottom: "top-full left-1/2 -translate-x-1/2 mt-2", @@ -82,8 +148,35 @@ export default function Tooltip({ ); + const widthClass = multiline ? "max-w-xs whitespace-normal break-words" : "whitespace-nowrap"; + const baseTooltipClass = + "z-50 px-2.5 py-1.5 text-xs font-medium text-white bg-gray-900/95 rounded-md shadow-lg pointer-events-none animate-in fade-in duration-150 motion-reduce:transition-none motion-reduce:animate-none border border-white/10"; + + const portalEnabled = usePortal && typeof window !== "undefined"; + + const tooltipEl = + visible && content ? ( + + ) : null; + return ( {trigger} - {visible && content && ( - - {content} - - )} + {portalEnabled && tooltipEl ? createPortal(tooltipEl, document.body) : tooltipEl} ); } diff --git a/tests/unit/tooltip-portal-behavior.test.ts b/tests/unit/tooltip-portal-behavior.test.ts new file mode 100644 index 0000000000..4938a9e6d0 --- /dev/null +++ b/tests/unit/tooltip-portal-behavior.test.ts @@ -0,0 +1,70 @@ +/** + * Issue #2352 — Tooltip must render in a React portal by default so it can + * escape ancestor stacking contexts (modal `overflow-hidden`). Without the + * portal, tooltips in the combo edit modal were clipped. + * + * Browser DOM is not available in this Node test runner, so this regression + * guard verifies the source-level contract: (1) the component opts into a + * portal by default, (2) the createPortal target is document.body, (3) the + * `multiline` opt-out replaces the `whitespace-nowrap` clamp. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const TOOLTIP_SRC = path.resolve(__dirname, "../../src/shared/components/Tooltip.tsx"); + +const src = fs.readFileSync(TOOLTIP_SRC, "utf8"); + +test("#2352 Tooltip imports createPortal from react-dom", () => { + assert.ok( + /createPortal\s*\}\s*from\s+"react-dom"/.test(src), + "Tooltip must import createPortal so it can break out of clipping ancestors" + ); +}); + +test("#2352 Tooltip exposes usePortal prop defaulted to true", () => { + const propTrue = /usePortal\s*=\s*true/; + const propDecl = /usePortal\s*\?:\s*boolean/; + assert.ok(propDecl.test(src), "TooltipProps must declare optional usePortal prop"); + assert.ok( + propTrue.test(src), + "usePortal must default to true (the unsafe-by-default was the bug)" + ); +}); + +test("#2352 Tooltip portal target is document.body", () => { + assert.ok( + /createPortal\([^)]+,\s*document\.body\)/.test(src), + "Portal target must be document.body so the tooltip escapes overflow:hidden ancestors" + ); +}); + +test("#2352 multiline prop swaps whitespace-nowrap for wrap-friendly classes", () => { + assert.ok( + /multiline\s*\?\s*"max-w-xs whitespace-normal break-words"\s*:\s*"whitespace-nowrap"/.test(src), + "multiline=true must enable wrapping; default keeps the legacy single-line layout" + ); +}); + +test("#2352 portal tooltip uses fixed positioning (not absolute)", () => { + // When portaled to document.body, absolute positioning relative to the + // trigger no longer works — we need fixed coords computed from the + // trigger's getBoundingClientRect(). + assert.ok( + /portalEnabled[\s\S]{0,200}?`fixed\b/.test(src), + "Portal-rendered tooltip must use position:fixed to align with computed coords" + ); +}); + +test("#2352 portal tooltip clamps to viewport bounds (overflow-aware)", () => { + // Make sure the layout effect prevents the tooltip from running off the + // right or left edge of the screen — that's the user's screenshot bug. + assert.ok( + /maxLeft|minLeft|window\.innerWidth/.test(src), + "Portal tooltip must clamp horizontally so a trigger near the viewport edge stays visible" + ); +});