diff --git a/apps/web/components/session-tabs.test.tsx b/apps/web/components/session-tabs.test.tsx new file mode 100644 index 0000000000..5465931404 --- /dev/null +++ b/apps/web/components/session-tabs.test.tsx @@ -0,0 +1,89 @@ +import type { ReactNode, RefObject } from "react"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@kandev/ui/context-menu", () => ({ + ContextMenu: ({ children }: { children: ReactNode }) => <>{children}, + ContextMenuTrigger: ({ children }: { children: ReactNode }) => <>{children}, +})); + +import { SessionTabs, type SessionTab } from "@/components/session-tabs"; + +describe("SessionTabs terminal tab contract", () => { + afterEach(cleanup); + + it("exposes separate context actions from an ordinary tab", () => { + const onRename = vi.fn(); + const onTerminate = vi.fn(); + const tabs: SessionTab[] = [ + { + id: "terminal-1", + label: "Terminal", + testId: "terminal-tab-terminal-1", + closable: true, + renderContextMenu: () => ( + <> + + + + ), + }, + ]; + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Rename" })); + fireEvent.click(screen.getByRole("button", { name: "Terminate" })); + + expect(onRename).toHaveBeenCalledOnce(); + expect(onTerminate).toHaveBeenCalledOnce(); + }); + + it("passes a populated close anchor to the context-menu renderer", () => { + let closeAnchorRef: RefObject | undefined; + const tabs: SessionTab[] = [ + { + id: "terminal-1", + label: "Terminal", + testId: "terminal-tab-terminal-1", + closeTestId: "terminal-tab-close-terminal-1", + closable: true, + onClose: vi.fn(), + renderContextMenu: (ref) => { + closeAnchorRef = ref; + return null; + }, + }, + ]; + + render(); + + expect(closeAnchorRef?.current).toBe(screen.getByTestId("terminal-tab-close-terminal-1")); + }); + + it("renders custom content beside the tab trigger without replacing tab semantics", () => { + const tabs: SessionTab[] = [ + { + id: "terminal-1", + label: "Terminal", + icon: , + badge: "#1", + testId: "terminal-tab-terminal-1", + content: , + }, + ]; + + render(); + + const customContent = screen.getByTestId("custom-content"); + expect(customContent.closest("button")).toBeNull(); + expect(screen.queryByTestId("default-icon")).toBeNull(); + expect(screen.queryByText("#1")).toBeNull(); + expect(screen.queryByText("Terminal", { selector: "span.truncate" })).toBeNull(); + expect(screen.getByRole("tab", { name: "Terminal" })).toBeTruthy(); + }); +}); diff --git a/apps/web/components/session-tabs.tsx b/apps/web/components/session-tabs.tsx index f10fc485bb..65e93f397c 100644 --- a/apps/web/components/session-tabs.tsx +++ b/apps/web/components/session-tabs.tsx @@ -1,8 +1,9 @@ "use client"; -import type { ReactNode, MouseEvent } from "react"; +import { useRef, type ReactNode, type MouseEvent, type RefObject } from "react"; import { Fragment } from "react"; import { Tabs, TabsList, TabsTrigger } from "@kandev/ui/tabs"; +import { ContextMenu, ContextMenuTrigger } from "@kandev/ui/context-menu"; export type SessionTab = { id: string; @@ -15,6 +16,8 @@ export type SessionTab = { onClose?: (event: MouseEvent) => void; onContextMenu?: (event: MouseEvent) => void; onDoubleClick?: (event: MouseEvent) => void; + renderContextMenu?: (closeAnchorRef: RefObject) => ReactNode; + content?: ReactNode; className?: string; testId?: string; closeTestId?: string; @@ -69,6 +72,51 @@ function CollapseIcon({ isCollapsed }: { isCollapsed: boolean }) { ); } +function getTabWidthClass(truncate: boolean | undefined) { + return truncate === false ? "max-w-[200px]" : "max-w-[120px]"; +} + +function DefaultTabContent({ tab }: { tab: SessionTab }) { + return ( + <> + {tab.icon} + {tab.badge && ( + + {tab.badge} + + )} + + {tab.label} + + + ); +} + +function TabCloseControl({ + tab, + closeAnchorRef, +}: { + tab: SessionTab; + closeAnchorRef: RefObject; +}) { + if (!tab.closable || !tab.onClose) return null; + return ( + + + + ); +} + function SessionTabItem({ tab, index, @@ -78,46 +126,52 @@ function SessionTabItem({ index: number; separatorAfterIndex?: number; }) { + const closeAnchorRef = useRef(null); + const tabWidthClass = getTabWidthClass(tab.truncate); + const triggerClassName = + (tab.className ?? "") + " group relative py-1 cursor-pointer rounded-sm " + tabWidthClass; + const trigger = ( + + {tab.content ? {tab.label} : } + + + ); + const tabTarget = tab.content ? ( +
+ {trigger} +
+ {tab.content} +
+
+ ) : ( + trigger + ); + return ( {separatorAfterIndex !== undefined && index === separatorAfterIndex + 1 && (
)} - - {tab.icon} - {tab.badge && ( - - {tab.badge} - - )} - - {tab.label} - - {tab.closable && tab.onClose && ( - - - - )} - + {tab.renderContextMenu ? ( + + {tabTarget} + {tab.renderContextMenu(closeAnchorRef)} + + ) : ( + tabTarget + )} ); } diff --git a/apps/web/components/task/task-right-panel.tsx b/apps/web/components/task/task-right-panel.tsx index 4e592494bc..dbae3bbf8e 100644 --- a/apps/web/components/task/task-right-panel.tsx +++ b/apps/web/components/task/task-right-panel.tsx @@ -1,6 +1,6 @@ "use client"; -import type { ReactNode, MouseEvent } from "react"; +import type { ReactNode, MouseEvent, RefObject } from "react"; import type { Layout } from "react-resizable-panels"; import { memo, useEffect, useCallback, useState, useMemo, useRef } from "react"; import { SessionPanel } from "@kandev/ui/pannel-session"; @@ -23,10 +23,11 @@ import { CommandsTabContent, TerminalTabContents, } from "@/components/task/task-right-panel-tab-contents"; +import { TerminalTabMenu } from "@/components/task/terminal-tab"; +import { TabRenameInput } from "@/components/task/tab-rename-input"; import type { RepositoryScript } from "@/lib/types/http"; import type { Terminal } from "@/hooks/domains/session/use-terminals"; import { useTranslation } from "react-i18next"; -import { t } from "@/lib/i18n"; type TaskRightPanelProps = { topPanel: ReactNode; @@ -38,22 +39,15 @@ type TaskRightPanelProps = { const DEFAULT_RIGHT_LAYOUT: Record = { top: 55, bottom: 45 }; -/** Typed verbatim by the user and compared with `===`, so it must never be - * translated. Passed into the prompt copy as an interpolation value so the - * sentence can be localized while the token itself survives every locale. */ -const DESTROY_TOKEN = "destroy"; - -function pickCurrentRenameValue(terminal: Terminal): string { - if (terminal.customName && terminal.customName !== "") return terminal.customName; - if (terminal.seq) return t("task:terminalWithSeq", { seq: terminal.seq }); - return terminal.label; -} - type TerminalTabsBuilderOpts = { terminals: Terminal[]; handleCloseDevTab: (event: MouseEvent) => void; handleCloseTab: (event: MouseEvent, id: string) => void; - onContextMenu?: (event: MouseEvent, terminal: Terminal) => void; + renderContextMenu?: ( + terminal: Terminal, + closeAnchorRef: RefObject, + ) => ReactNode; + renderContent?: (terminal: Terminal) => ReactNode; onDoubleClick?: (event: MouseEvent, terminal: Terminal) => void; }; @@ -68,7 +62,8 @@ function buildTerminalTabs({ terminals, handleCloseDevTab, handleCloseTab, - onContextMenu, + renderContextMenu, + renderContent, onDoubleClick, }: TerminalTabsBuilderOpts): SessionTab[] { return terminals.map((terminal) => { @@ -82,8 +77,11 @@ function buildTerminalTabs({ className: terminal.closable ? "group flex items-center cursor-pointer" : "cursor-pointer", closable: terminal.closable, onClose: isDev ? handleCloseDevTab : (e: MouseEvent) => handleCloseTab(e, terminal.id), - onContextMenu: - isOrdinary && onContextMenu ? (e: MouseEvent) => onContextMenu(e, terminal) : undefined, + renderContextMenu: + isOrdinary && renderContextMenu + ? (closeAnchorRef) => renderContextMenu(terminal, closeAnchorRef) + : undefined, + content: isOrdinary ? renderContent?.(terminal) : undefined, onDoubleClick: isOrdinary && onDoubleClick ? (e: MouseEvent) => onDoubleClick(e, terminal) : undefined, testId: `terminal-tab-${terminal.id}`, @@ -127,13 +125,25 @@ function useRightPanelPersistence({ }, [isBottomCollapsed]); } +function useRightPanelTabChange( + sessionId: string | null, + setRightPanelActiveTab: (sessionId: string, tabId: string) => void, +) { + return useCallback( + (value: string) => { + if (sessionId) setRightPanelActiveTab(sessionId, value); + }, + [sessionId, setRightPanelActiveTab], + ); +} + function useRightPanelTabs({ hasScripts, terminals, handleCloseDevTab, handleCloseTab, renameTerminal, - destroyTerminal, + requestCloseFromContext, sessionId, setRightPanelActiveTab, }: { @@ -142,33 +152,61 @@ function useRightPanelTabs({ handleCloseDevTab: (event: MouseEvent) => void; handleCloseTab: (event: MouseEvent, terminalId: string) => void; renameTerminal: (id: string, name: string | null) => Promise | void; - destroyTerminal: (id: string) => Promise | void; + requestCloseFromContext: (id: string, closeAnchor: HTMLElement | null) => void; sessionId: string | null; setRightPanelActiveTab: (sessionId: string, tabId: string) => void; }) { const { t } = useTranslation(); - const onContextMenu = useCallback( - (event: MouseEvent, terminal: Terminal) => { - event.preventDefault(); - // Browser prompt() is intentionally low-tech; the next iteration - // promotes this to a real shadcn ContextMenu, but window.prompt - // ships the renameable-terminal requirement today. - const current = pickCurrentRenameValue(terminal); - const choice = window.prompt( - t("task:renameTerminalLeaveEmptyToReset", { token: DESTROY_TOKEN }), - current, - ); - if (choice === null) return; - const trimmed = choice.trim(); - if (trimmed.toLowerCase() === DESTROY_TOKEN) { - void destroyTerminal(terminal.id); - return; - } - void renameTerminal(terminal.id, trimmed === "" ? null : trimmed); - }, - [renameTerminal, destroyTerminal], - ); + const [renamingTerminalId, setRenamingTerminalId] = useState(null); + const handleStartRename = useCallback((terminalId: string) => { + setRenamingTerminalId(terminalId); + }, []); + const handleRenameCommit = useCallback( + async (terminalId: string, next: string) => { + setRenamingTerminalId(null); + const trimmed = next.trim(); + try { + await renameTerminal(terminalId, trimmed === "" ? null : trimmed); + } catch (error) { + console.error(error); + } + }, + [renameTerminal], + ); + const handleRenameCancel = useCallback(() => { + setRenamingTerminalId(null); + }, []); + + const renderTerminalContextMenu = useCallback( + (terminal: Terminal, closeAnchorRef: RefObject) => ( + handleStartRename(terminal.id)} + onClosePanel={() => undefined} + onTerminatePanel={() => requestCloseFromContext(terminal.id, closeAnchorRef.current)} + /> + ), + [handleStartRename, requestCloseFromContext], + ); + const renderTerminalContent = useCallback( + (terminal: Terminal) => { + if (renamingTerminalId !== terminal.id) return undefined; + let initial = terminal.label; + if (terminal.seq) initial = t("task:terminalWithSeq", { seq: terminal.seq }); + if (terminal.customName && terminal.customName !== "") initial = terminal.customName; + return ( + void handleRenameCommit(terminal.id, next)} + onCancel={handleRenameCancel} + testId="terminal-tab-rename-input" + /> + ); + }, + [handleRenameCancel, handleRenameCommit, renamingTerminalId, t], + ); const tabs: SessionTab[] = useMemo(() => { const commandsTabs: SessionTab[] = hasScripts ? [{ id: "commands", label: t("common:commandGroupCommands") }] @@ -179,18 +217,22 @@ function useRightPanelTabs({ terminals, handleCloseDevTab, handleCloseTab, - onContextMenu, - onDoubleClick: onContextMenu, + renderContextMenu: renderTerminalContextMenu, + renderContent: renderTerminalContent, + onDoubleClick: (_event, terminal) => handleStartRename(terminal.id), }), ]; - }, [hasScripts, terminals, handleCloseDevTab, handleCloseTab, onContextMenu, t]); - - const handleTabChange = useCallback( - (value: string) => { - if (sessionId) setRightPanelActiveTab(sessionId, value); - }, - [sessionId, setRightPanelActiveTab], - ); + }, [ + hasScripts, + terminals, + handleCloseDevTab, + handleCloseTab, + renderTerminalContextMenu, + renderTerminalContent, + handleStartRename, + t, + ]); + const handleTabChange = useRightPanelTabChange(sessionId, setRightPanelActiveTab); return { tabs, handleTabChange }; } @@ -208,19 +250,27 @@ function useConfirmableTerminalClose({ const [pendingClose, setPendingClose] = useState(null); const closeAnchorRef = useRef(null); + const requestClose = useCallback( + (terminalId: string, anchor: HTMLElement | null) => { + const terminal = terminals.find((t) => t.id === terminalId); + if (!terminal || !anchor) return false; + closeAnchorRef.current = anchor; + setPendingClose(terminal); + return true; + }, + [terminals], + ); + const handleAskCloseTab = useCallback( (event: MouseEvent, terminalId: string) => { - const terminal = terminals.find((t) => t.id === terminalId); - if (terminal) { + if (requestClose(terminalId, event.currentTarget as HTMLElement)) { event.preventDefault(); event.stopPropagation(); - closeAnchorRef.current = event.currentTarget as HTMLElement; - setPendingClose(terminal); return; } handleCloseTab(event, terminalId); }, - [terminals, handleCloseTab], + [requestClose, handleCloseTab], ); const handleConfirmClose = useCallback(async () => { @@ -233,6 +283,7 @@ function useConfirmableTerminalClose({ setPendingClose, closeAnchorRef, handleAskCloseTab, + requestClose, handleConfirmClose, }; } @@ -447,7 +498,7 @@ function useTaskRightPanel({ handleCloseDevTab, handleCloseTab: closeConfirm.handleAskCloseTab, renameTerminal, - destroyTerminal, + requestCloseFromContext: closeConfirm.requestClose, sessionId, setRightPanelActiveTab, }); diff --git a/apps/web/components/task/terminal-tab.test.tsx b/apps/web/components/task/terminal-tab.test.tsx index 5bcc43fa14..47b914162e 100644 --- a/apps/web/components/task/terminal-tab.test.tsx +++ b/apps/web/components/task/terminal-tab.test.tsx @@ -71,14 +71,30 @@ vi.mock("@kandev/ui/context-menu", () => ({ }: { children: React.ReactNode; [key: string]: unknown; - }) =>
{children}
, + }) => ( +
+ {children} +
+ ), ContextMenuContent: ({ children, onCloseAutoFocus, }: { children: React.ReactNode; onCloseAutoFocus?: (event: { preventDefault: () => void }) => void; - }) =>
onCloseAutoFocus?.({ preventDefault: vi.fn() })}>{children}
, + }) => ( +
{ + const preventDefault = vi.fn(); + onCloseAutoFocus?.({ preventDefault }); + if (!preventDefault.mock.calls.length) { + document.querySelector('[data-testid="terminal-tab-shell-1"]')?.focus(); + } + }} + > + {children} +
+ ), ContextMenuItem: ({ children, onClick, @@ -169,6 +185,16 @@ describe("TerminalTab", () => { expect(mockDestroyUserShell).toHaveBeenCalledTimes(1); }); + it("focuses the inline rename input after the context menu closes", async () => { + render(); + + fireEvent.click(screen.getByRole("button", { name: /^Rename/ })); + + await waitFor(() => { + expect(document.activeElement).toBe(screen.getByTestId("terminal-tab-rename-input")); + }); + }); + it("keeps the panel dismissed and reports an error if destroy fails", async () => { mockDestroyUserShell.mockRejectedValueOnce(new Error("network down")); render(); diff --git a/apps/web/components/task/terminal-tab.tsx b/apps/web/components/task/terminal-tab.tsx index 32af0a4222..656d0afff7 100644 --- a/apps/web/components/task/terminal-tab.tsx +++ b/apps/web/components/task/terminal-tab.tsx @@ -342,7 +342,7 @@ function TerminalTabBody({ ); } -function TerminalTabMenu({ +export function TerminalTabMenu({ canMutate, onStartRename, onClosePanel, @@ -354,28 +354,37 @@ function TerminalTabMenu({ onTerminatePanel: () => void; }) { const { t } = useTranslation(); - const requestTerminateAfterCloseRef = useRef(false); + const pendingActionAfterCloseRef = useRef<"rename" | "terminate" | null>(null); + + const handleRename = useCallback(() => { + pendingActionAfterCloseRef.current = "rename"; + }, []); const handleTerminate = useCallback(() => { if (!canMutate) { onClosePanel(); return; } - requestTerminateAfterCloseRef.current = true; + pendingActionAfterCloseRef.current = "terminate"; }, [canMutate, onClosePanel]); return ( { - if (!requestTerminateAfterCloseRef.current) return; - requestTerminateAfterCloseRef.current = false; + const pendingAction = pendingActionAfterCloseRef.current; + if (!pendingAction) return; + pendingActionAfterCloseRef.current = null; event.preventDefault(); + if (pendingAction === "rename") { + queueMicrotask(onStartRename); + return; + } onTerminatePanel(); }} > {canMutate && ( <> - {t("task:rename2")} + {t("task:rename2")} )} diff --git a/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts b/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts index 0bf33b726f..d790298893 100644 --- a/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts +++ b/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts @@ -58,6 +58,42 @@ async function openTask(page: Page, title: string): Promise { return session; } +async function openTabletTask(page: Page, taskId: string): Promise { + await page.goto("/"); + await page.evaluate(() => { + window.localStorage.setItem("task-right-panel-collapsed", "false"); + }); + await page.goto(`/t/${taskId}`); + const session = new SessionPage(page); + await session.waitForLoad(); + await expect(page.getByTestId("tablet-task-layout")).toBeVisible({ timeout: 15_000 }); + return session; +} + +async function listTerminalIds( + apiClient: ApiClient, + taskId: string, + environmentId: string, +): Promise { + const response = await apiClient.wsRequest<{ + shells?: Array<{ id?: string; terminal_id?: string }>; + }>("user_shell.list", { + task_id: taskId, + task_environment_id: environmentId, + include_parked: true, + }); + return (response.shells ?? []) + .map((shell) => shell.id ?? shell.terminal_id) + .filter((id): id is string => Boolean(id)); +} + +async function settleBrowserFrames(page: Page) { + await page.evaluate( + () => + new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))), + ); +} + async function clickNewTerminalInPlusMenu(page: Page, session: SessionPage) { await session.addPanelButton().click(); const menu = page @@ -73,6 +109,96 @@ async function clickNewTerminalInPlusMenu(page: Page, session: SessionPage) { } test.describe("Terminals — dockview UI", () => { + test("right-panel terminal context menu supports inline rename and confirmed terminate", async ({ + tabletTestPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + const task = await createTaskAndWait(apiClient, seedData, "Right Panel Context Menu"); + const { sessions } = await apiClient.listTaskSessions(task.id); + const environmentId = sessions[0]?.task_environment_id; + expect(environmentId).toBeTruthy(); + if (!environmentId) throw new Error("task session is missing an environment id"); + + const first = await apiClient.wsRequest<{ terminal_id: string }>("user_shell.create", { + task_id: task.id, + task_environment_id: environmentId, + }); + const second = await apiClient.wsRequest<{ terminal_id: string }>("user_shell.create", { + task_id: task.id, + task_environment_id: environmentId, + }); + await openTabletTask(tabletTestPage, task.id); + const firstTab = tabletTestPage.getByTestId(`terminal-tab-${first.terminal_id}`); + const secondTab = tabletTestPage.getByTestId(`terminal-tab-${second.terminal_id}`); + const firstRenameInput = tabletTestPage + .getByTestId(`terminal-tab-${first.terminal_id}`) + .locator("..") + .getByTestId(`terminal-tab-${first.terminal_id}-content`) + .getByTestId("terminal-tab-rename-input"); + const secondRenameInput = tabletTestPage + .getByTestId(`terminal-tab-${second.terminal_id}`) + .locator("..") + .getByTestId(`terminal-tab-${second.terminal_id}-content`) + .getByTestId("terminal-tab-rename-input"); + await expect(firstTab).toBeVisible({ timeout: 15_000 }); + await expect(secondTab).toBeVisible({ timeout: 15_000 }); + + // Double-click keeps the existing inline rename entry point functional. + await firstTab.dblclick(); + await expect(firstRenameInput).toBeVisible(); + await expect(firstRenameInput).toBeFocused(); + await firstRenameInput.fill("double click rename"); + await tabletTestPage.keyboard.press("Enter"); + await expect(firstTab).toContainText("double click rename", { timeout: 10_000 }); + + // The close button still opens the same local confirmation and returns + // focus to that control when cancelled. + await firstTab.hover(); + const firstClose = tabletTestPage.getByTestId(`terminal-tab-close-${first.terminal_id}`); + await firstClose.click(); + const closeConfirmation = tabletTestPage.getByTestId("terminal-close-confirm-popover"); + await expect(closeConfirmation).toBeVisible(); + await closeConfirmation.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(firstClose).toBeFocused(); + await expect(firstTab).toBeVisible(); + + // Context-menu actions are separate and do not invoke browser-native + // prompt() or destroy the shell before the anchored confirmation. + let nativeDialogSeen = false; + tabletTestPage.once("dialog", (dialog) => { + nativeDialogSeen = true; + void dialog.dismiss(); + }); + await secondTab.click({ button: "right" }); + await tabletTestPage.getByRole("menuitem", { name: /^Rename/ }).click(); + await expect(secondRenameInput).toBeVisible(); + await expect(secondRenameInput).toBeFocused(); + await secondRenameInput.fill("context menu rename"); + await tabletTestPage.keyboard.press("Enter"); + await expect(secondTab).toContainText("context menu rename", { timeout: 10_000 }); + + await secondTab.click({ button: "right" }); + await tabletTestPage.getByRole("menuitem", { name: /^Terminate/ }).click(); + await expect(closeConfirmation).toBeVisible(); + expect(nativeDialogSeen).toBe(false); + await expect(secondTab).toBeVisible(); + await settleBrowserFrames(tabletTestPage); + expect( + await listTerminalIds(apiClient, task.id, environmentId), + "context-menu terminate must wait for confirmation", + ).toContain(second.terminal_id); + + await closeConfirmation.getByRole("button", { name: "Close terminal", exact: true }).click(); + await expect(secondTab).toHaveCount(0, { timeout: 10_000 }); + await expect + .poll(() => listTerminalIds(apiClient, task.id, environmentId), { + message: "confirmed context-menu terminate should destroy the shell", + }) + .not.toContain(second.terminal_id); + }); + /** * Regression: the tab title for ordinary terminals should be the * literal "Terminal" (no " N" suffix) with the sequence number in a