From 8bb5b615551a1f9ff49332162d8a9727e33db78b Mon Sep 17 00:00:00 2001 From: Toray Altas Date: Fri, 16 Jan 2026 23:20:23 -0500 Subject: [PATCH] fix: improve hooks settings layout and UI --- src/core/webview/ClineProvider.ts | 2 +- .../src/components/settings/HooksSettings.tsx | 163 +++++++++++------- .../src/components/settings/SettingsView.tsx | 4 +- .../settings/__tests__/HooksSettings.spec.tsx | 106 +++++++++++- webview-ui/src/i18n/locales/en/settings.json | 5 + 5 files changed, 207 insertions(+), 73 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 047bebbc15..d6ffa8b73a 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2254,7 +2254,7 @@ export class ClineProvider filePath: hook.filePath, event: hook.event, matcher: hook.matcher, - commandPreview: hook.command.length > 100 ? hook.command.substring(0, 97) + "..." : hook.command, + commandPreview: hook.command, enabled: (hook.enabled ?? true) && !(snapshot?.disabledHookIds?.has(hook.id) ?? false), source: hook.source, timeout: hook.timeout ?? 60, diff --git a/webview-ui/src/components/settings/HooksSettings.tsx b/webview-ui/src/components/settings/HooksSettings.tsx index 4282a7576c..0856ec3c40 100644 --- a/webview-ui/src/components/settings/HooksSettings.tsx +++ b/webview-ui/src/components/settings/HooksSettings.tsx @@ -1,5 +1,6 @@ import React, { useCallback, useEffect, useState } from "react" import { RefreshCw, FolderOpen, AlertTriangle, Clock, Zap, X } from "lucide-react" +import { VSCodePanels, VSCodePanelTab, VSCodePanelView } from "@vscode/webview-ui-toolkit/react" import { useAppTranslation } from "@src/i18n/TranslationContext" import { useExtensionState } from "@src/context/ExtensionStateContext" import { vscode } from "@src/utils/vscode" @@ -361,73 +362,103 @@ const HookItem: React.FC = ({ hook, onToggle }) => { {/* Expanded Content */} {isExpanded && ( -
- {/* Hook Details */} -
-
- - {t("settings:hooks.event")}: - - {hook.event} -
- {hook.matcher && ( -
- - {t("settings:hooks.matcher")}: - - - {hook.matcher} - -
- )} - {hook.description && ( -
- - {t("settings:hooks.description")}: - -

{hook.description}

-
- )} -
- - {t("settings:hooks.command")}: - - - {hook.commandPreview} - -
-
- {hook.shell && ( - - {t("settings:hooks.shell")}: {hook.shell} - - )} - - {t("settings:hooks.timeout")}: {hook.timeout}s - -
-
+
+ + {t("settings:hooks.tabs.config")} + {t("settings:hooks.tabs.command")} + + {t("settings:hooks.tabs.logs")} + {hookLogs.length > 0 && ({hookLogs.length})} + - {/* Logs Section */} -
-
- {t("settings:hooks.logs")} - {hookLogs.length > 0 && ( - ({hookLogs.length}) - )} -
- {hookLogs.length === 0 ? ( -
- {t("settings:hooks.noLogsForHook")} + +
+
+
+ + {t("settings:hooks.event")} + + + {hook.event} + +
+
+ + {t("settings:hooks.timeout")} + + {hook.timeout}s +
+
+ + {hook.matcher && ( +
+ + {t("settings:hooks.matcher")} + +
+
    + {hook.matcher + .split("|") + .map((m) => m.trim()) + .filter(Boolean) + .map((m, i) => ( +
  • {m}
    • + ))} +
+
+
+ )} + + {hook.shell && ( +
+ + {t("settings:hooks.shell")} + + + {hook.shell} + +
+ )} + + {hook.description && ( +
+ + {t("settings:hooks.description")} + +

{hook.description}

+
+ )}
- ) : ( -
- {hookLogs.map((record, index) => ( - - ))} + + + +
+
+ + {hook.commandPreview} + +
- )} -
+ + + +
+ {hookLogs.length === 0 ? ( +
+ {t("settings:hooks.noLogsForHook")} +
+ ) : ( +
+ {hookLogs.map((record, index) => ( + + ))} +
+ )} +
+ +
)}
@@ -484,7 +515,9 @@ const HookLogItem: React.FC = ({ record }) => { {status.label} {record.toolName && ( - + {record.toolName} )} diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 243556f693..0c4e2fb09d 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -99,6 +99,7 @@ export const sectionNames = [ "providers", "autoApprove", "slashCommands", + "hooks", "browser", "checkpoints", "notifications", @@ -106,7 +107,6 @@ export const sectionNames = [ "terminal", "modes", "mcp", - "hooks", "prompts", "ui", "experimental", @@ -528,12 +528,12 @@ const SettingsView = forwardRef(({ onDone, t { id: "mcp", icon: Server }, { id: "autoApprove", icon: CheckCheck }, { id: "slashCommands", icon: SquareSlash }, + { id: "hooks", icon: Zap }, { id: "browser", icon: SquareMousePointer }, { id: "checkpoints", icon: GitBranch }, { id: "notifications", icon: Bell }, { id: "contextManagement", icon: Database }, { id: "terminal", icon: SquareTerminal }, - { id: "hooks", icon: Zap }, { id: "prompts", icon: MessageSquare }, { id: "ui", icon: Glasses }, { id: "experimental", icon: FlaskConical }, diff --git a/webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx b/webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx index 5013e7b707..548eb1c5ac 100644 --- a/webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx @@ -5,6 +5,25 @@ import { vi, describe, it, expect, beforeEach } from "vitest" import { HooksSettings } from "../HooksSettings" import type { HookInfo, HookExecutionRecord, HooksState } from "@roo-code/types" +// Mock webview-ui-toolkit components +vi.mock("@vscode/webview-ui-toolkit/react", () => ({ + VSCodePanels: ({ children, ...props }: any) => ( +
+ {children} +
+ ), + VSCodePanelTab: ({ children, id, ...props }: any) => ( +
+ {children} +
+ ), + VSCodePanelView: ({ children, id, ...props }: any) => ( +
+ {children} +
+ ), +})) + // Mock vscode utilities vi.mock("@src/utils/vscode", () => ({ vscode: { @@ -147,9 +166,16 @@ describe("HooksSettings", () => { const hookHeader = screen.getByText(mockHook.id).closest("div") fireEvent.click(hookHeader!) - // Hook details should now be visible + // Should show tabs + expect(screen.getByTestId("tab-config")).toBeInTheDocument() + expect(screen.getByTestId("tab-command")).toBeInTheDocument() + expect(screen.getByTestId("tab-logs")).toBeInTheDocument() + + // Check Config tab content (visible by default usually or we can check panels exist) expect(screen.getByText(mockHook.event)).toBeInTheDocument() expect(screen.getByText(mockHook.matcher!)).toBeInTheDocument() + + // Check Command tab content expect(screen.getByText(mockHook.commandPreview)).toBeInTheDocument() // Click to collapse @@ -159,7 +185,7 @@ describe("HooksSettings", () => { expect(screen.queryByText(mockHook.event)).not.toBeInTheDocument() }) - it("shows per-hook logs in expanded view", () => { + it("shows per-hook logs in Logs tab", () => { const mockHook: HookInfo = { id: "hook-1", event: "before_execute_command", @@ -192,12 +218,82 @@ describe("HooksSettings", () => { const hookHeader = screen.getByText(mockHook.id).closest("div") fireEvent.click(hookHeader!) - // Logs section should be visible - expect(screen.getByText("settings:hooks.logs")).toBeInTheDocument() + // Find Logs tab panel content + expect(screen.getByTestId("panel-view-logs")).toBeInTheDocument() expect(screen.getByText(mockRecord.toolName!)).toBeInTheDocument() expect(screen.getByText("settings:hooks.status.completed")).toBeInTheDocument() }) + it("renders command preview with wrapping enabled (no truncation)", () => { + const longCommand = "long_command_".repeat(20) + const mockHook: HookInfo = { + id: "hook-1", + event: "before_execute_command", + matcher: "git*", + commandPreview: longCommand, + enabled: true, + source: "project", + timeout: 30, + } + + currentHooksState = { + enabledHooks: [mockHook], + executionHistory: [], + hasProjectHooks: false, + } + + render() + + // Expand hook + const hookHeader = screen.getByText(mockHook.id).closest("div") + fireEvent.click(hookHeader!) + + const commandCode = screen.getByTestId(`command-preview-${mockHook.id}`) + expect(commandCode).toHaveClass("whitespace-pre-wrap") + expect(commandCode).toHaveClass("break-words") + expect(commandCode).not.toHaveClass("truncate") + expect(commandCode).toHaveTextContent(longCommand) + expect(commandCode.textContent).not.toContain("...") + }) + + it("renders log items with wrapping enabled (no truncation)", () => { + const mockHook: HookInfo = { + id: "hook-1", + event: "before_execute_command", + commandPreview: "echo test", + enabled: true, + source: "global", + timeout: 30, + } + + const mockRecord: HookExecutionRecord = { + timestamp: new Date().toISOString(), + hookId: "hook-1", + event: "before_execute_command", + toolName: "very_long_tool_name_that_should_not_be_truncated_" + "x".repeat(20), + exitCode: 0, + duration: 150, + timedOut: false, + blocked: false, + } + + currentHooksState = { + enabledHooks: [mockHook], + executionHistory: [mockRecord], + hasProjectHooks: false, + } + + render() + + // Expand hook + const hookHeader = screen.getByText(mockHook.id).closest("div") + fireEvent.click(hookHeader!) + + const toolName = screen.getByTestId("log-tool-name") + expect(toolName).toHaveClass("break-words") + expect(toolName).not.toHaveClass("truncate") + }) + it("filters logs per hook correctly", () => { const mockHook1: HookInfo = { id: "hook-1", @@ -280,7 +376,7 @@ describe("HooksSettings", () => { fireEvent.click(hookHeader!) // Should show no logs message - expect(screen.getByText("settings:hooks.noLogsForHook")).toBeInTheDocument() + expect(screen.getAllByText("settings:hooks.noLogsForHook")[0]).toBeInTheDocument() }) it("shows project hooks warning when hasProjectHooks is true", () => { diff --git a/webview-ui/src/i18n/locales/en/settings.json b/webview-ui/src/i18n/locales/en/settings.json index a0533aec16..a492631a7f 100644 --- a/webview-ui/src/i18n/locales/en/settings.json +++ b/webview-ui/src/i18n/locales/en/settings.json @@ -83,6 +83,11 @@ "failed": "Failed", "blocked": "Blocked", "timeout": "Timeout" + }, + "tabs": { + "config": "Config", + "command": "Command", + "logs": "Logs" } }, "about": {