scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-08-28 05:27:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix(settings): move FCO toggle to Experimental, adopt debounced action hook, clean up UISettings and SettingsView imports/tests; fix lint unused args in FilesChangedOverview

Browse source
This commit is contained in:
Hannes Rudolph 2025-08-27 17:32:09 -06:00
parent 73cd31b492
commit 5f5cdf1b46
6 changed files with 75 additions and 216 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
webview-ui/src/components/file-changes/FilesChangedOverview.tsx
View file

@ -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<FilesChangedOverviewProps> = () => {
const FilesChangedOverview: React.FC = () => {
const { t } = useTranslation()
const { filesChangedEnabled } = useExtensionState()
@ -52,24 +50,13 @@ const FilesChangedOverview: React.FC<FilesChangedOverviewProps> = () => {
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<NodeJS.Timeout | null>(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<FilesChangedOverviewProps> = () => {
)
// 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<FilesChangedOverviewProps> = () => {
// 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<FilesChangedOverviewProps> = () => {
// 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<FilesChangedOverviewProps> = () => {
}
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
}

26
webview-ui/src/components/settings/ExperimentalSettings.tsx
View file

@ -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<HTMLDivElement> & {
experiments: Experiments
@ -23,6 +24,9 @@ type ExperimentalSettingsProps = HTMLAttributes<HTMLDivElement> & {
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 = ({
</div>
</SectionHeader>
{/* Files Changed Overview (moved from UI section to Experimental) */}
{typeof filesChangedEnabled !== "undefined" && setCachedStateField && (
<Section>
<div>
<VSCodeCheckbox
checked={filesChangedEnabled}
onChange={(e: any) => setCachedStateField("filesChangedEnabled", e.target.checked)}
data-testid="files-changed-enabled-checkbox">
{/* Reuse existing translation keys to avoid i18n churn */}
<label className="block font-medium mb-1">{t("settings:ui.filesChanged.label")}</label>
</VSCodeCheckbox>
<div className="text-vscode-descriptionForeground text-sm mt-1 mb-3">
{t("settings:ui.filesChanged.description")}
</div>
</div>
</Section>
)}
<Section>
{Object.entries(experimentConfigsMap)
.filter(([key]) => key in EXPERIMENT_IDS)

7
webview-ui/src/components/settings/SettingsView.tsx
View file

@ -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<SettingsViewRef, SettingsViewProps>(({ onDone, t
{activeTab === "ui" && (
<UISettings
filesChangedEnabled={filesChangedEnabled}
setCachedStateField={setCachedStateField}
setCachedStateField={setCachedStateField as SetCachedStateField<"filesChangedEnabled">}
/>
)}
@ -777,6 +778,8 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
}
setOpenRouterImageApiKey={setOpenRouterImageApiKey}
setImageGenerationSelectedModel={setImageGenerationSelectedModel}
filesChangedEnabled={filesChangedEnabled}
setCachedStateField={setCachedStateField as SetCachedStateField<"filesChangedEnabled">}
/>
)}

24
webview-ui/src/components/settings/UISettings.tsx
View file

@ -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<HTMLDivElement> & {
filesChangedEnabled?: boolean
setCachedStateField: SetCachedStateField<"filesChangedEnabled">
}
type UISettingsProps = HTMLAttributes<HTMLDivElement>
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
<div>{t("settings:sections.ui")}</div>
</div>
</SectionHeader>
<Section>
<div>
<VSCodeCheckbox
checked={filesChangedEnabled}
onChange={(e: any) => setCachedStateField("filesChangedEnabled", e.target.checked)}
data-testid="files-changed-enabled-checkbox">
<label className="block font-medium mb-1">{t("settings:ui.filesChanged.label")}</label>
</VSCodeCheckbox>
<div className="text-vscode-descriptionForeground text-sm mt-1 mb-3">
{t("settings:ui.filesChanged.description")}
</div>
</div>
</Section>
</div>
)
}

152
webview-ui/src/components/settings/__tests__/UISettings.spec.tsx
View file

@ -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) => (
<div>
<input
type="checkbox"
checked={checked}
onChange={onChange}
data-testid={dataTestId}
aria-label={children?.props?.children || children}
role="checkbox"
aria-checked={checked}
{...props}
/>
{children}
</div>
),
}))
describe("UISettings", () => {
const defaultProps = {
filesChangedEnabled: false,
setCachedStateField: vitest.fn(),
}
beforeEach(() => {
vitest.clearAllMocks()
})
it("renders the UI settings section", () => {
render(<UISettings {...defaultProps} />)
render(<UISettings />)
// 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(<UISettings {...defaultProps} />)
// 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(<UISettings {...propsWithEnabled} />)
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(<UISettings {...propsWithDisabled} />)
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(<UISettings {...props} />)
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(<UISettings {...props} />)
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(<UISettings {...propsWithUndefined} />)
}).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(<UISettings {...defaultProps} />)
// 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(<UISettings {...defaultProps} />)
expect(screen.getByTestId("files-changed-enabled-checkbox")).toBeInTheDocument()
})
it("has proper checkbox role and aria attributes", () => {
render(<UISettings {...defaultProps} />)
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(<UISettings {...propsWithEnabled} />)
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(<UISettings {...defaultProps} />)
render(<UISettings />)
// 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(<UISettings {...defaultProps} className="custom-class" />)
const { container } = render(<UISettings className="custom-class" />)
const uiSettingsDiv = container.firstChild as HTMLElement
expect(uiSettingsDiv).toHaveClass("custom-class")
})
it("passes through additional props", () => {
const { container } = render(<UISettings {...defaultProps} data-custom="test-value" />)
const { container } = render(<UISettings data-custom="test-value" />)
const uiSettingsDiv = container.firstChild as HTMLElement
expect(uiSettingsDiv).toHaveAttribute("data-custom", "test-value")

32
webview-ui/src/components/ui/hooks/useDebouncedAction.ts Normal file
View file

@ -0,0 +1,32 @@
import { useCallback, useRef, useState } from "react"
export function useDebouncedAction(delay = 300) {
const [isProcessing, setIsProcessing] = useState(false)
const timeoutRef = useRef<NodeJS.Timeout | null>(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