Skip to content

feat: replace auto-approve text labels with icons in collapsed view #6500

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.

Sign up for GitHub

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 4 commits into from
+138 −23
Closed

feat: replace auto-approve text labels with icons in collapsed view #6500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
228c9c7
feat: replace auto-approve text labels with icons in collapsed view
roomote Jul 31, 2025
cfa2081
feat: add tooltips to auto-approve icons in collapsed view
roomote Aug 5, 2025
782174e
feat(webview-ui): show all auto-approve icons in collapsed view and h…
Aug 19, 2025
e5b41c8
feat: make collapsed auto-approve icons clickable to toggle options d…
Aug 19, 2025
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
54 changes: 40 additions & 14 deletions webview-ui/src/components/chat/AutoApproveMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,18 +129,8 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
setIsExpanded((prev) => !prev)
}, [])

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

// Update displayed text logic
const displayText = useMemo(() => {
if (!effectiveAutoApprovalEnabled || !hasEnabledOptions) {
return t("chat:autoApprove.none")
}
return enabledActionsList || t("chat:autoApprove.none")
}, [effectiveAutoApprovalEnabled, hasEnabledOptions, enabledActionsList, t])
// Get all icons for display and highlight based on toggle state
const allConfigs = useMemo(() => Object.values(autoApproveSettingsConfig), [])

const handleOpenSettings = useCallback(
() =>
Expand Down Expand Up @@ -213,8 +203,44 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
whiteSpace: "nowrap",
flex: 1,
minWidth: 0,
}}>
{displayText}
display: "flex",
alignItems: "center",
gap: "6px",
}}
onClick={(e) => e.stopPropagation()}>
Copy link

Copilot AI Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onClick handler e.stopPropagation() on the span element is redundant since each individual button already handles its own click events. This could be removed to simplify the code.

Suggested change
onClick={(e) => e.stopPropagation()}>
>

Copilot uses AI. Check for mistakes.
{allConfigs.map(({ key, icon, labelKey }) => {
const isEnabled = !!toggles[key]
return (
<StandardTooltip key={key} content={t(labelKey)}>
<button
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding an accessible aria-label to the icon button (e.g., aria-label={t(labelKey)}) to ensure screen reader users can understand its purpose.

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

className={`codicon codicon-${icon}`}
data-active={isEnabled ? "true" : "false"}
onClick={() => onAutoApproveToggle(key, !isEnabled)}
style={{
fontSize: "14px",
flexShrink: 0,
opacity: isEnabled ? 1 : 0.5,
color: isEnabled
? "var(--vscode-foreground)"
: "var(--vscode-descriptionForeground)",
background: "transparent",
border: "none",
cursor: "pointer",
padding: "2px",
borderRadius: "3px",
transition: "background-color 0.1s",
}}
onMouseEnter={(e) => {
e.currentTarget.style.backgroundColor =
"var(--vscode-toolbar-hoverBackground)"
}}
onMouseLeave={(e) => {
e.currentTarget.style.backgroundColor = "transparent"
}}
Copy link

Copilot AI Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The inline mouse event handlers for hover effects could be extracted to improve readability and maintainability. Consider using CSS hover pseudo-selectors or extracting these into named functions.

Suggested change
}}

Copilot uses AI. Check for mistakes.
/>
</StandardTooltip>
)
})}
</span>
<span
className={`codicon codicon-chevron-${isExpanded ? "down" : "right"}`}
Expand Down
107 changes: 98 additions & 9 deletions webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { render, fireEvent, screen, waitFor } from "@/utils/test-utils"
import { useExtensionState } from "@src/context/ExtensionStateContext"
import { vscode } from "@src/utils/vscode"
import AutoApproveMenu from "../AutoApproveMenu"
import userEvent from "@testing-library/user-event"

