From 835e33951616b3ec4575607e1b86ff7e2b8c4d67 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 8 Nov 2026 11:34:25 +0700 Subject: [PATCH] fix(app): show one hover card at a time while scanning sidebar workspaces (#9071) * fix(app): keep one hover card open while scanning sidebar workspaces Sweeping the pointer down the workspace list showed cards about three frames late, stacked two or three at once, and left some open after the pointer moved on. The card was positioned through an async chain (measureInWindow, then onLayout, then a position effect, then the portal's passive mount), so it painted at -9999 first. While unplaced, its safe-zone bridge covered everything above the trigger, so upward pointer moves kept cancelling its close timer. The previous card also stayed for the close grace period plus its fade while the next one opened. Now each overlay layer holds one open card, and opening another trigger closes it immediately. The card reads its trigger and overlay rects in a layout effect inside the portal, so the first painted frame is already placed. The grace timer still covers jitter and the gap onto the card. * test(app): name the hover scan workspaces so the resting row's card is identified --- docs/hover.md | 2 +- .../app/e2e/browser/sidebar-workspace.spec.ts | 50 ++++++ packages/app/e2e/support/helpers/sidebar.ts | 29 ++++ packages/app/src/components/ui/hover-card.tsx | 146 +++++++++--------- 4 files changed, 155 insertions(+), 72 deletions(-) diff --git a/docs/hover.md b/docs/hover.md index fe8b539edc..9d523be064 100644 --- a/docs/hover.md +++ b/docs/hover.md @@ -135,7 +135,7 @@ Heuristic: if your hover state is going to be `useState`'d and read by anything Sometimes the revealed content can't live inside the trigger — a hover card portals into a different layer, a tooltip floats above other content, a popover renders into a `Portal`. There's a real visual gap the user has to cross with the cursor. -For a card whose content the user can point at or press, use `HoverCard` (`packages/app/src/components/ui/hover-card.tsx`). It is built on `useHoverSafeZone` (`packages/app/src/hooks/use-hover-safe-zone.ts`), which computes a rectangular "bridge" across the gap between the trigger and the content, whether the card sits beside, above, or below the trigger. While the pointer is inside trigger, content, or the bridge, the card stays open. A short grace timer absorbs jitter at the edges. `Tooltip` takes no pointer events and closes as soon as the pointer leaves the trigger. If the trigger has no focusable child, give `HoverCardTrigger` `focusable` and an accessible label so keyboard users can open the same content. +For a card whose content the user can point at or press, use `HoverCard` (`packages/app/src/components/ui/hover-card.tsx`). It is built on `useHoverSafeZone` (`packages/app/src/hooks/use-hover-safe-zone.ts`), which computes a rectangular "bridge" across the gap between the trigger and the content, whether the card sits beside, above, or below the trigger. While the pointer is inside trigger, content, or the bridge, the card stays open. A short grace timer absorbs jitter at the edges. Only one card per overlay layer is open: pointing at another trigger replaces the open card at once, so a sweep down a list never shows two cards or leaves one behind. `Tooltip` takes no pointer events and closes as soon as the pointer leaves the trigger. If the trigger has no focusable child, give `HoverCardTrigger` `focusable` and an accessible label so keyboard users can open the same content. Don't roll your own. The math is annoying, the edge cases (pointer leaves window, drag in progress, content unmounts) are subtle, and we already paid for the hook. diff --git a/packages/app/e2e/browser/sidebar-workspace.spec.ts b/packages/app/e2e/browser/sidebar-workspace.spec.ts index a7a7b4320a..a307002c3b 100644 --- a/packages/app/e2e/browser/sidebar-workspace.spec.ts +++ b/packages/app/e2e/browser/sidebar-workspace.spec.ts @@ -6,6 +6,7 @@ import { closeMobileAgentSidebar, expectMobileAgentSidebarHidden, expectMobileAgentSidebarVisible, + expectOnlyWorkspaceHoverCardFor, expectWorkspaceHoverCardClosed, expectWorkspaceHoverCardOpen, expectWorkspaceHoverCardStaysOpen, @@ -14,6 +15,7 @@ import { openMobileAgentSidebar, pinWorkspaceFromSidebar, scrollWorkspaceList, + sweepPointerAcrossWorkspaceRows, } from "../support/helpers/sidebar"; import { seedWorkspace } from "../support/helpers/seed-client"; import { expectWorkspaceHeader } from "../support/helpers/workspace-ui"; @@ -80,6 +82,22 @@ async function openWorkspaceHoverCard(page: import("@playwright/test").Page, wor return hoverCard; } +async function sortWorkspacesBySidebarOrder( + page: Page, + workspaces: T[], +): Promise { + const positioned = await Promise.all( + workspaces.map(async (workspace) => { + const bounds = await page + .getByTestId(getWorkspaceRowTestId(workspace.workspaceId)) + .boundingBox(); + if (!bounds) throw new Error(`Could not measure the row of ${workspace.workspaceId}`); + return { workspace, y: bounds.y }; + }), + ); + return positioned.sort((left, right) => left.y - right.y).map(({ workspace }) => workspace); +} + interface PaseoOwnedWorktree { projectName: string; workspaceId: string; @@ -430,6 +448,38 @@ test.describe("Sidebar workspace list", () => { } }); + test("scanning up the workspace rows leaves only the resting row's hover card open", async ({ + page, + }) => { + const workspaces = await Promise.all( + [1, 2, 3, 4, 5].map((index) => + seedWorkspace({ + repoPrefix: `sidebar-hover-scan-${index}-`, + title: `Hover scan ${index}`, + }), + ), + ); + + try { + await gotoAppShell(page); + for (const workspace of workspaces) { + await waitForSidebarWorkspace(page, workspace.workspaceId); + } + const rowsTopToBottom = await sortWorkspacesBySidebarOrder(page, workspaces); + const top = rowsTopToBottom[0]!; + + for (let scan = 0; scan < 3; scan += 1) { + await sweepPointerAcrossWorkspaceRows( + page, + rowsTopToBottom.map((workspace) => workspace.workspaceId).toReversed(), + ); + await expectOnlyWorkspaceHoverCardFor(page, top.workspaceName); + } + } finally { + await Promise.all(workspaces.map((workspace) => workspace.cleanup())); + } + }); + test("workspace hover card follows keyboard focus on a row", async ({ page }) => { const workspace = await seedWorkspace({ repoPrefix: "sidebar-hover-keyboard-" }); diff --git a/packages/app/e2e/support/helpers/sidebar.ts b/packages/app/e2e/support/helpers/sidebar.ts index 6e0a4abedc..96d1441070 100644 --- a/packages/app/e2e/support/helpers/sidebar.ts +++ b/packages/app/e2e/support/helpers/sidebar.ts @@ -96,6 +96,35 @@ export async function expectWorkspaceHoverCardStaysOpen(page: Page): Promise { + const centers: Array<{ x: number; y: number }> = []; + for (const workspaceId of workspaceIds) { + const row = page.getByTestId(`sidebar-workspace-row-${getServerId()}:${workspaceId}`); + await expect(row).toBeVisible({ timeout: 30_000 }); + const bounds = await row.boundingBox(); + if (!bounds) throw new Error(`Could not measure the row of ${workspaceId}`); + centers.push({ x: bounds.x + bounds.width / 2, y: bounds.y + bounds.height / 2 }); + } + const [first, ...rest] = centers; + await page.mouse.move(first!.x, first!.y); + for (const center of rest) { + await page.mouse.move(center.x, center.y, { steps: 3 }); + } +} + +export async function expectOnlyWorkspaceHoverCardFor( + page: Page, + workspaceName: string, +): Promise { + const cards = page.getByTestId("workspace-hover-card"); + await expect(cards).toHaveCount(1); + await expect(cards.getByTestId("hover-card-workspace-name")).toHaveText(workspaceName); +} + export async function expectWorkspaceHoverCardClosed(page: Page): Promise { await expect(page.getByTestId("workspace-hover-card")).toHaveCount(0); } diff --git a/packages/app/src/components/ui/hover-card.tsx b/packages/app/src/components/ui/hover-card.tsx index 164120c491..1ca418a541 100644 --- a/packages/app/src/components/ui/hover-card.tsx +++ b/packages/app/src/components/ui/hover-card.tsx @@ -4,6 +4,7 @@ import { useCallback, useContext, useEffect, + useLayoutEffect, useMemo, useRef, useState, @@ -17,23 +18,17 @@ import { StyleSheet } from "react-native-unistyles"; import { Portal } from "@gorhom/portal"; import { OverlayLayerProvider, useOverlayLayer } from "@/lib/overlay-root"; import { FloatingSurface } from "@/components/ui/floating"; -import { - measureFloatingPanelPortalHost, - useFloatingPanelPortalHostName, -} from "@/components/ui/floating-panel-portal"; +import { useFloatingPanelPortalHostName } from "@/components/ui/floating-panel-portal"; import { isWeb } from "@/constants/platform"; import { useHoverSafeZone } from "@/hooks/use-hover-safe-zone"; -import { - computePosition, - measureElement, - type Alignment, - type Placement, - type Rect, - type Size, -} from "./anchor"; +import { computePosition, type Alignment, type Placement, type Rect } from "./anchor"; const CLOSE_GRACE_MS = 100; +// One card per layer: moving onto a sibling trigger replaces the open card instead of stacking +// a second one beside it for the grace period. Nested cards open on a higher layer. +const openCardCloseByLayer = new Map void>(); + interface HoverCardContextValue { open: boolean; layer: number; @@ -116,12 +111,26 @@ function WebHoverCard({ }, CLOSE_GRACE_MS); }, [keyboardFocusInside]); + const closeNow = useCallback(() => { + clearGraceTimer(); + setOpen(false); + }, [clearGraceTimer]); + const openNow = useCallback(() => { clearGraceTimer(); - if (!disabled) { - setOpen(true); - } - }, [clearGraceTimer, disabled]); + if (disabled) return; + const openSibling = openCardCloseByLayer.get(layer); + if (openSibling && openSibling !== closeNow) openSibling(); + openCardCloseByLayer.set(layer, closeNow); + setOpen(true); + }, [clearGraceTimer, closeNow, disabled, layer]); + + useEffect(() => { + if (!open) return; + return () => { + if (openCardCloseByLayer.get(layer) === closeNow) openCardCloseByLayer.delete(layer); + }; + }, [closeNow, layer, open]); const keyPressed = useCallback( (event: KeyboardInputEvent): boolean => { @@ -299,17 +308,29 @@ interface HoverCardContentProps { export function HoverCardContent(props: HoverCardContentProps): ReactElement | null { const ctx = useContext(HoverCardContext); + const portalHostName = useFloatingPanelPortalHostName(); if (!ctx?.open) return null; return ( - + + + ); } +function readDomRect(view: View | null): Rect | null { + const element = view as unknown as HTMLElement | null; + if (!element) return null; + const { x, y, width, height } = element.getBoundingClientRect(); + return { x, y, width, height }; +} + +// Rendered inside the portal so the card is placed in the commit that mounts it: the first +// painted frame already sits beside the trigger. Web-only, like the rest of the open card. function HoverCardSurface({ layer, triggerRef, @@ -327,48 +348,29 @@ function HoverCardSurface({ triggerRef: RefObject; contentRef: RefObject; }): ReactElement { - const portalHostName = useFloatingPanelPortalHostName(); - const [anchor, setAnchor] = useState<{ triggerRect: Rect; hostRect: Rect } | null>(null); - const [contentSize, setContentSize] = useState(null); + const overlayRef = useRef(null); const [position, setPosition] = useState<{ x: number; y: number } | null>(null); - useEffect(() => { - if (!triggerRef.current) return; - let cancelled = false; - void Promise.all([ - measureElement(triggerRef.current), - measureFloatingPanelPortalHost(portalHostName), - ]).then(([triggerRect, hostRect]) => { - if (!cancelled && hostRect) setAnchor({ triggerRect, hostRect }); - return undefined; - }); - return () => { - cancelled = true; - }; - }, [portalHostName, triggerRef]); - - useEffect(() => { - if (!anchor || !contentSize) return; + const place = useCallback(() => { + const triggerRect = readDomRect(triggerRef.current); + const overlayRect = readDomRect(overlayRef.current); + const content = contentRef.current as unknown as HTMLElement | null; + if (!triggerRect || !overlayRect || !content) return; const { width, height } = Dimensions.get("window"); const { x, y } = computePosition({ - triggerRect: anchor.triggerRect, - contentSize, + triggerRect, + contentSize: { width: content.offsetWidth, height: content.offsetHeight }, displayArea: { x: 0, y: 0, width, height }, placement, flipHorizontal: true, alignment, offset, }); - setPosition({ x: x - anchor.hostRect.x, y: y - anchor.hostRect.y }); - }, [alignment, anchor, contentSize, offset, placement]); + const next = { x: x - overlayRect.x, y: y - overlayRect.y }; + setPosition((current) => (current?.x === next.x && current.y === next.y ? current : next)); + }, [alignment, contentRef, offset, placement, triggerRef]); - const handleLayout = useCallback( - (event: { nativeEvent: { layout: { width: number; height: number } } }) => { - const { width, height } = event.nativeEvent.layout; - setContentSize({ width, height }); - }, - [], - ); + useLayoutEffect(place, [place]); const frameStyle = useMemo( () => ({ @@ -381,24 +383,26 @@ function HoverCardSurface({ const surfaceStyle = useMemo(() => [styles.surface, style], [style]); return ( - - - - {children} - - - + + + {children} + + ); }