Skip to content

fix: keep auto-approve checkbox responsive when menu is expanded #6062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions webview-ui/src/components/chat/AutoApproveMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,11 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
setIsExpanded((prev) => !prev)
}, [])

// Disable main checkbox while menu is open or no options selected
// Disable main checkbox only when no options are selected
// Allow toggling even when menu is expanded for better UX and safety
const isCheckboxDisabled = useMemo(() => {
return !hasEnabledOptions || isExpanded
}, [hasEnabledOptions, isExpanded])
return !hasEnabledOptions
}, [hasEnabledOptions])

const enabledActionsList = Object.entries(toggles)
.filter(([_key, value]) => !!value)
Expand Down Expand Up @@ -184,14 +185,14 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
? t("chat:autoApprove.toggleAriaLabel")
: t("chat:autoApprove.disabledAriaLabel")
}
onChange={() => {
onChange={useCallback(() => {
if (hasEnabledOptions) {
const newValue = !(autoApprovalEnabled ?? false)
setAutoApprovalEnabled(newValue)
vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue })
}
// If no options enabled, do nothing
}}
}, [hasEnabledOptions, autoApprovalEnabled, setAutoApprovalEnabled])}
/>
</StandardTooltip>
</div>
Expand Down
100 changes: 100 additions & 0 deletions webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,35 @@ vi.mock("@src/utils/vscode", () => ({
// Mock ExtensionStateContext
vi.mock("@src/context/ExtensionStateContext")

// Mock VSCode webview UI toolkit
vi.mock("@vscode/webview-ui-toolkit/react", () => ({
VSCodeCheckbox: ({ children, checked, disabled, onChange, ...props }: any) => (
<label>
<input
type="checkbox"
role="checkbox"
checked={checked}
disabled={disabled}
{...props}
onChange={(e: any) => {
if (!disabled && onChange) {
onChange(e)
}
}}
/>
{children}
</label>
),
VSCodeLink: ({ children, onClick, ...props }: any) => (
<a onClick={onClick} {...props}>
{children}
</a>
),
VSCodeTextField: ({ value, onInput, ...props }: any) => (
<input type="text" value={value} onInput={onInput} {...props} />
),
}))

// Mock translation hook
vi.mock("@src/i18n/TranslationContext", () => ({
useAppTranslation: () => ({
Expand Down Expand Up @@ -303,5 +332,76 @@ describe("AutoApproveMenu", () => {
bool: false,
})
})

it("should keep master checkbox responsive when menu is expanded", async () => {
const mockSetAutoApprovalEnabled = vi.fn()

;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue({
...defaultExtensionState,
autoApprovalEnabled: true,
alwaysAllowReadOnly: true,
setAutoApprovalEnabled: mockSetAutoApprovalEnabled,
})

render(<AutoApproveMenu />)

// Expand the menu
const menuContainer = screen.getByText("Auto-approve").parentElement
fireEvent.click(menuContainer!)

// Wait for the menu to expand
await waitFor(() => {
expect(screen.getByTestId("always-allow-readonly-toggle")).toBeInTheDocument()
})

// The master checkbox should still be enabled and clickable
const masterCheckbox = screen.getByRole("checkbox")
expect(masterCheckbox).not.toBeDisabled()

// Click the master checkbox while menu is expanded
fireEvent.click(masterCheckbox)

// Should successfully toggle the master checkbox
expect(mockPostMessage).toHaveBeenCalledWith({
type: "autoApprovalEnabled",
bool: false,
})
expect(mockSetAutoApprovalEnabled).toHaveBeenCalledWith(false)
})

it("should not flicker when state changes rapidly", async () => {
const mockSetAutoApprovalEnabled = vi.fn()

// Start with enabled state
const enabledState = {
...defaultExtensionState,
autoApprovalEnabled: true,
alwaysAllowReadOnly: true,
setAutoApprovalEnabled: mockSetAutoApprovalEnabled,
}

;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue(enabledState)

const { rerender } = render(<AutoApproveMenu />)

const masterCheckbox = screen.getByRole("checkbox")
expect(masterCheckbox).toBeChecked()

// Simulate rapid state changes
for (let i = 0; i < 5; i++) {
const newState = {
...defaultExtensionState,
autoApprovalEnabled: i % 2 === 0,
alwaysAllowReadOnly: true,
setAutoApprovalEnabled: mockSetAutoApprovalEnabled,
}
;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue(newState)
rerender(<AutoApproveMenu />)
}

// Checkbox should reflect the final state without issues (i=4, so 4 % 2 === 0, which is true)
expect(masterCheckbox).toBeChecked()
expect(masterCheckbox).not.toBeDisabled()
})
})
})