// Mock vscode API
vi.mock("@src/utils/vscode", () => ({
Expand Down Expand Up @@ -84,7 +85,7 @@ describe("AutoApproveMenu", () => {
})

describe("Master checkbox behavior", () => {
it("should show 'None selected' when no sub-options are selected", () => {
it("should show all icons with none highlighted when no sub-options are selected", () => {
;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue({
...defaultExtensionState,
autoApprovalEnabled: false,
Expand All @@ -97,11 +98,23 @@ describe("AutoApproveMenu", () => {

render(<AutoApproveMenu />)

// Check that the text shows "None selected"
expect(screen.getByText("None selected")).toBeInTheDocument()
const container = screen.getByText("Auto-approve").parentElement?.parentElement

// All primary icons are rendered
expect(container?.querySelector(".codicon-eye")).toBeInTheDocument()
expect(container?.querySelector(".codicon-edit")).toBeInTheDocument()
expect(container?.querySelector(".codicon-terminal")).toBeInTheDocument()

// None are active
expect(container?.querySelector(".codicon-eye")?.getAttribute("data-active")).toBe("false")
expect(container?.querySelector(".codicon-edit")?.getAttribute("data-active")).toBe("false")
expect(container?.querySelector(".codicon-terminal")?.getAttribute("data-active")).toBe("false")

// "None selected" helper text is not shown anymore
expect(screen.queryByText("None selected")).not.toBeInTheDocument()
})

it("should show enabled options when sub-options are selected", () => {
it("should highlight the enabled icons when sub-options are selected", () => {
;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue({
...defaultExtensionState,
autoApprovalEnabled: true,
Expand All @@ -111,8 +124,15 @@ describe("AutoApproveMenu", () => {

render(<AutoApproveMenu />)

// Check that the text shows the enabled option
expect(screen.getByText("Read-only operations")).toBeInTheDocument()
const container = screen.getByText("Auto-approve").parentElement?.parentElement as HTMLElement
const eyeIcon = container.querySelector(".codicon-eye") as HTMLElement
const editIcon = container.querySelector(".codicon-edit") as HTMLElement

expect(eyeIcon).toBeInTheDocument()
expect(editIcon).toBeInTheDocument()

expect(eyeIcon.getAttribute("data-active")).toBe("true")
expect(editIcon.getAttribute("data-active")).toBe("false")
})

it("should not allow toggling master checkbox when no options are selected", () => {
Expand Down Expand Up @@ -211,7 +231,62 @@ describe("AutoApproveMenu", () => {
})

describe("Complex scenarios", () => {
it("should display multiple enabled options in summary text", () => {
it("should highlight multiple enabled icons in collapsed view", () => {
;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue({
...defaultExtensionState,
autoApprovalEnabled: true,
alwaysAllowReadOnly: true,
alwaysAllowWrite: true,
alwaysAllowExecute: true,
})

render(<AutoApproveMenu />)

const container = screen.getByText("Auto-approve").parentElement?.parentElement as HTMLElement
expect(container.querySelector("button.codicon-eye")?.getAttribute("data-active")).toBe("true") // Read
expect(container.querySelector("button.codicon-edit")?.getAttribute("data-active")).toBe("true") // Write
expect(container.querySelector("button.codicon-terminal")?.getAttribute("data-active")).toBe("true") // Execute
})

it("should toggle options when clicking icons in collapsed view", () => {
const mockSetAlwaysAllowReadOnly = vi.fn()
const mockSetAlwaysAllowWrite = vi.fn()

;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue({
...defaultExtensionState,
autoApprovalEnabled: false,
alwaysAllowReadOnly: false,
alwaysAllowWrite: false,
setAlwaysAllowReadOnly: mockSetAlwaysAllowReadOnly,
setAlwaysAllowWrite: mockSetAlwaysAllowWrite,
})

render(<AutoApproveMenu />)

const container = screen.getByText("Auto-approve").parentElement?.parentElement as HTMLElement
const eyeButton = container.querySelector("button.codicon-eye") as HTMLElement
const editButton = container.querySelector("button.codicon-edit") as HTMLElement

// Click the read-only icon
fireEvent.click(eyeButton)
expect(mockPostMessage).toHaveBeenCalledWith({
type: "alwaysAllowReadOnly",
bool: true,
})
expect(mockSetAlwaysAllowReadOnly).toHaveBeenCalledWith(true)

// Click the write icon
fireEvent.click(editButton)
expect(mockPostMessage).toHaveBeenCalledWith({
type: "alwaysAllowWrite",
bool: true,
})
expect(mockSetAlwaysAllowWrite).toHaveBeenCalledWith(true)
})

it("should display tooltips on icon buttons in collapsed view", async () => {
const user = userEvent.setup()

;(useExtensionState as ReturnType<typeof vi.fn>).mockReturnValue({
...defaultExtensionState,
autoApprovalEnabled: true,
Expand All @@ -222,8 +297,22 @@ describe("AutoApproveMenu", () => {

render(<AutoApproveMenu />)

// Should show all enabled options in the summary
expect(screen.getByText("Read-only operations, Write operations, Execute operations")).toBeInTheDocument()
// Find the icon buttons
const container = screen.getByText("Auto-approve").parentElement?.parentElement
const eyeButton = container?.querySelector("button.codicon-eye") as HTMLElement
const editButton = container?.querySelector("button.codicon-edit") as HTMLElement
const terminalButton = container?.querySelector("button.codicon-terminal") as HTMLElement

// Verify icon buttons are present
expect(eyeButton).toBeInTheDocument()
expect(editButton).toBeInTheDocument()
expect(terminalButton).toBeInTheDocument()

// Test read-only icon tooltip
await user.hover(eyeButton)
await waitFor(() => {
expect(screen.getByRole("tooltip")).toHaveTextContent("Read-only operations")
})
})

it("should handle enabling first option when none selected", async () => {
Expand Down
Loading