diff --git a/apps/web/components/keyboard-shortcut-tooltip.tsx b/apps/web/components/keyboard-shortcut-tooltip.tsx index 1f18e18180..cbf206b32c 100644 --- a/apps/web/components/keyboard-shortcut-tooltip.tsx +++ b/apps/web/components/keyboard-shortcut-tooltip.tsx @@ -64,7 +64,7 @@ export function KeyboardShortcutTooltip({ * @example * ```tsx * - * // Renders: Cmd+Enter (on Mac) or Ctrl+Enter (on Windows/Linux) + * // Renders: ⌘+Enter (on Mac) or Ctrl+Enter (on Windows/Linux) * ``` */ export function KeyboardShortcutText({ shortcut }: { shortcut: KeyboardShortcut }) { diff --git a/apps/web/components/quick-chat/quick-chat-content.tsx b/apps/web/components/quick-chat/quick-chat-content.tsx index f0d2292182..010478a566 100644 --- a/apps/web/components/quick-chat/quick-chat-content.tsx +++ b/apps/web/components/quick-chat/quick-chat-content.tsx @@ -1,6 +1,6 @@ "use client"; -import { memo, useCallback, useEffect, useId, useRef, useState } from "react"; +import { memo, useCallback, useEffect, useId, useRef, useState, type RefObject } from "react"; import { IconChevronDown, IconChevronUp, IconMessageQuestion } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; import { useSettingsData } from "@/hooks/domains/settings/use-settings-data"; @@ -16,6 +16,7 @@ import { ClarificationInputOverlay } from "@/components/task/chat/clarification- import { ResizeHandle } from "@/components/task/chat/resize-handle"; import { useResizableClarificationOverlay } from "@/hooks/use-resizable-clarification-overlay"; import type { Message } from "@/lib/types/http"; +import { routePanelMouseDown } from "@/components/task/chat/route-panel-mouse-down"; type QuickChatContentProps = { sessionId: string; @@ -29,12 +30,14 @@ type QuickChatClarificationSectionProps = { pending: boolean; messages: readonly Message[] | null | undefined; onResolved: () => void; + shortcutScopeRef: RefObject; }; function QuickChatClarificationSection({ pending, messages, onResolved, + shortcutScopeRef, }: QuickChatClarificationSectionProps) { const [collapsed, setCollapsed] = useState(false); const contentId = useId(); @@ -99,6 +102,7 @@ function QuickChatClarificationSection({ @@ -137,6 +141,7 @@ export const QuickChatContent = memo(function QuickChatContent({ }: QuickChatContentProps) { const [clarificationKey, setClarificationKey] = useState(0); const initialPromptSentFor = useRef(null); + const shortcutScopeRef = useRef(null); const state = useQuickChatState(sessionId); const { chatInputRef, panelState, isSending, handleSubmit, handleCancelTurn } = state; const { taskId, pendingClarification, pendingClarificationGroup } = panelState; @@ -154,9 +159,19 @@ export const QuickChatContent = memo(function QuickChatContent({ }, [initialPrompt, taskId, handleSubmit, onInitialPromptSent, sessionId]); const handleClarificationResolved = useCallback(() => setClarificationKey((k) => k + 1), []); + const handleShortcutScopeMouseDown = useCallback( + (event: React.MouseEvent) => routePanelMouseDown(event, shortcutScopeRef), + [], + ); return ( -
+
void; + shortcutScopeRef: RefObject; keyboardShortcutsEnabled?: boolean; }; @@ -169,6 +172,7 @@ function useResolveCallback( type CarouselShortcutArgs = { enabled: boolean; + scopeRef: RefObject; meta: SingleQuestionMeta; activeIndex: number; total: number; @@ -211,13 +215,14 @@ function tryHandleMetaEnter(e: KeyboardEvent, canSubmit: boolean, onSubmit: () = } function CarouselKeyboardShortcuts(args: CarouselShortcutArgs) { - const { enabled } = args; + const { enabled, scopeRef } = args; const optionsCount = args.meta.question.options.length; const isLast = args.activeIndex === args.total - 1; const { canSubmit, onPick, onPrev, onNext, onSkip, onSubmit } = args; useEffect(() => { if (!enabled) return; const onKey = (e: KeyboardEvent) => { + if (!(e.target instanceof Node) || !scopeRef.current?.contains(e.target)) return; if (tryHandleMetaEnter(e, canSubmit, onSubmit)) return; if (shouldIgnoreShortcut(e)) return; if (e.key === "Escape") { @@ -244,7 +249,18 @@ function CarouselKeyboardShortcuts(args: CarouselShortcutArgs) { }; window.addEventListener("keydown", onKey); return () => window.removeEventListener("keydown", onKey); - }, [enabled, optionsCount, isLast, canSubmit, onPick, onPrev, onNext, onSkip, onSubmit]); + }, [ + enabled, + scopeRef, + optionsCount, + isLast, + canSubmit, + onPick, + onPrev, + onNext, + onSkip, + onSubmit, + ]); return null; } @@ -257,6 +273,7 @@ type CarouselBodyProps = { setCustomDrafts: React.Dispatch>>; allAnswered: boolean; isSubmitting: boolean; + shortcutScopeRef: RefObject; keyboardShortcutsEnabled: boolean; onSubmit: () => void; }; @@ -362,6 +379,7 @@ function ClarificationCarouselBody({ setCustomDrafts, allAnswered, isSubmitting, + shortcutScopeRef, keyboardShortcutsEnabled, onSubmit, }: CarouselBodyProps) { @@ -418,7 +436,8 @@ function ClarificationCarouselBody({ /> )} void; + onSkip: () => void; +}) { + return ( +
+ {total > 1 && ( + + + + + + )} + + + + + +
+ ); +} + export function ClarificationInputOverlay({ messages, onResolved, + shortcutScopeRef, keyboardShortcutsEnabled = true, }: ClarificationInputOverlayProps) { const sortedMessages = useMemo( @@ -492,35 +578,13 @@ export function ClarificationInputOverlay({ )}
-
- {total > 1 && ( - - )} - -
+ void group.skipAll("User skipped")} + />
diff --git a/apps/web/components/task/chat/clarification-overlay-parts.tsx b/apps/web/components/task/chat/clarification-overlay-parts.tsx index 105e0a74bb..2e1ec3877e 100644 --- a/apps/web/components/task/chat/clarification-overlay-parts.tsx +++ b/apps/web/components/task/chat/clarification-overlay-parts.tsx @@ -6,6 +6,8 @@ import { Textarea } from "@kandev/ui/textarea"; import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { cn } from "@/lib/utils"; import type { ClarificationOption } from "@/lib/types/http"; +import { KeyboardShortcutTooltip } from "@/components/keyboard-shortcut-tooltip"; +import { KEYS } from "@/lib/keyboard/constants"; // Grow the custom-answer box up to ~6 lines, then scroll internally so the // clarification overlay stays compact. @@ -328,36 +330,52 @@ export function ClarificationCarouselNav({ const isLast = activeIndex === total - 1; return (
- - + + + - Next - - + + + +
); } diff --git a/apps/web/components/task/chat/route-panel-mouse-down.ts b/apps/web/components/task/chat/route-panel-mouse-down.ts new file mode 100644 index 0000000000..ec2c29209a --- /dev/null +++ b/apps/web/components/task/chat/route-panel-mouse-down.ts @@ -0,0 +1,13 @@ +import type { MouseEvent, RefObject } from "react"; + +const interactiveSelector = + "input, textarea, select, button, a, [contenteditable], [tabindex]:not([tabindex='-1'])"; + +export function routePanelMouseDown( + event: MouseEvent, + ref: RefObject, +): void { + const target = event.target as HTMLElement | null; + if (!target || target.closest(interactiveSelector)) return; + ref.current?.focus({ preventScroll: true }); +} diff --git a/apps/web/components/task/task-chat-panel.tsx b/apps/web/components/task/task-chat-panel.tsx index 5016c83f99..caab03007a 100644 --- a/apps/web/components/task/task-chat-panel.tsx +++ b/apps/web/components/task/task-chat-panel.tsx @@ -18,6 +18,7 @@ import { useSessionSearch } from "@/hooks/domains/session/use-session-search"; import { useLazyLoadMessages } from "@/hooks/use-lazy-load-messages"; import { useAppStore } from "@/components/state-provider"; import type { Message } from "@/lib/types/http"; +import { routePanelMouseDown } from "./chat/route-panel-mouse-down"; function useClarificationKey(agentMessageCount: number) { const lastCountRef = useRef(agentMessageCount); @@ -236,6 +237,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({ height={clarificationHeight} messages={pendingClarificationGroup} onResolved={handleClarificationResolved} + shortcutScopeRef={panelRef} /> void; + shortcutScopeRef: RefObject; }; function ClarificationSection({ @@ -272,6 +275,7 @@ function ClarificationSection({ height, messages, onResolved, + shortcutScopeRef, }: ClarificationSectionProps) { if (!pendingClarification || isArchived) return null; return ( @@ -283,7 +287,11 @@ function ClarificationSection({ className="px-1 overflow-y-scroll overscroll-contain max-h-[50vh]" style={height === null ? undefined : { height }} > - +
); @@ -340,21 +348,3 @@ function ChatFooter({ /> ); } - -// routePanelMouseDown routes non-interactive clicks to the panel root so that -// Ctrl+F can detect focus within the session panel. Extracted to keep -// TaskChatPanel's cyclomatic complexity within limits. -const interactiveSelector = - "input, textarea, select, button, a, [contenteditable], [tabindex]:not([tabindex='-1'])"; - -function routePanelMouseDown( - e: React.MouseEvent, - ref: React.RefObject, -): void { - const target = e.target as HTMLElement | null; - if (!target) return; - // Exclude `tabindex="-1"` so we don't match PanelRoot itself (which is - // marked focus-receivable but shouldn't short-circuit this handler). - if (target.closest(interactiveSelector)) return; - ref.current?.focus({ preventScroll: true }); -} diff --git a/apps/web/e2e/tests/chat/clarification.spec.ts b/apps/web/e2e/tests/chat/clarification.spec.ts index f50f7085eb..e7ebee5661 100644 --- a/apps/web/e2e/tests/chat/clarification.spec.ts +++ b/apps/web/e2e/tests/chat/clarification.spec.ts @@ -21,6 +21,12 @@ function seedClarificationTask( return seedClarificationSession(testPage, apiClient, seedData, title, { scenario }); } +const PLAN_WITH_CLARIFICATION_SCRIPT = [ + 'e2e:mcp:kandev:create_task_plan_kandev({"task_id":"{task_id}","content":"## Plan\\n\\nEdit 1 item","title":"Implementation Plan"})', + "e2e:delay(100)", + 'e2e:mcp:kandev:ask_user_question_kandev({"questions":[{"id":"db","prompt":"Which database should we use?","options":[{"label":"PostgreSQL","description":"Relational"},{"label":"SQLite","description":"Embedded"}]},{"id":"language","prompt":"Which language should we use?","options":[{"label":"Go","description":"Compiled"},{"label":"TypeScript","description":"Web"}]},{"id":"deploy","prompt":"How should we deploy?","options":[{"label":"Docker","description":"Containerized"},{"label":"Bare metal","description":"Direct"}]}]})', +].join("\n"); + // Exercises the regular task-create dialog (New Task in the sidebar); run with office off. useRegularMode(); @@ -478,6 +484,7 @@ test.describe("Multi-question clarification carousel", () => { await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 }); // Press "1" → first option of the first question, auto-advance. + await session.chat.focus(); await testPage.keyboard.press("1"); await expect(session.clarificationStep(1)).toHaveAttribute("data-active", "true"); await expect(session.clarificationGroupProgress()).toContainText("1 of 3 answered"); @@ -497,6 +504,129 @@ test.describe("Multi-question clarification carousel", () => { await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); }); + test("question shortcuts only fire while the chat panel has focus", async ({ + testPage, + apiClient, + seedData, + }) => { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Clarification shortcuts stay in chat", + seedData.agentProfileId, + { + description: PLAN_WITH_CLARIFICATION_SCRIPT, + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id"); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 }); + if (!(await session.planPanel.isVisible())) await session.togglePlanMode(); + await expect(session.planEditor()).toBeVisible({ timeout: 15_000 }); + + const planEditor = session.planEditor(); + await planEditor.click(); + await testPage.keyboard.press("1"); + await expect(planEditor).toContainText("Edit 1 item1"); + await expect(session.clarificationStep(0)).toHaveAttribute("data-active", "true"); + await expect(session.clarificationGroupProgress()).toContainText("0 of 3 answered"); + + await session.chat.focus(); + await testPage.keyboard.press("1"); + await expect(session.clarificationStep(1)).toHaveAttribute("data-active", "true"); + await session.clarificationOption("Go").click(); + await session.clarificationOption("Docker").click(); + await expect(session.clarificationGroupProgress()).toContainText("3 of 3 answered"); + + await planEditor.focus(); + await testPage.keyboard.press("ControlOrMeta+Enter"); + await expect(session.clarificationOverlay()).toBeVisible(); + + await session.chat.focus(); + await testPage.keyboard.press("ControlOrMeta+Enter"); + await expect(session.clarificationOverlay()).not.toBeVisible({ timeout: 30_000 }); + }); + + test("action tooltips show their keyboard shortcuts", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await seedClarificationTask( + testPage, + apiClient, + seedData, + "Clarification shortcut hints", + "clarification-multi", + ); + + await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 }); + + await testPage.getByTestId("clarification-submit-shortcut").hover(); + const submitTooltip = testPage.getByRole("tooltip", { name: /Submit answers/ }); + await expect(submitTooltip).toContainText(/Ctrl|⌘/); + await expect(submitTooltip).toContainText("Enter"); + + await testPage.getByTestId("clarification-skip-shortcut").hover(); + const skipTooltip = testPage.getByRole("tooltip", { name: /Skip all questions/ }); + await expect(skipTooltip).toContainText("Esc"); + + await session.clarificationOption("PostgreSQL").click(); + await session.clarificationPrev().hover(); + const previousTooltip = testPage.getByRole("tooltip", { name: /Previous question/ }); + await expect(previousTooltip).toContainText("←"); + + await session.clarificationNext().hover(); + const nextTooltip = testPage.getByRole("tooltip", { name: /Next question/ }); + await expect(nextTooltip).toContainText("→"); + }); + + test("question shortcuts stay disabled while answers are submitting", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await seedClarificationTask( + testPage, + apiClient, + seedData, + "Clarification shortcuts while submitting", + "clarification-multi", + ); + + await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 }); + await session.clarificationOption("PostgreSQL").click(); + await session.clarificationOption("Go").click(); + await session.clarificationOption("Docker").click(); + + let releaseResponse = () => undefined; + const heldResponse = new Promise((resolve) => { + releaseResponse = resolve; + }); + await testPage.route("**/api/v1/clarification/*/respond", async (route) => { + await heldResponse; + await route.fulfill({ status: 200, contentType: "application/json", body: "{}" }); + }); + + await session.chat.focus(); + await testPage.keyboard.press("ControlOrMeta+Enter"); + await expect(session.clarificationSubmit()).toContainText("Submitting"); + + await testPage.keyboard.press("ArrowLeft"); + await expect(session.clarificationStep(2)).toHaveAttribute("data-active", "true"); + + await testPage.getByTestId("clarification-submit-shortcut").hover(); + await expect(testPage.getByRole("tooltip", { name: /Submit answers/ })).not.toBeVisible(); + + releaseResponse(); + await expect(session.clarificationOverlay()).not.toBeVisible({ timeout: 30_000 }); + }); + test("Esc skips the entire bundle from anywhere in the carousel", async ({ testPage, apiClient, diff --git a/apps/web/e2e/tests/chat/quick-chat.spec.ts b/apps/web/e2e/tests/chat/quick-chat.spec.ts index 8aee67d9a8..a43aaff5a5 100644 --- a/apps/web/e2e/tests/chat/quick-chat.spec.ts +++ b/apps/web/e2e/tests/chat/quick-chat.spec.ts @@ -96,6 +96,22 @@ async function waitForQuickChatWidth(dialog: Locator) { } test.describe("Quick Chat", () => { + test("clarification shortcuts work after clicking the message surface", async ({ testPage }) => { + const dialog = await openQuickChatWithAgent(testPage); + await sendQuickChatMessage(dialog, testPage, "/e2e:clarification-multi"); + + const clarification = dialog.getByTestId("clarification-overlay"); + await expect(clarification).toBeVisible({ timeout: 30_000 }); + + await dialog.getByTestId("quick-chat-messages").click({ position: { x: 8, y: 8 } }); + await expect(dialog.getByTestId("quick-chat-content")).toBeFocused(); + + await testPage.keyboard.press("1"); + await expect( + clarification.locator('[data-testid="clarification-step"][data-step-index="1"]'), + ).toHaveAttribute("data-active", "true"); + }); + test("offers configuration chat in setup and hides it once one exists", async ({ testPage, apiClient, diff --git a/apps/web/lib/keyboard/utils.test.ts b/apps/web/lib/keyboard/utils.test.ts index 2bbe8e8909..96e4f60a62 100644 --- a/apps/web/lib/keyboard/utils.test.ts +++ b/apps/web/lib/keyboard/utils.test.ts @@ -82,12 +82,12 @@ describe("isMac", () => { }); describe("formatShortcut", () => { - it("formats Cmd+Enter on Mac", () => { + it("formats Command+Enter with the macOS symbol", () => { const shortcut: KeyboardShortcut = { key: KEYS.ENTER, modifiers: { ctrlOrCmd: true }, }; - expect(formatShortcut(shortcut, "mac")).toBe("Cmd+Enter"); + expect(formatShortcut(shortcut, "mac")).toBe("⌘+Enter"); }); it("formats Ctrl+Enter on Windows", () => { @@ -114,20 +114,28 @@ describe("formatShortcut", () => { expect(formatShortcut(shortcut, "windows")).toBe("Ctrl+S"); }); - it("formats Cmd+S on Mac", () => { + it("formats Command+S with the macOS symbol", () => { const shortcut: KeyboardShortcut = { key: KEYS.S, modifiers: { cmd: true }, }; - expect(formatShortcut(shortcut, "mac")).toBe("Cmd+S"); + expect(formatShortcut(shortcut, "mac")).toBe("⌘+S"); }); - it("formats Alt as Option on Mac", () => { + it("formats Alt with the macOS Option symbol", () => { const shortcut: KeyboardShortcut = { key: KEYS.A, modifiers: { alt: true }, }; - expect(formatShortcut(shortcut, "mac")).toBe("Option+A"); + expect(formatShortcut(shortcut, "mac")).toBe("⌥+A"); + }); + + it("formats every macOS modifier with its keyboard symbol", () => { + const shortcut: KeyboardShortcut = { + key: KEYS.S, + modifiers: { ctrl: true, cmd: true, alt: true, shift: true }, + }; + expect(formatShortcut(shortcut, "mac")).toBe("⌃+⌘+⌥+⇧+S"); }); it("formats Alt on Windows", () => { @@ -181,7 +189,7 @@ describe("formatShortcut", () => { key: KEYS.ENTER, modifiers: { ctrlOrCmd: true }, }; - expect(formatShortcut(shortcut)).toBe("Cmd+Enter"); + expect(formatShortcut(shortcut)).toBe("⌘+Enter"); }); }); diff --git a/apps/web/lib/keyboard/utils.ts b/apps/web/lib/keyboard/utils.ts index 09061211e9..a1c643d4d1 100644 --- a/apps/web/lib/keyboard/utils.ts +++ b/apps/web/lib/keyboard/utils.ts @@ -46,13 +46,13 @@ function collectModifierNames( const parts: string[] = []; if (modifiers.ctrlOrCmd) { - parts.push(currentPlatform === "mac" ? "Cmd" : "Ctrl"); + parts.push(currentPlatform === "mac" ? "⌘" : "Ctrl"); } else { - if (modifiers.ctrl) parts.push("Ctrl"); - if (modifiers.cmd && currentPlatform === "mac") parts.push("Cmd"); + if (modifiers.ctrl) parts.push(currentPlatform === "mac" ? "⌃" : "Ctrl"); + if (modifiers.cmd && currentPlatform === "mac") parts.push("⌘"); } - if (modifiers.alt) parts.push(currentPlatform === "mac" ? "Option" : "Alt"); - if (modifiers.shift) parts.push("Shift"); + if (modifiers.alt) parts.push(currentPlatform === "mac" ? "⌥" : "Alt"); + if (modifiers.shift) parts.push(currentPlatform === "mac" ? "⇧" : "Shift"); return parts; } @@ -60,7 +60,7 @@ function collectModifierNames( * Format a keyboard shortcut for display based on platform * @param shortcut - The keyboard shortcut definition * @param platform - Optional platform override (defaults to current platform) - * @returns Formatted string like "Cmd+Enter" or "Ctrl+Enter" + * @returns Formatted string like "⌘+Enter" or "Ctrl+Enter" */ export function formatShortcut(shortcut: KeyboardShortcut, platform?: Platform): string { const currentPlatform = platform ?? detectPlatform();