diff --git a/webview-ui/src/components/file-changes/FilesChangedOverview.tsx b/webview-ui/src/components/file-changes/FilesChangedOverview.tsx index f8f01268f6..5bdc144665 100644 --- a/webview-ui/src/components/file-changes/FilesChangedOverview.tsx +++ b/webview-ui/src/components/file-changes/FilesChangedOverview.tsx @@ -3,9 +3,7 @@ import { FileChangeset, FileChange } from "@roo-code/types" import { useTranslation } from "react-i18next" import { useExtensionState } from "@/context/ExtensionStateContext" import { vscode } from "@/utils/vscode" - -// eslint-disable-next-line @typescript-eslint/no-empty-object-type -interface FilesChangedOverviewProps {} +import { useDebouncedAction } from "@/components/ui/hooks/useDebouncedAction" interface _CheckpointEventData { type: "checkpoint_created" | "checkpoint_restored" @@ -18,7 +16,7 @@ interface _CheckpointEventData { * and displays file changes. It manages its own state and communicates with the backend * through VS Code message passing. */ -const FilesChangedOverview: React.FC = () => { +const FilesChangedOverview: React.FC = () => { const { t } = useTranslation() const { filesChangedEnabled } = useExtensionState() @@ -52,24 +50,13 @@ const FilesChangedOverview: React.FC = () => { const totalHeight = shouldVirtualize ? files.length * ITEM_HEIGHT : "auto" const offsetY = shouldVirtualize ? Math.floor(scrollTop / ITEM_HEIGHT) * ITEM_HEIGHT : 0 - // Simple double-click prevention - const [isProcessing, setIsProcessing] = React.useState(false) - const timeoutRef = React.useRef(null) - - // Cleanup timeout on unmount - React.useEffect(() => { - return () => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current) - } - } - }, []) + // Debounced click handling for double-click prevention + const { isProcessing, handleWithDebounce } = useDebouncedAction(300) // FCO initialization logic const checkInit = React.useCallback( - (baseCheckpoint: string) => { + (_baseCheckpoint: string) => { if (!isInitialized) { - console.log("[FCO] Initializing with base checkpoint:", baseCheckpoint) setIsInitialized(true) } }, @@ -94,9 +81,7 @@ const FilesChangedOverview: React.FC = () => { ) // Handle checkpoint restoration with the 4 examples logic - const handleCheckpointRestored = React.useCallback((restoredCheckpoint: string) => { - console.log("[FCO] Handling checkpoint restore to:", restoredCheckpoint) - + const handleCheckpointRestored = React.useCallback((_restoredCheckpoint: string) => { // Request file changes after checkpoint restore // Backend should calculate changes from initial baseline to restored checkpoint vscode.postMessage({ type: "filesChangedRequest" }) @@ -128,25 +113,6 @@ const FilesChangedOverview: React.FC = () => { // Backend will send updated filesChanged message with filtered results }, [files]) - const handleWithDebounce = React.useCallback( - async (operation: () => void) => { - if (isProcessing) return - setIsProcessing(true) - try { - operation() - } catch (_error) { - // Silently handle any errors to prevent crashing - // Debug logging removed for production - } - // Brief delay to prevent double-clicks - if (timeoutRef.current) { - clearTimeout(timeoutRef.current) - } - timeoutRef.current = setTimeout(() => setIsProcessing(false), 300) - }, - [isProcessing], - ) - /** * Handles scroll events for virtualization * Updates scrollTop state to calculate visible items @@ -167,14 +133,12 @@ const FilesChangedOverview: React.FC = () => { // Guard against null/undefined/malformed messages if (!message || typeof message !== "object" || !message.type) { - console.debug("[FCO] Ignoring malformed message:", message) return } switch (message.type) { case "filesChanged": if (message.filesChanged) { - console.log("[FCO] Received filesChanged message:", message.filesChanged) checkInit(message.filesChanged.baseCheckpoint) updateChangeset(message.filesChanged) } else { @@ -183,11 +147,9 @@ const FilesChangedOverview: React.FC = () => { } break case "checkpoint_created": - console.log("[FCO] Checkpoint created:", message.checkpoint) handleCheckpointCreated(message.checkpoint, message.previousCheckpoint) break case "checkpoint_restored": - console.log("[FCO] Checkpoint restored:", message.checkpoint) handleCheckpointRestored(message.checkpoint) break } diff --git a/webview-ui/src/components/settings/ExperimentalSettings.tsx b/webview-ui/src/components/settings/ExperimentalSettings.tsx index 6883975d02..0f4d0e6778 100644 --- a/webview-ui/src/components/settings/ExperimentalSettings.tsx +++ b/webview-ui/src/components/settings/ExperimentalSettings.tsx @@ -8,11 +8,12 @@ import { EXPERIMENT_IDS, experimentConfigsMap } from "@roo/experiments" import { useAppTranslation } from "@src/i18n/TranslationContext" import { cn } from "@src/lib/utils" -import { SetExperimentEnabled } from "./types" +import { SetExperimentEnabled, SetCachedStateField } from "./types" import { SectionHeader } from "./SectionHeader" import { Section } from "./Section" import { ExperimentalFeature } from "./ExperimentalFeature" import { ImageGenerationSettings } from "./ImageGenerationSettings" +import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react" type ExperimentalSettingsProps = HTMLAttributes & { experiments: Experiments @@ -23,6 +24,9 @@ type ExperimentalSettingsProps = HTMLAttributes & { openRouterImageGenerationSelectedModel?: string setOpenRouterImageApiKey?: (apiKey: string) => void setImageGenerationSelectedModel?: (model: string) => void + // Include Files Changed Overview toggle in Experimental section per review feedback + filesChangedEnabled?: boolean + setCachedStateField?: SetCachedStateField<"filesChangedEnabled"> } export const ExperimentalSettings = ({ @@ -34,6 +38,8 @@ export const ExperimentalSettings = ({ openRouterImageGenerationSelectedModel, setOpenRouterImageApiKey, setImageGenerationSelectedModel, + filesChangedEnabled, + setCachedStateField, className, ...props }: ExperimentalSettingsProps) => { @@ -48,6 +54,24 @@ export const ExperimentalSettings = ({ + {/* Files Changed Overview (moved from UI section to Experimental) */} + {typeof filesChangedEnabled !== "undefined" && setCachedStateField && ( +
+
+ setCachedStateField("filesChangedEnabled", e.target.checked)} + data-testid="files-changed-enabled-checkbox"> + {/* Reuse existing translation keys to avoid i18n churn */} + + +
+ {t("settings:ui.filesChanged.description")} +
+
+
+ )} +
{Object.entries(experimentConfigsMap) .filter(([key]) => key in EXPERIMENT_IDS) diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 47801a4bb9..5e4eb5ef59 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -50,8 +50,9 @@ import { } from "@src/components/ui" import { Tab, TabContent, TabHeader, TabList, TabTrigger } from "../common/Tab" -import { SetCachedStateField, SetExperimentEnabled } from "./types" +import { SetExperimentEnabled } from "./types" import { SectionHeader } from "./SectionHeader" +import type { SetCachedStateField } from "./types" import ApiConfigManager from "./ApiConfigManager" import ApiOptions from "./ApiOptions" import { AutoApproveSettings } from "./AutoApproveSettings" @@ -730,7 +731,7 @@ const SettingsView = forwardRef(({ onDone, t {activeTab === "ui" && ( } /> )} @@ -777,6 +778,8 @@ const SettingsView = forwardRef(({ onDone, t } setOpenRouterImageApiKey={setOpenRouterImageApiKey} setImageGenerationSelectedModel={setImageGenerationSelectedModel} + filesChangedEnabled={filesChangedEnabled} + setCachedStateField={setCachedStateField as SetCachedStateField<"filesChangedEnabled">} /> )} diff --git a/webview-ui/src/components/settings/UISettings.tsx b/webview-ui/src/components/settings/UISettings.tsx index bf1a8edd70..8bf16287d1 100644 --- a/webview-ui/src/components/settings/UISettings.tsx +++ b/webview-ui/src/components/settings/UISettings.tsx @@ -1,21 +1,15 @@ import { HTMLAttributes } from "react" import React from "react" import { useAppTranslation } from "@/i18n/TranslationContext" -import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react" import { Monitor } from "lucide-react" import { cn } from "@/lib/utils" -import { SetCachedStateField } from "./types" import { SectionHeader } from "./SectionHeader" -import { Section } from "./Section" -type UISettingsProps = HTMLAttributes & { - filesChangedEnabled?: boolean - setCachedStateField: SetCachedStateField<"filesChangedEnabled"> -} +type UISettingsProps = HTMLAttributes -export const UISettings = ({ filesChangedEnabled, setCachedStateField, className, ...props }: UISettingsProps) => { +export const UISettings = ({ className, ...props }: UISettingsProps) => { const { t } = useAppTranslation() return ( @@ -26,20 +20,6 @@ export const UISettings = ({ filesChangedEnabled, setCachedStateField, className
{t("settings:sections.ui")}
- -
-
- setCachedStateField("filesChangedEnabled", e.target.checked)} - data-testid="files-changed-enabled-checkbox"> - - -
- {t("settings:ui.filesChanged.description")} -
-
-
) } diff --git a/webview-ui/src/components/settings/__tests__/UISettings.spec.tsx b/webview-ui/src/components/settings/__tests__/UISettings.spec.tsx index 4ba8c447fc..0319ef9da2 100644 --- a/webview-ui/src/components/settings/__tests__/UISettings.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/UISettings.spec.tsx @@ -1,4 +1,4 @@ -import { render, screen, fireEvent } from "@/utils/test-utils" +import { render, screen } from "@/utils/test-utils" import { UISettings } from "@src/components/settings/UISettings" @@ -9,181 +9,39 @@ vitest.mock("@/i18n/TranslationContext", () => ({ }), })) -// Mock VSCode components to behave like standard HTML elements -vitest.mock("@vscode/webview-ui-toolkit/react", () => ({ - VSCodeCheckbox: ({ checked, onChange, children, "data-testid": dataTestId, ...props }: any) => ( -
- - {children} -
- ), -})) - describe("UISettings", () => { - const defaultProps = { - filesChangedEnabled: false, - setCachedStateField: vitest.fn(), - } - beforeEach(() => { vitest.clearAllMocks() }) it("renders the UI settings section", () => { - render() + render() // Check that the section header is rendered expect(screen.getByText("settings:sections.ui")).toBeInTheDocument() expect(screen.getByText("settings:ui.description")).toBeInTheDocument() }) - it("renders the files changed overview checkbox", () => { - render() - - // Files changed overview checkbox - const filesChangedCheckbox = screen.getByTestId("files-changed-enabled-checkbox") - expect(filesChangedCheckbox).toBeInTheDocument() - expect(filesChangedCheckbox).not.toBeChecked() - - // Check label and description are present - expect(screen.getByText("settings:ui.filesChanged.label")).toBeInTheDocument() - expect(screen.getByText("settings:ui.filesChanged.description")).toBeInTheDocument() - }) - - it("displays correct state when filesChangedEnabled is true", () => { - const propsWithEnabled = { - ...defaultProps, - filesChangedEnabled: true, - } - render() - - const checkbox = screen.getByTestId("files-changed-enabled-checkbox") - expect(checkbox).toBeChecked() - }) - - it("displays correct state when filesChangedEnabled is false", () => { - const propsWithDisabled = { - ...defaultProps, - filesChangedEnabled: false, - } - render() - - const checkbox = screen.getByTestId("files-changed-enabled-checkbox") - expect(checkbox).not.toBeChecked() - }) - - it("calls setCachedStateField when files changed checkbox is toggled", () => { - const mockSetCachedStateField = vitest.fn() - const props = { - ...defaultProps, - filesChangedEnabled: false, - setCachedStateField: mockSetCachedStateField, - } - render() - - const checkbox = screen.getByTestId("files-changed-enabled-checkbox") - fireEvent.click(checkbox) - - expect(mockSetCachedStateField).toHaveBeenCalledWith("filesChangedEnabled", true) - }) - - it("calls setCachedStateField with false when enabled checkbox is clicked", () => { - const mockSetCachedStateField = vitest.fn() - const props = { - ...defaultProps, - filesChangedEnabled: true, - setCachedStateField: mockSetCachedStateField, - } - render() - - const checkbox = screen.getByTestId("files-changed-enabled-checkbox") - fireEvent.click(checkbox) - - expect(mockSetCachedStateField).toHaveBeenCalledWith("filesChangedEnabled", false) - }) - - it("handles undefined filesChangedEnabled gracefully", () => { - const propsWithUndefined = { - ...defaultProps, - filesChangedEnabled: undefined, - } - - expect(() => { - render() - }).not.toThrow() - - const checkbox = screen.getByTestId("files-changed-enabled-checkbox") - expect(checkbox).not.toBeChecked() // Should default to false for undefined - }) - - describe("Accessibility", () => { - it("has proper labels and descriptions", () => { - render() - - // Check that labels are present - expect(screen.getByText("settings:ui.filesChanged.label")).toBeInTheDocument() - - // Check that descriptions are present - expect(screen.getByText("settings:ui.filesChanged.description")).toBeInTheDocument() - }) - - it("has proper test ids for all interactive elements", () => { - render() - - expect(screen.getByTestId("files-changed-enabled-checkbox")).toBeInTheDocument() - }) - - it("has proper checkbox role and aria attributes", () => { - render() - - const checkbox = screen.getByTestId("files-changed-enabled-checkbox") - expect(checkbox).toHaveAttribute("role", "checkbox") - expect(checkbox).toHaveAttribute("aria-checked", "false") - }) - - it("updates aria-checked when state changes", () => { - const propsWithEnabled = { - ...defaultProps, - filesChangedEnabled: true, - } - render() - - const checkbox = screen.getByTestId("files-changed-enabled-checkbox") - expect(checkbox).toHaveAttribute("aria-checked", "true") - }) - }) - describe("Integration with translation system", () => { it("uses translation keys for all text content", () => { - render() + render() // Verify that translation keys are being used (mocked to return the key) expect(screen.getByText("settings:sections.ui")).toBeInTheDocument() expect(screen.getByText("settings:ui.description")).toBeInTheDocument() - expect(screen.getByText("settings:ui.filesChanged.label")).toBeInTheDocument() - expect(screen.getByText("settings:ui.filesChanged.description")).toBeInTheDocument() }) }) describe("Component structure", () => { it("renders with custom className", () => { - const { container } = render() + const { container } = render() const uiSettingsDiv = container.firstChild as HTMLElement expect(uiSettingsDiv).toHaveClass("custom-class") }) it("passes through additional props", () => { - const { container } = render() + const { container } = render() const uiSettingsDiv = container.firstChild as HTMLElement expect(uiSettingsDiv).toHaveAttribute("data-custom", "test-value") diff --git a/webview-ui/src/components/ui/hooks/useDebouncedAction.ts b/webview-ui/src/components/ui/hooks/useDebouncedAction.ts new file mode 100644 index 0000000000..66eeb9f8df --- /dev/null +++ b/webview-ui/src/components/ui/hooks/useDebouncedAction.ts @@ -0,0 +1,32 @@ +import { useCallback, useRef, useState } from "react" + +export function useDebouncedAction(delay = 300) { + const [isProcessing, setIsProcessing] = useState(false) + const timeoutRef = useRef(null) + + const handleWithDebounce = useCallback( + (operation: () => void) => { + if (isProcessing) return + setIsProcessing(true) + try { + operation() + } catch { + // no-op: swallow errors from caller operations + } + if (timeoutRef.current) { + clearTimeout(timeoutRef.current) + } + timeoutRef.current = setTimeout( + () => { + setIsProcessing(false) + }, + Math.max(0, delay), + ) + }, + [isProcessing, delay], + ) + + return { isProcessing, handleWithDebounce } +} + +export default useDebouncedAction