diff --git a/apps/web/src/components/Sidebar.dblclick.browser.tsx b/apps/web/src/components/Sidebar.dblclick.browser.tsx new file mode 100644 index 0000000000..71d744be19 --- /dev/null +++ b/apps/web/src/components/Sidebar.dblclick.browser.tsx @@ -0,0 +1,255 @@ +import "../index.css"; + +import { EnvironmentId, ProjectId, ThreadId } from "@t3tools/contracts"; +import { useCallback, useRef, useState } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import { page, userEvent } from "vite-plus/test/browser"; +import { cleanup, render } from "vitest-browser-react"; + +import { AppAtomRegistryProvider } from "../rpc/atomRegistry"; +import { DEFAULT_INTERACTION_MODE } from "../types"; +import type { SidebarThreadSummary } from "../types"; +import { SidebarThreadRow } from "./Sidebar"; + +// Double-click-to-rename is a desktop affordance; force the non-mobile path so +// the rename input is reachable regardless of the test browser viewport. +vi.mock("~/hooks/useMediaQuery", () => ({ + useIsMobile: () => false, + useMediaQuery: () => false, +})); + +const THREAD_ID = ThreadId.make("thread-1"); +const ENVIRONMENT_ID = EnvironmentId.make("environment-local"); +const PROJECT_ID = ProjectId.make("project-1"); +const INITIAL_TITLE = "Original title"; + +const ROW_TESTID = `thread-row-${THREAD_ID}`; +const TITLE_TESTID = `thread-title-${THREAD_ID}`; + +// Spies live at module scope so their call history survives the row's +// re-renders; reset between tests. +const spies = { + handleThreadClick: vi.fn(), + startThreadRename: vi.fn(), + navigateToThread: vi.fn(), + handleMultiSelectContextMenu: vi.fn(async () => {}), + handleThreadContextMenu: vi.fn(async () => {}), + clearSelection: vi.fn(), + commitRename: vi.fn(), + attemptArchiveThread: vi.fn(async () => {}), + openPrLink: vi.fn(), +}; + +function buildThread(title: string): SidebarThreadSummary { + return { + id: THREAD_ID, + environmentId: ENVIRONMENT_ID, + projectId: PROJECT_ID, + title, + interactionMode: DEFAULT_INTERACTION_MODE, + session: null, + createdAt: "2024-01-01T00:00:00.000Z", + archivedAt: null, + updatedAt: undefined, + latestTurn: null, + branch: null, + worktreePath: null, + latestUserMessageAt: null, + hasPendingApprovals: false, + hasPendingUserInput: false, + hasActionableProposedPlan: false, + }; +} + +// Mirrors the real parent (`SidebarProjectItem`): holds the rename state, wires +// `startThreadRename`, and commits by clearing the rename state and persisting +// the new title back onto the thread so the row re-renders with it. +function Harness() { + const [title, setTitle] = useState(INITIAL_TITLE); + const [renamingThreadKey, setRenamingThreadKey] = useState(null); + const [renamingTitle, setRenamingTitle] = useState(""); + const [confirmingArchiveThreadKey, setConfirmingArchiveThreadKey] = useState(null); + const renamingInputRef = useRef(null); + const renamingCommittedRef = useRef(false); + const confirmArchiveButtonRefs = useRef(new Map()); + + const startThreadRename = useCallback((threadKey: string, nextTitle: string) => { + spies.startThreadRename(threadKey, nextTitle); + setRenamingThreadKey(threadKey); + setRenamingTitle(nextTitle); + renamingCommittedRef.current = false; + }, []); + + const commitRename = useCallback( + async (threadRef: unknown, newTitle: string, originalTitle: string) => { + spies.commitRename(threadRef, newTitle, originalTitle); + const trimmed = newTitle.trim(); + if (trimmed.length > 0) { + setTitle(trimmed); + } + setRenamingThreadKey(null); + renamingInputRef.current = null; + }, + [], + ); + + const cancelRename = useCallback(() => { + setRenamingThreadKey(null); + renamingInputRef.current = null; + }, []); + + return ( + +
    + +
