diff --git a/webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx b/webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx index eba47699abc..d2fb8606681 100644 --- a/webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx +++ b/webview-ui/src/components/chat/checkpoints/CheckpointMenu.tsx @@ -8,16 +8,32 @@ import { useRooPortal } from "@/components/ui/hooks" import { vscode } from "@src/utils/vscode" import { Checkpoint } from "./schema" -type CheckpointMenuProps = { +type CheckpointMenuBaseProps = { ts: number commitHash: string currentHash?: string checkpoint: Checkpoint } +type CheckpointMenuControlledProps = { + open: boolean + onOpenChange: (open: boolean) => void +} +type CheckpointMenuUncontrolledProps = { + open?: undefined + onOpenChange?: undefined +} +type CheckpointMenuProps = CheckpointMenuBaseProps & (CheckpointMenuControlledProps | CheckpointMenuUncontrolledProps) -export const CheckpointMenu = ({ ts, commitHash, currentHash, checkpoint }: CheckpointMenuProps) => { +export const CheckpointMenu = ({ + ts, + commitHash, + currentHash, + checkpoint, + 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") @@ -25,6 +41,9 @@ export const CheckpointMenu = ({ ts, commitHash, currentHash, checkpoint }: Chec const previousCommitHash = checkpoint?.from + const isOpen = open ?? internalOpen + const setOpen = onOpenChange ?? setInternalOpen + const onCheckpointDiff = useCallback(() => { vscode.postMessage({ type: "checkpointDiff", @@ -34,13 +53,23 @@ export const CheckpointMenu = ({ ts, commitHash, currentHash, checkpoint }: Chec 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) + if (!open) { + setIsConfirming(false) + } + }, + [setOpen], + ) return (
@@ -49,15 +78,10 @@ export const CheckpointMenu = ({ ts, commitHash, currentHash, checkpoint }: Chec - { - setIsOpen(open) - setIsConfirming(false) - }}> + - @@ -66,7 +90,7 @@ export const CheckpointMenu = ({ ts, commitHash, currentHash, checkpoint }: Chec
{!isCurrent && (
-
@@ -74,39 +98,50 @@ export const CheckpointMenu = ({ ts, commitHash, currentHash, checkpoint }: Chec
)} -
+ {!isCurrent && (
- {!isConfirming ? ( - - ) : ( - <> - - - - )} - {isConfirming ? ( -
- {t("chat:checkpoint.menu.cannotUndo")} -
- ) : ( -
- {t("chat:checkpoint.menu.restoreFilesAndTaskDescription")} -
- )} + ) : ( + <> + + + + )} + {isConfirming ? ( +
+ {t("chat:checkpoint.menu.cannotUndo")} +
+ ) : ( +
+ {t("chat:checkpoint.menu.restoreFilesAndTaskDescription")} +
+ )} +
-
+ )}
diff --git a/webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx b/webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx index b98f38b6d33..0172c1a344f 100644 --- a/webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx +++ b/webview-ui/src/components/chat/checkpoints/CheckpointSaved.tsx @@ -1,5 +1,6 @@ -import { useMemo } from "react" +import { useMemo, useRef, useState, useEffect } from "react" import { useTranslation } from "react-i18next" +import { cn } from "@/lib/utils" import { CheckpointMenu } from "./CheckpointMenu" import { checkpointSchema } from "./schema" @@ -15,6 +16,37 @@ type CheckpointSavedProps = { export const CheckpointSaved = ({ checkpoint, ...props }: CheckpointSavedProps) => { const { t } = useTranslation() const isCurrent = props.currentHash === props.commitHash + const [isPopoverOpen, setIsPopoverOpen] = useState(false) + const [isClosing, setIsClosing] = useState(false) + const closeTimer = useRef(null) + + useEffect(() => { + return () => { + if (closeTimer.current) { + window.clearTimeout(closeTimer.current) + closeTimer.current = null + } + } + }, []) + + const handlePopoverOpenChange = (open: boolean) => { + setIsPopoverOpen(open) + if (open) { + setIsClosing(false) + if (closeTimer.current) { + window.clearTimeout(closeTimer.current) + closeTimer.current = null + } + } else { + setIsClosing(true) + closeTimer.current = window.setTimeout(() => { + setIsClosing(false) + closeTimer.current = null + }, 200) // keep menu visible briefly to avoid popover jump + } + } + + const menuVisible = isPopoverOpen || isClosing const metadata = useMemo(() => { if (!checkpoint) { @@ -48,8 +80,16 @@ 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%)", }}> -
- + {/* Keep menu visible while popover is open or briefly after close to prevent jump */} +
+
) diff --git a/webview-ui/src/components/chat/checkpoints/__tests__/CheckpointSaved.spec.tsx b/webview-ui/src/components/chat/checkpoints/__tests__/CheckpointSaved.spec.tsx new file mode 100644 index 00000000000..3e1cd2a4652 --- /dev/null +++ b/webview-ui/src/components/chat/checkpoints/__tests__/CheckpointSaved.spec.tsx @@ -0,0 +1,141 @@ +// npx vitest run src/components/chat/checkpoints/__tests__/CheckpointSaved.spec.tsx + +vi.mock("@/components/ui", () => { + // Minimal UI primitives to ensure deterministic behavior in tests + return { + Button: ({ children, ...rest }: any) => , + StandardTooltip: ({ children }: any) => <>{children}, + Popover: ({ children, onOpenChange, open }: any) => { + lastOnOpenChange = onOpenChange + return ( +
+ {children} +
+ ) + }, + PopoverTrigger: ({ children }: any) =>
{children}
, + PopoverContent: ({ children }: any) =>
{children}
, + } +}) + +import { render, waitFor, screen } from "@/utils/test-utils" +import React from "react" +import userEvent from "@testing-library/user-event" +import { CheckpointSaved } from "../CheckpointSaved" + +// Capture onOpenChange from Popover to control open/close in tests +let lastOnOpenChange: ((open: boolean) => void) | undefined + +const waitForOpenHandler = async () => { + await waitFor(() => { + // ensure Popover mock captured the onOpenChange handler before using it + expect(lastOnOpenChange).toBeTruthy() + }) +} + +describe("CheckpointSaved popover visibility", () => { + // Timers are controlled per-test to avoid interfering with i18n init + const baseProps = { + ts: 123, + commitHash: "abc123", + currentHash: "zzz999", + checkpoint: { from: "prev123", to: "abc123" } as Record, + } + + it("shows menu while popover is open and hides when closed", async () => { + const { getByTestId } = render() + + const getMenu = () => getByTestId("checkpoint-menu-container") as HTMLElement + + // Initially hidden (relies on group-hover) + expect(getMenu()).toBeTruthy() + expect(getMenu().className).toContain("hidden") + + // Open via captured handler + await waitForOpenHandler() + lastOnOpenChange?.(true) + + await waitFor(() => { + expect(getMenu().className).toContain("block") + expect(getMenu().className).not.toContain("hidden") + }) + + // Close via captured handler — menu remains visible briefly, then hides + lastOnOpenChange?.(false) + + await waitFor(() => { + expect(getMenu().className).toContain("block") + }) + + await waitFor(() => { + expect(getMenu().className).toContain("hidden") + }) + }) + + it("resets confirm state when popover closes", async () => { + const { getByTestId } = render() + + // Open the popover + await waitForOpenHandler() + lastOnOpenChange?.(true) + + // Enter confirm state + const restoreFilesAndTaskBtn = await waitFor(() => getByTestId("restore-files-and-task-btn")) + await userEvent.click(restoreFilesAndTaskBtn) + + // Confirm warning should be visible + expect(getByTestId("checkpoint-confirm-warning")).toBeTruthy() + + // Close popover -> confirm state should reset + lastOnOpenChange?.(false) + + // Reopen + lastOnOpenChange?.(true) + + // Confirm warning should be gone after reopening + await waitFor(() => { + expect(screen.queryByTestId("checkpoint-confirm-warning")).toBeNull() + }) + }) + + it("closes popover after preview and after confirm restore", async () => { + const { getByTestId } = render() + + const popoverRoot = () => getByTestId("popover-root") + const menuContainer = () => getByTestId("checkpoint-menu-container") + + // Open + await waitForOpenHandler() + lastOnOpenChange?.(true) + await waitFor(() => { + expect(popoverRoot().getAttribute("data-open")).toBe("true") + expect(menuContainer().className).toContain("block") + }) + + // Click preview -> popover closes; menu remains briefly visible, then hides + await userEvent.click(getByTestId("restore-files-btn")) + await waitFor(() => { + expect(popoverRoot().getAttribute("data-open")).toBe("false") + expect(menuContainer().className).toContain("block") + }) + await waitFor(() => { + expect(menuContainer().className).toContain("hidden") + }) + + // Reopen + lastOnOpenChange?.(true) + await waitFor(() => { + expect(popoverRoot().getAttribute("data-open")).toBe("true") + }) + + // Enter confirm and confirm restore -> popover closes; menu then hides + await userEvent.click(getByTestId("restore-files-and-task-btn")) + await userEvent.click(getByTestId("confirm-restore-btn")) + await waitFor(() => { + expect(popoverRoot().getAttribute("data-open")).toBe("false") + }) + await waitFor(() => { + expect(menuContainer().className).toContain("hidden") + }) + }) +})