Skip to content

Commit 1852ad1

Browse files
committed
fix: prevent confusing auto-approve checkbox states
- Add derived state to check if any auto-approve options are enabled - Disable master checkbox when no sub-options selected - Show 'None' when no options enabled regardless of checkbox state - Add automatic master checkbox toggling - Add tooltip explaining disabled state - Create custom hook useAutoApprovalState for shared logic - Add accessibility aria-labels - Update translations for all 17 supported languages - Add comprehensive unit tests Closes #2579
1 parent 88c4261 commit 1852ad1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+1092
-58
lines changed

webview-ui/src/components/chat/AutoApproveMenu.tsx

Lines changed: 96 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@ import { vscode } from "@src/utils/vscode"
66
import { useExtensionState } from "@src/context/ExtensionStateContext"
77
import { useAppTranslation } from "@src/i18n/TranslationContext"
88
import { AutoApproveToggle, AutoApproveSetting, autoApproveSettingsConfig } from "../settings/AutoApproveToggle"
9+
import { StandardTooltip } from "@src/components/ui"
10+
import { useAutoApprovalState } from "@src/hooks/useAutoApprovalState"
911

1012
interface AutoApproveMenuProps {
1113
style?: React.CSSProperties
1214
}
1315

1416
const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
1517
const [isExpanded, setIsExpanded] = useState(false)
18+
const [isMenuExpanded, setIsMenuExpanded] = useState(false)
1619

