diff --git a/frontend/packages/shared/src/dropdown/Dropdown.tsx b/frontend/packages/shared/src/dropdown/Dropdown.tsx index 3e70341d..8825d168 100644 --- a/frontend/packages/shared/src/dropdown/Dropdown.tsx +++ b/frontend/packages/shared/src/dropdown/Dropdown.tsx @@ -9,12 +9,23 @@ // for free, so this hand-rolls the same "outside click or Escape // closes" contract `Dialog` gets from the platform. // -// Positioning is the caller's job: render `` as a child of a -// `position: relative` wrapper (the badge + dropdown pair) and it anchors -// to that box's bottom-left via CSS. No portal — a badge in a normal -// document-flow header never needs one, and skipping it keeps focus -// management simple (no re-parenting to `` to reason about). -import { useEffect, useRef } from "preact/hooks"; +// Positioning is the caller's job by default: render `` as a +// child of a `position: relative` wrapper (the badge + dropdown pair) +// and it anchors to that box's bottom-left via CSS — no portal, no +// re-parenting to `` to reason about. That's wrong the moment the +// wrapper sits inside a scrolling container, though: an ancestor with +// `overflow-x: auto` computes `overflow-y` to `auto` too (per the CSS +// overflow spec), so it clips the dropdown vertically the instant the +// container is shorter than header-plus-dropdown — reported against +// swarm-ui's agent table (a `WantedMenu` badge two rows from the +// bottom). `../../ui/table/Table.tsx` hit the identical clip for its +// own column-filter popover and fixed it with `position: fixed` +// computed from the anchor's `getBoundingClientRect()`, rendered via a +// portal outside the scrolling subtree; the optional `portal` prop +// below opts a caller into that same recipe instead of a second +// hand-rolled copy of it. +import { useEffect, useRef, useState } from "preact/hooks"; +import { createPortal } from "preact/compat"; import type { ComponentChildren, RefObject } from "preact"; import "./Dropdown.css"; @@ -51,6 +62,13 @@ export interface DropdownProps { * one ref) already avoids this. */ anchorRef?: RefObject; + /** + * Escape a clipping scroll ancestor by portal-rendering to `` + * with `position: fixed`, computed from `anchorRef`'s rect — see the + * file-top comment. Requires `anchorRef`; a caller in an unscrolled, + * un-clipped context (the common case) can omit this entirely. + */ + portal?: boolean; } export function Dropdown({ @@ -61,8 +79,13 @@ export function Dropdown({ onClose, label, anchorRef, + portal, }: DropdownProps) { const ref = useRef(null); + // Only populated (and only consulted) in `portal` mode — see the + // effect below and the file-top comment for why fixed-position + // coordinates are needed at all here. + const [pos, setPos] = useState<{ top: number; left: number } | null>(null); useEffect(() => { if (!open) return; @@ -86,10 +109,35 @@ export function Dropdown({ }; }, [open, onClose]); + // Same recipe as `Table.tsx`'s column-filter popover: compute once on + // open, then keep it pinned to the anchor across any scroll in the + // tree (`capture: true` sees a nested scroll container too, since + // plain `scroll` doesn't bubble) or a viewport resize. + useEffect(() => { + if (!open || !portal) return; + function recompute() { + const rect = anchorRef?.current?.getBoundingClientRect(); + if (rect) setPos({ top: rect.bottom + 4, left: rect.left }); + } + recompute(); + window.addEventListener("scroll", recompute, true); + window.addEventListener("resize", recompute); + return () => { + window.removeEventListener("scroll", recompute, true); + window.removeEventListener("resize", recompute); + }; + }, [open, portal, anchorRef]); + if (!open) return null; - return ( -