From cfd2f340fb87d8e3a0279bc2a52bfac99db8949a Mon Sep 17 00:00:00 2001 From: iris Date: Sun, 1 Nov 2026 01:32:47 +0100 Subject: [PATCH] shared: drop Dropdown's portal mode, which only the agents table used The agents table passed portal so a wanted menu on its last row escaped the table's scroll clip. The table is gone from that page and nothing else passes it, so the prop, its position tracking and the portal render go with it. --- .../packages/shared/src/dropdown/Dropdown.tsx | 74 +++---------------- 1 file changed, 11 insertions(+), 63 deletions(-) diff --git a/frontend/packages/shared/src/dropdown/Dropdown.tsx b/frontend/packages/shared/src/dropdown/Dropdown.tsx index 039fd9cd..8593fd46 100644 --- a/frontend/packages/shared/src/dropdown/Dropdown.tsx +++ b/frontend/packages/shared/src/dropdown/Dropdown.tsx @@ -8,23 +8,15 @@ // 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 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"; +// 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. Put the wrapper outside any scrolling +// container: an ancestor with `overflow-x: auto` computes `overflow-y` to `auto` +// too (per the CSS overflow spec), so it clips the dropdown vertically. A +// `position: fixed` list rendered through a portal escapes that, as +// `../../ui/table/Table.tsx` does for its column filters; this component had +// that mode for the agents table, and dropped it with the table. +import { useEffect, useRef } from "preact/hooks"; import type { ComponentChildren, RefObject } from "preact"; import "./Dropdown.css"; @@ -61,13 +53,6 @@ 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({ @@ -78,14 +63,8 @@ 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; function handlePointerDown(e: PointerEvent) { @@ -108,35 +87,10 @@ 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; - const list = ( -