1720
const {
1821
autoApprovalEnabled,
@@ -43,10 +46,40 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
4346

4447
const { t } = useAppTranslation()
4548

49+
const toggles = useMemo(
50+
() => ({
51+
alwaysAllowReadOnly: alwaysAllowReadOnly,
52+
alwaysAllowWrite: alwaysAllowWrite,
53+
alwaysAllowExecute: alwaysAllowExecute,
54+
alwaysAllowBrowser: alwaysAllowBrowser,
55+
alwaysAllowMcp: alwaysAllowMcp,
56+
alwaysAllowModeSwitch: alwaysAllowModeSwitch,
57+
alwaysAllowSubtasks: alwaysAllowSubtasks,
58+
alwaysApproveResubmit: alwaysApproveResubmit,
59+
alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions,
60+
alwaysAllowUpdateTodoList: alwaysAllowUpdateTodoList,
61+
}),
62+
[
63+
alwaysAllowReadOnly,
64+
alwaysAllowWrite,
65+
alwaysAllowExecute,
66+
alwaysAllowBrowser,
67+
alwaysAllowMcp,
68+
alwaysAllowModeSwitch,
69+
alwaysAllowSubtasks,
70+
alwaysApproveResubmit,
71+
alwaysAllowFollowupQuestions,
72+
alwaysAllowUpdateTodoList,
73+
],
74+
)
75+
76+
const { hasEnabledOptions, effectiveAutoApprovalEnabled } = useAutoApprovalState(toggles, autoApprovalEnabled)
77+
4678
const onAutoApproveToggle = useCallback(
4779
(key: AutoApproveSetting, value: boolean) => {
4880
vscode.postMessage({ type: key, bool: value })
4981

82+
// Update the specific toggle state
5083
switch (key) {
5184
case "alwaysAllowReadOnly":
5285
setAlwaysAllowReadOnly(value)
@@ -79,8 +112,30 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
79112
setAlwaysAllowUpdateTodoList(value)
80113
break
81114
}
115+
116+
// Check if we need to update the master auto-approval state
117+
// Create a new toggles state with the updated value
118+
const updatedToggles = {
119+
...toggles,
120+
[key]: value,
121+
}
122+
123+
const willHaveEnabledOptions = Object.values(updatedToggles).some((v) => !!v)
124+
125+
// If enabling the first option, enable master auto-approval
126+
if (value && !hasEnabledOptions && willHaveEnabledOptions) {
127+
setAutoApprovalEnabled(true)
128+
vscode.postMessage({ type: "autoApprovalEnabled", bool: true })
129+
}
130+
// If disabling the last option, disable master auto-approval
131+
else if (!value && hasEnabledOptions && !willHaveEnabledOptions) {
132+
setAutoApprovalEnabled(false)
133+
vscode.postMessage({ type: "autoApprovalEnabled", bool: false })
134+
}
82135
},
83136
[
137+
toggles,
138+
hasEnabledOptions,
84139
setAlwaysAllowReadOnly,
85140
setAlwaysAllowWrite,
86141
setAlwaysAllowExecute,
@@ -91,43 +146,36 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
91146
setAlwaysApproveResubmit,
92147
setAlwaysAllowFollowupQuestions,
93148
setAlwaysAllowUpdateTodoList,
149+
setAutoApprovalEnabled,
94150
],
95151
)
96152

97-
const toggleExpanded = useCallback(() => setIsExpanded((prev) => !prev), [])
153+
const toggleExpanded = useCallback(() => {
154+
setIsExpanded((prev) => {
155+
const newExpanded = !prev
156+
setIsMenuExpanded(newExpanded)
157+
return newExpanded
158+
})
159+
}, [])
98160

99-
const toggles = useMemo(
100-
() => ({
101-
alwaysAllowReadOnly: alwaysAllowReadOnly,
102-
alwaysAllowWrite: alwaysAllowWrite,
103-
alwaysAllowExecute: alwaysAllowExecute,
104-
alwaysAllowBrowser: alwaysAllowBrowser,
105-
alwaysAllowMcp: alwaysAllowMcp,
106-
alwaysAllowModeSwitch: alwaysAllowModeSwitch,
107-
alwaysAllowSubtasks: alwaysAllowSubtasks,
108-
alwaysApproveResubmit: alwaysApproveResubmit,
109-
alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions,
110-
alwaysAllowUpdateTodoList: alwaysAllowUpdateTodoList,
111-
}),
112-
[
113-
alwaysAllowReadOnly,
114-
alwaysAllowWrite,
115-
alwaysAllowExecute,
116-
alwaysAllowBrowser,
117-
alwaysAllowMcp,
118-
alwaysAllowModeSwitch,
119-
alwaysAllowSubtasks,
120-
alwaysApproveResubmit,
121-
alwaysAllowFollowupQuestions,
122-
alwaysAllowUpdateTodoList,
123-
],
124-
)
161+
// Disable main checkbox while menu is open or no options selected
162+
const isCheckboxDisabled = useMemo(() => {
163+
return !hasEnabledOptions || isMenuExpanded
164+
}, [hasEnabledOptions, isMenuExpanded])
125165

126166
const enabledActionsList = Object.entries(toggles)
127167
.filter(([_key, value]) => !!value)
128168
.map(([key]) => t(autoApproveSettingsConfig[key as AutoApproveSetting].labelKey))
129169
.join(", ")
130170

171+
// Update displayed text logic
172+
const displayText = useMemo(() => {
173+
if (!effectiveAutoApprovalEnabled || !hasEnabledOptions) {
174+
return t("chat:autoApprove.none")
175+
}
176+
return enabledActionsList || t("chat:autoApprove.none")
177+
}, [effectiveAutoApprovalEnabled, hasEnabledOptions, enabledActionsList, t])
178+
131179
const handleOpenSettings = useCallback(
132180
() =>
133181
window.postMessage({ type: "action", action: "settingsButtonClicked", values: { section: "autoApprove" } }),
@@ -155,14 +203,26 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
155203
}}
156204
onClick={toggleExpanded}>
157205
<div onClick={(e) => e.stopPropagation()}>
158-
<VSCodeCheckbox
159-
checked={autoApprovalEnabled ?? false}
160-
onChange={() => {
161-
const newValue = !(autoApprovalEnabled ?? false)
162-
setAutoApprovalEnabled(newValue)
163-
vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue })
164-
}}
165-
/>
206+
<StandardTooltip content={!hasEnabledOptions ? t("chat:autoApprove.selectOptionsFirst") : ""}>
207+
<VSCodeCheckbox
208+
checked={effectiveAutoApprovalEnabled}
209+
disabled={isCheckboxDisabled}
210+
aria-label={
211+
hasEnabledOptions
212+
? t("chat:autoApprove.toggleAriaLabel")
213+
: t("chat:autoApprove.disabledAriaLabel")
214+
}
215+
onChange={() => {
216+
if (!hasEnabledOptions) {
217+
// Show a message or do nothing
218+
return
219+
}
220+
const newValue = !(autoApprovalEnabled ?? false)
221+
setAutoApprovalEnabled(newValue)
222+
vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue })
223+
}}
224+
/>
225+
</StandardTooltip>
166226
</div>
167227
<div
168228
style={{
@@ -188,7 +248,7 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
188248
flex: 1,
189249
minWidth: 0,
190250
}}>
191-
{enabledActionsList || t("chat:autoApprove.none")}
251+
{displayText}
192252
</span>
193253
<span
194254
className={`codicon codicon-chevron-${isExpanded ? "down" : "right"}`}

webview-ui/src/components/chat/ChatView.tsx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,10 +961,27 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
961961

962962
const isAutoApproved = useCallback(
963963
(message: ClineMessage | undefined) => {
964+
// First check if auto-approval is enabled AND we have at least one permission
964965
if (!autoApprovalEnabled || !message || message.type !== "ask") {
965966
return false
966967
}
967968

969+
// Check if ANY auto-approve option is enabled
970+
const hasAnyAutoApproveEnabled =
971+
alwaysAllowReadOnly ||
972+
alwaysAllowWrite ||
973+
alwaysAllowBrowser ||
974+
alwaysAllowExecute ||
975+
alwaysAllowMcp ||
976+
alwaysAllowModeSwitch ||
977+
alwaysAllowSubtasks ||
978+
alwaysAllowFollowupQuestions ||
979+
alwaysAllowUpdateTodoList
980+
981+
if (!hasAnyAutoApproveEnabled) {
982+
return false
983+
}
984+
968985
if (message.ask === "followup") {
969986
return alwaysAllowFollowupQuestions
970987
}

0 commit comments

Comments
 (0)