diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index f680f3e5fe..d377e02fee 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -18,12 +18,13 @@ import { Database, SquareTerminal, FlaskConical, - AlertTriangle, Globe, Info, MessageSquare, LucideIcon, + Circle, } from "lucide-react" +import { useDebounce } from "react-use" import type { ProviderSettings, ExperimentId, TelemetrySetting } from "@roo-code/types" @@ -31,22 +32,7 @@ import { vscode } from "@src/utils/vscode" import { cn } from "@src/lib/utils" import { useAppTranslation } from "@src/i18n/TranslationContext" import { ExtensionStateContextType, useExtensionState } from "@src/context/ExtensionStateContext" -import { - AlertDialog, - AlertDialogContent, - AlertDialogTitle, - AlertDialogDescription, - AlertDialogCancel, - AlertDialogAction, - AlertDialogHeader, - AlertDialogFooter, - Button, - Tooltip, - TooltipContent, - TooltipProvider, - TooltipTrigger, - StandardTooltip, -} from "@src/components/ui" +import { Button, Tooltip, TooltipContent, TooltipProvider, TooltipTrigger, StandardTooltip } from "@src/components/ui" import { Tab, TabContent, TabHeader, TabList, TabTrigger } from "../common/Tab" import { SetCachedStateField, SetExperimentEnabled } from "./types" @@ -103,17 +89,16 @@ const SettingsView = forwardRef(({ onDone, t const extensionState = useExtensionState() const { currentApiConfigName, listApiConfigMeta, uriScheme, settingsImportedAt } = extensionState - const [isDiscardDialogShow, setDiscardDialogShow] = useState(false) - const [isChangeDetected, setChangeDetected] = useState(false) const [errorMessage, setErrorMessage] = useState(undefined) const [activeTab, setActiveTab] = useState( targetSection && sectionNames.includes(targetSection as SectionName) ? (targetSection as SectionName) : "providers", ) + const [saveStatus, setSaveStatus] = useState<"idle" | "saving" | "saved" | "error">("idle") + const [saveError, setSaveError] = useState(undefined) const prevApiConfigName = useRef(currentApiConfigName) - const confirmDialogHandler = useRef<() => void>() const [cachedState, setCachedState] = useState(extensionState) @@ -196,14 +181,14 @@ const SettingsView = forwardRef(({ onDone, t setCachedState((prevCachedState) => ({ ...prevCachedState, ...extensionState })) prevApiConfigName.current = currentApiConfigName - setChangeDetected(false) - }, [currentApiConfigName, extensionState, isChangeDetected]) + setSaveStatus("idle") + }, [currentApiConfigName, extensionState]) // Bust the cache when settings are imported. useEffect(() => { if (settingsImportedAt) { setCachedState((prevCachedState) => ({ ...prevCachedState, ...extensionState })) - setChangeDetected(false) + setSaveStatus("idle") } }, [settingsImportedAt, extensionState]) @@ -213,7 +198,7 @@ const SettingsView = forwardRef(({ onDone, t return prevState } - setChangeDetected(true) + setSaveStatus("idle") // Reset to idle when changes are made return { ...prevState, [field]: value } }) }, []) @@ -232,7 +217,7 @@ const SettingsView = forwardRef(({ onDone, t const isInitialSync = !isUserAction && previousValue === undefined && value !== undefined if (!isInitialSync) { - setChangeDetected(true) + setSaveStatus("idle") // Reset to idle when changes are made } return { ...prevState, apiConfiguration: { ...prevState.apiConfiguration, [field]: value } } }) @@ -246,7 +231,7 @@ const SettingsView = forwardRef(({ onDone, t return prevState } - setChangeDetected(true) + setSaveStatus("idle") // Reset to idle when changes are made return { ...prevState, experiments: { ...prevState.experiments, [id]: enabled } } }) }, []) @@ -257,21 +242,21 @@ const SettingsView = forwardRef(({ onDone, t return prevState } - setChangeDetected(true) + setSaveStatus("idle") // Reset to idle when changes are made return { ...prevState, telemetrySetting: setting } }) }, []) const setOpenRouterImageApiKey = useCallback((apiKey: string) => { setCachedState((prevState) => { - setChangeDetected(true) + setSaveStatus("idle") // Reset to idle when changes are made return { ...prevState, openRouterImageApiKey: apiKey } }) }, []) const setImageGenerationSelectedModel = useCallback((model: string) => { setCachedState((prevState) => { - setChangeDetected(true) + setSaveStatus("idle") // Reset to idle when changes are made return { ...prevState, openRouterImageGenerationSelectedModel: model } }) }, []) @@ -282,15 +267,24 @@ const SettingsView = forwardRef(({ onDone, t return prevState } - setChangeDetected(true) + setSaveStatus("idle") // Reset to idle when changes are made return { ...prevState, customSupportPrompts: prompts } }) }, []) const isSettingValid = !errorMessage - const handleSubmit = () => { - if (isSettingValid) { + const performSave = useCallback(() => { + if (!isSettingValid) { + setSaveStatus("error") + setSaveError(errorMessage) + return + } + + setSaveStatus("saving") + setSaveError(undefined) + + try { vscode.postMessage({ type: "language", text: language }) vscode.postMessage({ type: "alwaysAllowReadOnly", bool: alwaysAllowReadOnly }) vscode.postMessage({ @@ -364,44 +358,126 @@ const SettingsView = forwardRef(({ onDone, t type: "openRouterImageGenerationSelectedModel", text: openRouterImageGenerationSelectedModel, }) - setChangeDetected(false) + + setSaveStatus("saved") + // Reset to idle after showing saved status for a bit + setTimeout(() => { + setSaveStatus("idle") + }, 2000) + } catch (error) { + setSaveStatus("error") + setSaveError(error instanceof Error ? error.message : "Failed to save settings") } - } + }, [ + isSettingValid, + errorMessage, + language, + alwaysAllowReadOnly, + alwaysAllowReadOnlyOutsideWorkspace, + alwaysAllowWrite, + alwaysAllowWriteOutsideWorkspace, + alwaysAllowWriteProtected, + alwaysAllowExecute, + alwaysAllowBrowser, + alwaysAllowMcp, + allowedCommands, + deniedCommands, + allowedMaxRequests, + allowedMaxCost, + autoCondenseContext, + autoCondenseContextPercent, + browserToolEnabled, + soundEnabled, + ttsEnabled, + ttsSpeed, + soundVolume, + diffEnabled, + enableCheckpoints, + browserViewportSize, + remoteBrowserHost, + remoteBrowserEnabled, + fuzzyMatchThreshold, + writeDelayMs, + screenshotQuality, + terminalOutputLineLimit, + terminalOutputCharacterLimit, + terminalShellIntegrationTimeout, + terminalShellIntegrationDisabled, + terminalCommandDelay, + terminalPowershellCounter, + terminalZshClearEolMark, + terminalZshOhMy, + terminalZshP10k, + terminalZdotdir, + terminalCompressProgressBar, + mcpEnabled, + alwaysApproveResubmit, + requestDelaySeconds, + maxOpenTabsContext, + maxWorkspaceFiles, + showRooIgnoredFiles, + maxReadFileLine, + maxImageFileSize, + maxTotalImageSize, + cachedState.maxConcurrentFileReads, + includeDiagnosticMessages, + maxDiagnosticMessages, + currentApiConfigName, + experiments, + alwaysAllowModeSwitch, + alwaysAllowSubtasks, + alwaysAllowFollowupQuestions, + alwaysAllowUpdateTodoList, + followupAutoApproveTimeoutMs, + condensingApiConfigId, + customCondensingPrompt, + customSupportPrompts, + includeTaskHistoryInEnhance, + apiConfiguration, + telemetrySetting, + profileThresholds, + openRouterImageApiKey, + openRouterImageGenerationSelectedModel, + ]) + + // Auto-save with debouncing + useDebounce( + () => { + // Only auto-save if we have changes (status is idle) and settings are valid + if (saveStatus === "idle" && isSettingValid) { + performSave() + } + }, + 500, // 500ms debounce + [saveStatus, isSettingValid, performSave], + ) + // Simplified checkUnsaveChanges - just execute the action immediately since we auto-save const checkUnsaveChanges = useCallback( (then: () => void) => { - if (isChangeDetected) { - confirmDialogHandler.current = then - setDiscardDialogShow(true) + // If we're currently saving, wait for it to complete + if (saveStatus === "saving") { + // Wait a bit for save to complete, then execute + setTimeout(then, 100) } else { then() } }, - [isChangeDetected], + [saveStatus], ) useImperativeHandle(ref, () => ({ checkUnsaveChanges }), [checkUnsaveChanges]) - const onConfirmDialogResult = useCallback( - (confirm: boolean) => { - if (confirm) { - // Discard changes: Reset state and flag - setCachedState(extensionState) // Revert to original state - setChangeDetected(false) // Reset change flag - confirmDialogHandler.current?.() // Execute the pending action (e.g., tab switch) - } - // If confirm is false (Cancel), do nothing, dialog closes automatically - }, - [extensionState], // Depend on extensionState to get the latest original state - ) - - // Handle tab changes with unsaved changes check + // Handle tab changes - no need to check for unsaved changes anymore const handleTabChange = useCallback( (newTab: SectionName) => { - // Directly switch tab without checking for unsaved changes + // Prevent tab switching while saving + if (saveStatus === "saving") { + return + } setActiveTab(newTab) }, - [], // No dependency on isChangeDetected needed anymore + [saveStatus], ) // Store direct DOM element refs for each tab @@ -494,26 +570,32 @@ const SettingsView = forwardRef(({ onDone, t

{t("settings:header.title")}

-
- - - +
+ {/* Status indicator */} +
+ {saveStatus === "saving" && ( + + + + )} + {saveStatus === "saved" && ( + + + + )} + {saveStatus === "error" && ( + + + + )} + {saveStatus === "idle" && errorMessage && ( + + + + )} +
- @@ -594,11 +676,13 @@ const SettingsView = forwardRef(({ onDone, t - checkUnsaveChanges(() => - vscode.postMessage({ type: "loadApiConfiguration", text: configName }), - ) - } + onSelectConfig={(configName: string) => { + // Prevent profile switching while saving + if (saveStatus === "saving") { + return + } + vscode.postMessage({ type: "loadApiConfiguration", text: configName }) + }} onDeleteConfig={(configName: string) => vscode.postMessage({ type: "deleteApiConfiguration", text: configName }) } @@ -764,28 +848,6 @@ const SettingsView = forwardRef(({ onDone, t )}
- - - - - - - {t("settings:unsavedChangesDialog.title")} - - - {t("settings:unsavedChangesDialog.description")} - - - - onConfirmDialogResult(false)}> - {t("settings:unsavedChangesDialog.cancelButton")} - - onConfirmDialogResult(true)}> - {t("settings:unsavedChangesDialog.discardButton")} - - - - ) }) diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx index 694ff174a7..fb302a1193 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx @@ -292,7 +292,7 @@ describe("SettingsView - Sound Settings", () => { expect(screen.queryByTestId("sound-volume-slider")).not.toBeInTheDocument() }) - it("toggles tts setting and sends message to VSCode", () => { + it("toggles tts setting and auto-saves to VSCode", async () => { // Render once and get the activateTab helper const { activateTab } = renderSettingsView() @@ -305,9 +305,8 @@ describe("SettingsView - Sound Settings", () => { fireEvent.click(ttsCheckbox) expect(ttsCheckbox).toBeChecked() - // Click Save to save settings - const saveButton = screen.getByTestId("save-button") - fireEvent.click(saveButton) + // Wait for auto-save debounce (500ms) + await new Promise((resolve) => setTimeout(resolve, 600)) expect(vscode.postMessage).toHaveBeenCalledWith( expect.objectContaining({ @@ -317,7 +316,7 @@ describe("SettingsView - Sound Settings", () => { ) }) - it("toggles sound setting and sends message to VSCode", () => { + it("toggles sound setting and auto-saves to VSCode", async () => { // Render once and get the activateTab helper const { activateTab } = renderSettingsView() @@ -330,9 +329,8 @@ describe("SettingsView - Sound Settings", () => { fireEvent.click(soundCheckbox) expect(soundCheckbox).toBeChecked() - // Click Save to save settings - const saveButton = screen.getByTestId("save-button") - fireEvent.click(saveButton) + // Wait for auto-save debounce (500ms) + await new Promise((resolve) => setTimeout(resolve, 600)) expect(vscode.postMessage).toHaveBeenCalledWith( expect.objectContaining({ @@ -376,7 +374,7 @@ describe("SettingsView - Sound Settings", () => { expect(volumeSlider).toHaveValue("0.5") }) - it("updates speed and sends message to VSCode when slider changes", () => { + it("updates speed and auto-saves to VSCode when slider changes", async () => { // Render once and get the activateTab helper const { activateTab } = renderSettingsView() @@ -391,9 +389,8 @@ describe("SettingsView - Sound Settings", () => { const speedSlider = screen.getByTestId("tts-speed-slider") fireEvent.change(speedSlider, { target: { value: "0.75" } }) - // Click Save to save settings - const saveButton = screen.getByTestId("save-button") - fireEvent.click(saveButton) + // Wait for auto-save debounce (500ms) + await new Promise((resolve) => setTimeout(resolve, 600)) // Verify message sent to VSCode expect(vscode.postMessage).toHaveBeenCalledWith({ @@ -402,7 +399,7 @@ describe("SettingsView - Sound Settings", () => { }) }) - it("updates volume and sends message to VSCode when slider changes", () => { + it("updates volume and auto-saves to VSCode when slider changes", async () => { // Render once and get the activateTab helper const { activateTab } = renderSettingsView() @@ -417,9 +414,8 @@ describe("SettingsView - Sound Settings", () => { const volumeSlider = screen.getByTestId("sound-volume-slider") fireEvent.change(volumeSlider, { target: { value: "0.75" } }) - // Click Save to save settings - use getAllByTestId to handle multiple elements - const saveButtons = screen.getAllByTestId("save-button") - fireEvent.click(saveButtons[0]) + // Wait for auto-save debounce (500ms) + await new Promise((resolve) => setTimeout(resolve, 600)) // Verify message sent to VSCode expect(vscode.postMessage).toHaveBeenCalledWith({ @@ -536,14 +532,14 @@ describe("SettingsView - Allowed Commands", () => { expect(screen.getByTestId("api-config-management")).toBeInTheDocument() }) - it("shows unsaved changes dialog when clicking Done with unsaved changes", () => { + it("does not show unsaved changes dialog with auto-save", () => { // Render once and get the activateTab helper - const { activateTab } = renderSettingsView() + const { activateTab, onDone } = renderSettingsView() // Activate the notifications tab activateTab("notifications") - // Make a change to create unsaved changes + // Make a change (which will auto-save) const soundCheckbox = screen.getByTestId("sound-enabled-checkbox") fireEvent.click(soundCheckbox) @@ -551,8 +547,10 @@ describe("SettingsView - Allowed Commands", () => { const doneButton = screen.getByText("settings:common.done") fireEvent.click(doneButton) - // Check that unsaved changes dialog is shown - expect(screen.getByText("settings:unsavedChangesDialog.title")).toBeInTheDocument() + // Check that onDone was called directly (no dialog) + expect(onDone).toHaveBeenCalled() + // Check that unsaved changes dialog is NOT shown + expect(screen.queryByText("settings:unsavedChangesDialog.title")).not.toBeInTheDocument() }) it("renders with targetSection prop", () => { @@ -607,7 +605,7 @@ describe("SettingsView - Duplicate Commands", () => { expect(commands).toHaveLength(1) }) - it("saves allowed commands when clicking Save", () => { + it("auto-saves allowed commands when added", async () => { // Render once and get the activateTab helper const { activateTab } = renderSettingsView() @@ -624,9 +622,8 @@ describe("SettingsView - Duplicate Commands", () => { const addButton = screen.getByTestId("add-command-button") fireEvent.click(addButton) - // Click Save - use getAllByTestId to handle multiple elements - const saveButtons = screen.getAllByTestId("save-button") - fireEvent.click(saveButtons[0]) + // Wait for auto-save debounce (500ms) + await new Promise((resolve) => setTimeout(resolve, 600)) // Verify VSCode messages were sent expect(vscode.postMessage).toHaveBeenCalledWith( diff --git a/webview-ui/src/i18n/locales/en/settings.json b/webview-ui/src/i18n/locales/en/settings.json index 7df37a0270..b2e981199f 100644 --- a/webview-ui/src/i18n/locales/en/settings.json +++ b/webview-ui/src/i18n/locales/en/settings.json @@ -12,7 +12,12 @@ "title": "Settings", "saveButtonTooltip": "Save changes", "nothingChangedTooltip": "Nothing changed", - "doneButtonTooltip": "Discard unsaved changes and close settings panel" + "doneButtonTooltip": "Close settings panel" + }, + "status": { + "saving": "Saving changes...", + "saved": "All changes saved", + "error": "Failed to save changes" }, "unsavedChangesDialog": { "title": "Unsaved Changes",