fix(checkpoints): control Popover from parent to prevent anchor loss; add focused test for visibility logic (#8220)

Author: daniel-lxs

webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx

@@ -13,25 +13,30 @@ type CheckpointMenuProps = {
 	commitHash: string
 	currentHash?: string
 	checkpoint: Checkpoint
-	onPopoverOpenChange?: (open: boolean) => void
+	open?: boolean
+	onOpenChange?: (open: boolean) => void
 }
 
 export const CheckpointMenu = ({
 	ts,
 	commitHash,
 	currentHash,
 	checkpoint,
-	onPopoverOpenChange,
+	open,
+	onOpenChange,
 }: CheckpointMenuProps) => {
 	const { t } = useTranslation()
-	const [isOpen, setIsOpen] = useState(false)
+	const [internalOpen, setInternalOpen] = useState(false)
 	const [isConfirming, setIsConfirming] = useState(false)
 	const portalContainer = useRooPortal("roo-portal")
 
 	const isCurrent = currentHash === commitHash
 
 	const previousCommitHash = checkpoint?.from
 
+	const isOpen = open ?? internalOpen
+	const setOpen = onOpenChange ?? setInternalOpen
+
 	const onCheckpointDiff = useCallback(() => {
 		vscode.postMessage({
 			type: "checkpointDiff",
@@ -41,13 +46,21 @@ export const CheckpointMenu = ({
 
 	const onPreview = useCallback(() => {
 		vscode.postMessage({ type: "checkpointRestore", payload: { ts, commitHash, mode: "preview" } })
-		setIsOpen(false)
-	}, [ts, commitHash])
+		setOpen(false)
+	}, [ts, commitHash, setOpen])
 
 	const onRestore = useCallback(() => {
 		vscode.postMessage({ type: "checkpointRestore", payload: { ts, commitHash, mode: "restore" } })
-		setIsOpen(false)
-	}, [ts, commitHash])
+		setOpen(false)
+	}, [ts, commitHash, setOpen])
+
+	const handleOpenChange = useCallback(
+		(open: boolean) => {
+			setOpen(open)
+			setIsConfirming(false)
+		},
+		[setOpen],
+	)
 
 	return (
 		<div className="flex flex-row gap-1">
@@ -56,13 +69,7 @@ export const CheckpointMenu = ({
 					<span className="codicon codicon-diff-single" />
 				</Button>
 			</StandardTooltip>
-			<Popover
-				open={isOpen}
-				onOpenChange={(open) => {
-					setIsOpen(open)
-					setIsConfirming(false)
-					onPopoverOpenChange?.(open)
-				}}>
+			<Popover open={isOpen} onOpenChange={handleOpenChange}>
 				<StandardTooltip content={t("chat:checkpoint.menu.restore")}>
 					<PopoverTrigger asChild>
 						<Button variant="ghost" size="icon">

webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx

@@ -1,5 +1,6 @@
 import { useMemo, useState } from "react"
 import { useTranslation } from "react-i18next"
+import { cn } from "@/lib/utils"
 
 import { CheckpointMenu } from "./CheckpointMenu"
 import { checkpointSchema } from "./schema"
@@ -49,8 +50,9 @@ export const CheckpointSaved = ({ checkpoint, ...props }: CheckpointSavedProps)
 						"linear-gradient(90deg, rgba(0, 188, 255, .65), rgba(0, 188, 255, .65) 80%, rgba(0, 188, 255, 0) 99%)",
 				}}></span>
 
-			<div className={`h-4 -mt-2 ${isPopoverOpen ? "block" : "hidden group-hover:block"}`}>
-				<CheckpointMenu {...props} checkpoint={metadata} onPopoverOpenChange={setIsPopoverOpen} />
+			{/* Keep menu visible while popover is open; otherwise rely on group-hover */}
+			<div className={cn("h-4 -mt-2", isPopoverOpen ? "block" : "hidden group-hover:block")}>
+				<CheckpointMenu {...props} checkpoint={metadata} open={isPopoverOpen} onOpenChange={setIsPopoverOpen} />
 			</div>
 		</div>
 	)

webview-ui/src/components/chat/checkpoints/__tests__/CheckpointSaved.spec.tsx

@@ -0,0 +1,71 @@
+// npx vitest run src/components/chat/checkpoints/__tests__/CheckpointSaved.spec.tsx
+
+import { render, waitFor } from "@/utils/test-utils"
+import React from "react"
+import { CheckpointSaved } from "../CheckpointSaved"
+
+// Capture onOpenChange from Popover to control open/close in tests
+let lastOnOpenChange: ((open: boolean) => void) | undefined
+
+vi.mock("@/components/ui", async () => {
+	const actual = await vi.importActual<any>("@/components/ui")
+	return {
+		...actual,
+		Popover: ({
+			children,
+			onOpenChange,
+			open,
+		}: {
+			children: React.ReactNode
+			open?: boolean
+			onOpenChange?: (open: boolean) => void
+		}) => {
+			lastOnOpenChange = onOpenChange
+			return (
+				<div data-testid="popover-root" data-open={open}>
+					{children}
+				</div>
+			)
+		},
+		PopoverTrigger: ({ children }: { children: React.ReactNode }) => (
+			<div data-testid="popover-trigger">{children}</div>
+		),
+		PopoverContent: ({ children }: { children: React.ReactNode }) => (
+			<div data-testid="popover-content">{children}</div>
+		),
+	}
+})
+
+describe("CheckpointSaved popover visibility", () => {
+	const baseProps = {
+		ts: 123,
+		commitHash: "abc123",
+		currentHash: "zzz999",
+		checkpoint: { from: "prev123", to: "abc123" } as Record<string, unknown>,
+	}
+
+	it("shows menu while popover is open and hides when closed", async () => {
+		const { container } = render(<CheckpointSaved {...baseProps} />)
+
+		const getMenu = () => container.querySelector("div.h-4.-mt-2") as HTMLElement
+
+		// Initially hidden (relies on group-hover)
+		expect(getMenu()).toBeTruthy()
+		expect(getMenu().className).toContain("hidden")
+
+		// Open via captured handler
+		lastOnOpenChange?.(true)
+
+		await waitFor(() => {
+			expect(getMenu().className).toContain("block")
+			expect(getMenu().className).not.toContain("hidden")
+		})
+
+		// Close via captured handler
+		lastOnOpenChange?.(false)
+
+		await waitFor(() => {
+			expect(getMenu().className).toContain("hidden")
+		})
+	})
+})