Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
201 changes: 201 additions & 0 deletions apps/web/src/components/Sidebar.dblclick.browser.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
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";

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(),
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<string | null>(null);
const [renamingTitle, setRenamingTitle] = useState("");
const [confirmingArchiveThreadKey, setConfirmingArchiveThreadKey] = useState<string | null>(null);
const renamingInputRef = useRef<HTMLInputElement | null>(null);
const renamingCommittedRef = useRef(false);
const confirmArchiveButtonRefs = useRef(new Map<string, HTMLButtonElement>());

const startThreadRename = useCallback((threadKey: string, nextTitle: string) => {
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 (
<AppAtomRegistryProvider>
<ul>
<SidebarThreadRow
thread={buildThread(title)}
projectCwd={null}
orderedProjectThreadKeys={[]}
isActive={false}
jumpLabel={null}
appSettingsConfirmThreadArchive={false}
renamingThreadKey={renamingThreadKey}
renamingTitle={renamingTitle}
setRenamingTitle={setRenamingTitle}
startThreadRename={startThreadRename}
renamingInputRef={renamingInputRef}
renamingCommittedRef={renamingCommittedRef}
confirmingArchiveThreadKey={confirmingArchiveThreadKey}
setConfirmingArchiveThreadKey={setConfirmingArchiveThreadKey}
confirmArchiveButtonRefs={confirmArchiveButtonRefs}
handleThreadClick={spies.handleThreadClick}
navigateToThread={spies.navigateToThread}
handleMultiSelectContextMenu={spies.handleMultiSelectContextMenu}
handleThreadContextMenu={spies.handleThreadContextMenu}
clearSelection={spies.clearSelection}
commitRename={commitRename}
cancelRename={cancelRename}
attemptArchiveThread={spies.attemptArchiveThread}
openPrLink={spies.openPrLink}
/>
</ul>
</AppAtomRegistryProvider>
);
}

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(<Harness />);

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(<Harness />);

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(<Harness />);

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("single click routes through the navigation handler and does not start a rename", async () => {
render(<Harness />);

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);
});
});
19 changes: 19 additions & 0 deletions apps/web/src/components/Sidebar.logic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
getProjectSortTimestamp,
hasUnseenCompletion,
isContextMenuPointerDown,
isTrailingDoubleClick,
orderItemsByPreferredIds,
resolveProjectStatusIndicator,
resolveSidebarNewThreadSeedContext,
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions apps/web/src/components/Sidebar.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
33 changes: 29 additions & 4 deletions apps/web/src/components/Sidebar.tsx

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium

Double-clicking inside the rename input to select text bubbles dblclick to SidebarMenuSubButton, triggering handleRowDoubleClick which resets renamingTitle to thread.title and discards the user's edits. The input's onClick handler only stops click propagation, not dblclick. Consider adding onDoubleClick={stopPropagation} to the rename input to prevent the event from reaching the row button.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 595:

Double-clicking inside the rename input to select text bubbles `dblclick` to `SidebarMenuSubButton`, triggering `handleRowDoubleClick` which resets `renamingTitle` to `thread.title` and discards the user's edits. The input's `onClick` handler only stops `click` propagation, not `dblclick`. Consider adding `onDoubleClick={stopPropagation}` to the rename input to prevent the event from reaching the row button.

