From 38c5d8c08c15c4efa6e91a3aa8f7a83562c2e202 Mon Sep 17 00:00:00 2001
From: hannesrudolph <hrudolph@gmail.com>
Date: Mon, 14 Jul 2025 17:03:01 -0600
Subject: [PATCH] fix: address PR review comments

- Fix tooltip rendering to conditionally show only when needed
- Reuse useAutoApprovalState hook in ChatView to avoid code duplication
- Improve code quality and consistency across auto-approve components
---
 .../src/components/chat/AutoApproveMenu.tsx   | 26 ++++++-----
 webview-ui/src/components/chat/ChatView.tsx   | 45 +++++++++++++------
 .../settings/AutoApproveSettings.tsx          | 25 ++++++-----
 3 files changed, 62 insertions(+), 34 deletions(-)

diff --git a/webview-ui/src/components/chat/AutoApproveMenu.tsx b/webview-ui/src/components/chat/AutoApproveMenu.tsx
index f5c62df2af..4162b505d9 100644
--- a/webview-ui/src/components/chat/AutoApproveMenu.tsx
+++ b/webview-ui/src/components/chat/AutoApproveMenu.tsx
@@ -198,26 +198,30 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
 				}}
 				onClick={toggleExpanded}>
 				<div onClick={(e) => e.stopPropagation()}>
-					<StandardTooltip content={!hasEnabledOptions ? t("chat:autoApprove.selectOptionsFirst") : ""}>
+					{!hasEnabledOptions ? (
+						<StandardTooltip content={t("chat:autoApprove.selectOptionsFirst")}>
+							<VSCodeCheckbox
+								checked={effectiveAutoApprovalEnabled}
+								disabled={isCheckboxDisabled}
+								aria-label={t("chat:autoApprove.disabledAriaLabel")}
+								onChange={() => {
+									// Show a message or do nothing
+									return
+								}}
+							/>
+						</StandardTooltip>
+					) : (
 						<VSCodeCheckbox
 							checked={effectiveAutoApprovalEnabled}
 							disabled={isCheckboxDisabled}
-							aria-label={
-								hasEnabledOptions
-									? t("chat:autoApprove.toggleAriaLabel")
-									: t("chat:autoApprove.disabledAriaLabel")
-							}
+							aria-label={t("chat:autoApprove.toggleAriaLabel")}
 							onChange={() => {
-								if (!hasEnabledOptions) {
-									// Show a message or do nothing
-									return
-								}
 								const newValue = !(autoApprovalEnabled ?? false)
 								setAutoApprovalEnabled(newValue)
 								vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue })
 							}}
 						/>
-					</StandardTooltip>
+					)}
 				</div>
 				<div
 					style={{
diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx
index 584268f239..c48730fde7 100644
--- a/webview-ui/src/components/chat/ChatView.tsx
+++ b/webview-ui/src/components/chat/ChatView.tsx
@@ -38,6 +38,7 @@ import { useSelectedModel } from "@src/components/ui/hooks/useSelectedModel"
 import RooHero from "@src/components/welcome/RooHero"
 import RooTips from "@src/components/welcome/RooTips"
 import { StandardTooltip } from "@src/components/ui"
+import { useAutoApprovalState } from "@src/hooks/useAutoApprovalState"
 
 import TelemetryBanner from "../common/TelemetryBanner"
 import VersionIndicator from "../common/VersionIndicator"
@@ -959,6 +960,34 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 		[deniedCommands],
 	)
 
+	// Create toggles object for useAutoApprovalState hook
+	const autoApprovalToggles = useMemo(
+		() => ({
+			alwaysAllowReadOnly,
+			alwaysAllowWrite,
+			alwaysAllowExecute,
+			alwaysAllowBrowser,
+			alwaysAllowMcp,
+			alwaysAllowModeSwitch,
+			alwaysAllowSubtasks,
+			alwaysAllowFollowupQuestions,
+			alwaysAllowUpdateTodoList,
+		}),
+		[
+			alwaysAllowReadOnly,
+			alwaysAllowWrite,
+			alwaysAllowExecute,
+			alwaysAllowBrowser,
+			alwaysAllowMcp,
+			alwaysAllowModeSwitch,
+			alwaysAllowSubtasks,
+			alwaysAllowFollowupQuestions,
+			alwaysAllowUpdateTodoList,
+		],
+	)
+
+	const { hasEnabledOptions } = useAutoApprovalState(autoApprovalToggles, autoApprovalEnabled)
+
 	const isAutoApproved = useCallback(
 		(message: ClineMessage | undefined) => {
 			// First check if auto-approval is enabled AND we have at least one permission
@@ -966,19 +995,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 				return false
 			}
 
-			// Check if ANY auto-approve option is enabled
-			const hasAnyAutoApproveEnabled =
-				alwaysAllowReadOnly ||
-				alwaysAllowWrite ||
-				alwaysAllowBrowser ||
-				alwaysAllowExecute ||
-				alwaysAllowMcp ||
-				alwaysAllowModeSwitch ||
-				alwaysAllowSubtasks ||
-				alwaysAllowFollowupQuestions ||
-				alwaysAllowUpdateTodoList
-
-			if (!hasAnyAutoApproveEnabled) {
+			// Use the hook's result instead of duplicating the logic
+			if (!hasEnabledOptions) {
 				return false
 			}
 
@@ -1055,6 +1073,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 		},
 		[
 			autoApprovalEnabled,
+			hasEnabledOptions,
 			alwaysAllowBrowser,
 			alwaysAllowReadOnly,
 			alwaysAllowReadOnlyOutsideWorkspace,
diff --git a/webview-ui/src/components/settings/AutoApproveSettings.tsx b/webview-ui/src/components/settings/AutoApproveSettings.tsx
index 1e84575ce5..cb8a961e16 100644
--- a/webview-ui/src/components/settings/AutoApproveSettings.tsx
+++ b/webview-ui/src/components/settings/AutoApproveSettings.tsx
@@ -136,25 +136,30 @@ export const AutoApproveSettings = ({
 		<div {...props}>
 			<SectionHeader description={t("settings:autoApprove.description")}>
 				<div className="flex items-center gap-2">
-					<StandardTooltip content={!hasEnabledOptions ? t("settings:autoApprove.selectOptionsFirst") : ""}>
+					{!hasEnabledOptions ? (
+						<StandardTooltip content={t("settings:autoApprove.selectOptionsFirst")}>
+							<VSCodeCheckbox
+								checked={effectiveAutoApprovalEnabled}
+								disabled={!hasEnabledOptions}
+								aria-label={t("settings:autoApprove.disabledAriaLabel")}
+								onChange={() => {
+									// Do nothing when no options are enabled
+									return
+								}}
+							/>
+						</StandardTooltip>
+					) : (
 						<VSCodeCheckbox
 							checked={effectiveAutoApprovalEnabled}
 							disabled={!hasEnabledOptions}
-							aria-label={
-								hasEnabledOptions
-									? t("settings:autoApprove.toggleAriaLabel")
-									: t("settings:autoApprove.disabledAriaLabel")
-							}
+							aria-label={t("settings:autoApprove.toggleAriaLabel")}
 							onChange={() => {
-								if (!hasEnabledOptions) {
-									return
-								}
 								const newValue = !(autoApprovalEnabled ?? false)
 								setAutoApprovalEnabled(newValue)
 								vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue })
 							}}
 						/>
-					</StandardTooltip>
+					)}
 					<span className="codicon codicon-check w-4" />
 					<div>{t("settings:sections.autoApprove")}</div>
 				</div>