+ + ); +} + +describe("SidebarThreadRow double-click rename", () => { + beforeEach(() => { + for (const spy of Object.values(spies)) spy.mockClear(); + }); + + afterEach(() => { + cleanup(); + }); + + it("double-clicking a row starts the inline rename, focused with text selected", async () => { + render(); + + await expect.element(page.getByTestId(TITLE_TESTID)).toBeVisible(); + + await userEvent.dblClick(page.getByTestId(ROW_TESTID)); + + const input = page.getByRole("textbox"); + await expect.element(input).toBeVisible(); + + const element = input.element() as HTMLInputElement; + expect(element.value).toBe(INITIAL_TITLE); + // The existing rename-input ref focuses + selects the whole title. + expect(document.activeElement).toBe(element); + expect(element.selectionStart).toBe(0); + expect(element.selectionEnd).toBe(INITIAL_TITLE.length); + }); + + it("Enter commits the rename and the new title persists on the row", async () => { + render(); + + await userEvent.dblClick(page.getByTestId(ROW_TESTID)); + const input = page.getByRole("textbox"); + await expect.element(input).toBeVisible(); + + await userEvent.fill(input, "Renamed thread"); + await userEvent.keyboard("{Enter}"); + + // commitRename was invoked with (threadRef, newTitle, originalTitle). + expect(spies.commitRename).toHaveBeenCalledTimes(1); + expect(spies.commitRename).toHaveBeenCalledWith( + expect.anything(), + "Renamed thread", + INITIAL_TITLE, + ); + + // Input is gone and the row now shows the persisted title. + const title = page.getByTestId(TITLE_TESTID); + await expect.element(title).toBeVisible(); + await expect.element(title).toHaveTextContent("Renamed thread"); + }); + + it("Escape cancels the rename without committing", async () => { + render(); + + await userEvent.dblClick(page.getByTestId(ROW_TESTID)); + await expect.element(page.getByRole("textbox")).toBeVisible(); + + await userEvent.keyboard("{Escape}"); + + expect(spies.commitRename).not.toHaveBeenCalled(); + const title = page.getByTestId(TITLE_TESTID); + await expect.element(title).toBeVisible(); + await expect.element(title).toHaveTextContent(INITIAL_TITLE); + }); + + it("double-clicking inside the rename input keeps the edit (does not reset to the title)", async () => { + render(); + + await userEvent.dblClick(page.getByTestId(ROW_TESTID)); + const input = page.getByRole("textbox"); + await expect.element(input).toBeVisible(); + + await userEvent.fill(input, "Edited but not committed"); + // Double-clicking inside the input (e.g. to select a word) must not bubble + // to the row and restart the rename, which would wipe the edit. + await userEvent.dblClick(input); + + expect((input.element() as HTMLInputElement).value).toBe("Edited but not committed"); + expect(spies.commitRename).not.toHaveBeenCalled(); + }); + + it("double-clicking the row chrome while already renaming does not restart/reset it", async () => { + render(); + + await userEvent.dblClick(page.getByTestId(ROW_TESTID)); + const input = page.getByRole("textbox"); + await expect.element(input).toBeVisible(); + await userEvent.fill(input, "Edited"); + expect(spies.startThreadRename).toHaveBeenCalledTimes(1); + + // Double-click the row element itself (chrome, not the input). + const rowEl = page.getByTestId(ROW_TESTID).element(); + rowEl.dispatchEvent(new MouseEvent("dblclick", { bubbles: true, cancelable: true, detail: 2 })); + + // Guard short-circuits: rename is not restarted and the edit is preserved. + expect(spies.startThreadRename).toHaveBeenCalledTimes(1); + expect((input.element() as HTMLInputElement).value).toBe("Edited"); + }); + + it("modifier double-click is multi-select intent and does not start a rename", async () => { + render(); + + await userEvent.keyboard("{Shift>}"); + await userEvent.dblClick(page.getByTestId(ROW_TESTID)); + await userEvent.keyboard("{/Shift}"); + + await expect.element(page.getByTestId(TITLE_TESTID)).toBeVisible(); + expect(page.getByRole("textbox").elements()).toHaveLength(0); + }); + + it("single click routes through the navigation handler and does not start a rename", async () => { + render(); + + await userEvent.click(page.getByTestId(ROW_TESTID)); + + expect(spies.handleThreadClick).toHaveBeenCalledTimes(1); + // No rename input: the title span is still shown. + await expect.element(page.getByTestId(TITLE_TESTID)).toBeVisible(); + expect(page.getByRole("textbox").elements()).toHaveLength(0); + }); +}); diff --git a/apps/web/src/components/Sidebar.logic.test.ts b/apps/web/src/components/Sidebar.logic.test.ts index bdbbf6f849..fc6cbd1c0e 100644 --- a/apps/web/src/components/Sidebar.logic.test.ts +++ b/apps/web/src/components/Sidebar.logic.test.ts @@ -11,6 +11,7 @@ import { getProjectSortTimestamp, hasUnseenCompletion, isContextMenuPointerDown, + isTrailingDoubleClick, orderItemsByPreferredIds, resolveProjectStatusIndicator, resolveSidebarNewThreadSeedContext, @@ -171,6 +172,24 @@ describe("shouldClearThreadSelectionOnMouseDown", () => { }); }); +describe("isTrailingDoubleClick", () => { + it("treats a single click as a normal activation", () => { + expect(isTrailingDoubleClick(1)).toBe(false); + }); + + it("treats synthetic/keyboard activations (detail 0) as a normal activation", () => { + expect(isTrailingDoubleClick(0)).toBe(false); + }); + + it("ignores the second click of a double-click so it does not navigate", () => { + expect(isTrailingDoubleClick(2)).toBe(true); + }); + + it("ignores further clicks of a triple-click", () => { + expect(isTrailingDoubleClick(3)).toBe(true); + }); +}); + describe("resolveSidebarNewThreadEnvMode", () => { it("uses the app default when the caller does not request a specific mode", () => { expect( diff --git a/apps/web/src/components/Sidebar.logic.ts b/apps/web/src/components/Sidebar.logic.ts index b9dd27dfb0..41f4e39bb7 100644 --- a/apps/web/src/components/Sidebar.logic.ts +++ b/apps/web/src/components/Sidebar.logic.ts @@ -160,6 +160,15 @@ export function shouldClearThreadSelectionOnMouseDown(target: HTMLElement | null return !target.closest(THREAD_SELECTION_SAFE_SELECTOR); } +// A double-click dispatches two `click` events before `dblclick`: the first has +// `detail === 1`, the second `detail === 2`. The second click must not run the +// row's single-click navigation, otherwise double-click-to-rename would also +// navigate. `MouseEvent.detail` is 0 for synthetic/keyboard activations, which +// still count as a normal single activation. +export function isTrailingDoubleClick(detail: number): boolean { + return detail > 1; +} + export function resolveSidebarNewThreadEnvMode(input: { requestedEnvMode?: SidebarNewThreadEnvMode; defaultEnvMode: SidebarNewThreadEnvMode; diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 67b575e4b4..9e6ff1c34c 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -164,6 +164,7 @@ import { getSidebarThreadIdsToPrewarm, resolveAdjacentThreadId, isContextMenuPointerDown, + isTrailingDoubleClick, resolveProjectStatusIndicator, resolveSidebarNewThreadSeedContext, resolveSidebarNewThreadEnvMode, @@ -178,6 +179,7 @@ import { import { sortThreads } from "../lib/threadSort"; import { SidebarUpdatePill } from "./sidebar/SidebarUpdatePill"; import { useCopyToClipboard } from "~/hooks/useCopyToClipboard"; +import { useIsMobile } from "~/hooks/useMediaQuery"; import { CommandDialogTrigger } from "./ui/command"; import { readEnvironmentApi } from "../environmentApi"; import { useSettings, useUpdateSettings } from "~/hooks/useSettings"; @@ -292,6 +294,7 @@ interface SidebarThreadRowProps { renamingThreadKey: string | null; renamingTitle: string; setRenamingTitle: (title: string) => void; + startThreadRename: (threadKey: string, title: string) => void; renamingInputRef: React.RefObject; renamingCommittedRef: React.RefObject; confirmingArchiveThreadKey: string | null; @@ -319,7 +322,7 @@ interface SidebarThreadRowProps { openPrLink: (event: React.MouseEvent, prUrl: string) => void; } -const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowProps) { +export const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowProps) { const { orderedProjectThreadKeys, isActive, @@ -328,6 +331,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP renamingThreadKey, renamingTitle, setRenamingTitle, + startThreadRename, renamingInputRef, renamingCommittedRef, confirmingArchiveThreadKey, @@ -352,6 +356,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP environmentId: thread.environmentId, threadId: thread.id, }); + const isMobile = useIsMobile(); const discoveredPorts = useThreadDiscoveredPorts({ environmentId: thread.environmentId, threadId: thread.id, @@ -437,6 +442,24 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP }, [discoveredPorts, navigateToThread, threadRef], ); + const handleRowDoubleClick = useCallback( + (event: React.MouseEvent) => { + // Already renaming this row: a double-click on the row chrome (outside the + // input) must not restart and discard the in-progress edit. + if (renamingThreadKey === threadKey) return; + // On mobile the first tap navigates and closes the sidebar sheet, so the + // inline rename can't be shown. Renaming there stays on the context menu. + if (isMobile) return; + // cmd/ctrl/shift double-clicks are multi-select intent, not rename. + if (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) return; + // Ignore double-clicks bubbling from nested controls (PR status, port, + // archive buttons) — only the row body should enter inline rename. + if ((event.target as HTMLElement).closest("button, a")) return; + event.preventDefault(); + startThreadRename(threadKey, thread.title); + }, + [isMobile, renamingThreadKey, startThreadRename, threadKey, thread.title], + ); const handleRowKeyDown = useCallback( (event: React.KeyboardEvent) => { if (event.key !== "Enter" && event.key !== " ") return; @@ -510,6 +533,9 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP void commitRename(threadRef, renamingTitle, thread.title); } }, [commitRename, renamingCommittedRef, renamingTitle, thread.title, threadRef]); + // Keep clicks/double-clicks inside the rename input from bubbling to the row. + // Without stopping `dblclick`, double-clicking to select a word would re-fire + // the row's rename handler and reset the in-progress edit back to the title. const handleRenameInputClick = useCallback((event: React.MouseEvent) => { event.stopPropagation(); }, []); @@ -576,6 +602,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP isSelected, })} relative isolate`} onClick={handleRowClick} + onDoubleClick={handleRowDoubleClick} onKeyDown={handleRowKeyDown} onContextMenu={handleRowContextMenu} > @@ -607,6 +634,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP onKeyDown={handleRenameInputKeyDown} onBlur={handleRenameInputBlur} onClick={handleRenameInputClick} + onDoubleClick={handleRenameInputClick} /> ) : ( @@ -789,6 +817,7 @@ interface SidebarProjectThreadListProps { renamingThreadKey: string | null; renamingTitle: string; setRenamingTitle: (title: string) => void; + startThreadRename: (threadKey: string, title: string) => void; renamingInputRef: React.RefObject; renamingCommittedRef: React.RefObject; confirmingArchiveThreadKey: string | null; @@ -839,6 +868,7 @@ const SidebarProjectThreadList = memo(function SidebarProjectThreadList( renamingThreadKey, renamingTitle, setRenamingTitle, + startThreadRename, renamingInputRef, renamingCommittedRef, confirmingArchiveThreadKey, @@ -890,6 +920,7 @@ const SidebarProjectThreadList = memo(function SidebarProjectThreadList( renamingThreadKey={renamingThreadKey} renamingTitle={renamingTitle} setRenamingTitle={setRenamingTitle} + startThreadRename={startThreadRename} renamingInputRef={renamingInputRef} renamingCommittedRef={renamingCommittedRef} confirmingArchiveThreadKey={confirmingArchiveThreadKey} @@ -1626,6 +1657,13 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec return; } + // Ignore the trailing click of a plain double-click so it doesn't navigate + // while a double-click is starting an inline rename. Placed after the + // modifier branches so cmd/shift selection still processes every click. + if (isTrailingDoubleClick(event.detail)) { + return; + } + if (currentSelectionCount > 0) { clearSelection(); } @@ -1820,6 +1858,12 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec renamingInputRef.current = null; }, []); + const startThreadRename = useCallback((threadKey: string, title: string) => { + setRenamingThreadKey(threadKey); + setRenamingTitle(title); + renamingCommittedRef.current = false; + }, []); + const commitRename = useCallback( async (threadRef: ScopedThreadRef, newTitle: string, originalTitle: string) => { const threadKey = scopedThreadKey(threadRef); @@ -1979,9 +2023,7 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec ); if (clicked === "rename") { - setRenamingThreadKey(threadKey); - setRenamingTitle(thread.title); - renamingCommittedRef.current = false; + startThreadRename(threadKey, thread.title); return; } @@ -2029,6 +2071,7 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec markThreadUnread, memberProjectByScopedKey, project.cwd, + startThreadRename, ], ); @@ -2151,6 +2194,7 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec renamingThreadKey={renamingThreadKey} renamingTitle={renamingTitle} setRenamingTitle={setRenamingTitle} + startThreadRename={startThreadRename} renamingInputRef={renamingInputRef} renamingCommittedRef={renamingCommittedRef} confirmingArchiveThreadKey={confirmingArchiveThreadKey}