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
This commit is contained in:
hannesrudolph 2025-07-14 17:03:01 -06:00
parent e734ec7c87
commit 38c5d8c08c
3 changed files with 62 additions and 34 deletions

View file

@ -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={{

View file

@ -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,

View file

@ -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>