Evidence trail:
apps/web/src/components/Sidebar.tsx lines 595-603 (input element with onClick but no onDoubleClick), lines 505-507 (handleRenameInputClick stops propagation for click only), line 571 (parent onDoubleClick={handleRowDoubleClick}), lines 425-431 (handleRowDoubleClick calls startThreadRename(threadKey, thread.title)), lines 1804-1808 (startThreadRename calls setRenamingTitle(title) resetting to original thread.title).

Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ import {
getSidebarThreadIdsToPrewarm,
resolveAdjacentThreadId,
isContextMenuPointerDown,
isTrailingDoubleClick,
resolveProjectStatusIndicator,
resolveSidebarNewThreadSeedContext,
resolveSidebarNewThreadEnvMode,
Expand Down Expand Up @@ -289,6 +290,7 @@ interface SidebarThreadRowProps {
renamingThreadKey: string | null;
renamingTitle: string;
setRenamingTitle: (title: string) => void;
startThreadRename: (threadKey: string, title: string) => void;
renamingInputRef: React.RefObject<HTMLInputElement | null>;
renamingCommittedRef: React.RefObject<boolean>;
confirmingArchiveThreadKey: string | null;
Expand Down Expand Up @@ -316,7 +318,7 @@ interface SidebarThreadRowProps {
openPrLink: (event: React.MouseEvent<HTMLElement>, prUrl: string) => void;
}

const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowProps) {
export const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowProps) {
const {
orderedProjectThreadKeys,
isActive,
Expand All @@ -325,6 +327,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP
renamingThreadKey,
renamingTitle,
setRenamingTitle,
startThreadRename,
renamingInputRef,
renamingCommittedRef,
confirmingArchiveThreadKey,
Expand Down Expand Up @@ -419,6 +422,13 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP
},
[handleThreadClick, orderedProjectThreadKeys, threadRef],
);
const handleRowDoubleClick = useCallback(
(event: React.MouseEvent) => {
event.preventDefault();
startThreadRename(threadKey, thread.title);
},
[startThreadRename, threadKey, thread.title],
);
const handleRowKeyDown = useCallback(
(event: React.KeyboardEvent) => {
if (event.key !== "Enter" && event.key !== " ") return;
Expand Down Expand Up @@ -558,6 +568,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP
isSelected,
})} relative isolate`}
onClick={handleRowClick}
onDoubleClick={handleRowDoubleClick}
onKeyDown={handleRowKeyDown}
onContextMenu={handleRowContextMenu}
>
Expand Down Expand Up @@ -751,6 +762,7 @@ interface SidebarProjectThreadListProps {
renamingThreadKey: string | null;
renamingTitle: string;
setRenamingTitle: (title: string) => void;
startThreadRename: (threadKey: string, title: string) => void;
renamingInputRef: React.RefObject<HTMLInputElement | null>;
renamingCommittedRef: React.RefObject<boolean>;
confirmingArchiveThreadKey: string | null;
Expand Down Expand Up @@ -801,6 +813,7 @@ const SidebarProjectThreadList = memo(function SidebarProjectThreadList(
renamingThreadKey,
renamingTitle,
setRenamingTitle,
startThreadRename,
renamingInputRef,
renamingCommittedRef,
confirmingArchiveThreadKey,
Expand Down Expand Up @@ -852,6 +865,7 @@ const SidebarProjectThreadList = memo(function SidebarProjectThreadList(
renamingThreadKey={renamingThreadKey}
renamingTitle={renamingTitle}
setRenamingTitle={setRenamingTitle}
startThreadRename={startThreadRename}
renamingInputRef={renamingInputRef}
renamingCommittedRef={renamingCommittedRef}
confirmingArchiveThreadKey={confirmingArchiveThreadKey}
Expand Down Expand Up @@ -1570,6 +1584,11 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec
threadRef: ScopedThreadRef,
orderedProjectThreadKeys: readonly string[],
) => {
// Ignore the trailing click of a double-click so it doesn't navigate
// while a double-click is starting an inline rename.
if (isTrailingDoubleClick(event.detail)) {
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mobile sidebar closes before rename

Medium Severity

On mobile, the first tap of a double-tap still runs handleThreadClick, which calls setOpenMobile(false) and closes the sidebar before the row’s dblclick handler can start inline rename. Double-tap-to-rename therefore likely never works in the mobile sidebar even though desktop double-click does.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ba0abbd. Configure here.

const isMac = isMacPlatform(navigator.platform);
const isModClick = isMac ? event.metaKey : event.ctrlKey;
const isShiftClick = event.shiftKey;
Expand Down Expand Up @@ -1782,6 +1801,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);
Expand Down Expand Up @@ -1941,9 +1966,7 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec
);

if (clicked === "rename") {
setRenamingThreadKey(threadKey);
setRenamingTitle(thread.title);
renamingCommittedRef.current = false;
startThreadRename(threadKey, thread.title);
return;
}

Expand Down Expand Up @@ -1991,6 +2014,7 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec
markThreadUnread,
memberProjectByScopedKey,
project.cwd,
startThreadRename,
],
);

Expand Down Expand Up @@ -2113,6 +2137,7 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec
renamingThreadKey={renamingThreadKey}
renamingTitle={renamingTitle}
setRenamingTitle={setRenamingTitle}
startThreadRename={startThreadRename}
renamingInputRef={renamingInputRef}
renamingCommittedRef={renamingCommittedRef}
confirmingArchiveThreadKey={confirmingArchiveThreadKey}
Expand Down
Loading