-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Double-click a sidebar thread row to rename #3064
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -162,6 +162,7 @@ import { | |
| getSidebarThreadIdsToPrewarm, | ||
| resolveAdjacentThreadId, | ||
| isContextMenuPointerDown, | ||
| isTrailingDoubleClick, | ||
| resolveProjectStatusIndicator, | ||
| resolveSidebarNewThreadSeedContext, | ||
| resolveSidebarNewThreadEnvMode, | ||
|
|
@@ -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; | ||
|
|
@@ -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, | ||
|
|
@@ -325,6 +327,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP | |
| renamingThreadKey, | ||
| renamingTitle, | ||
| setRenamingTitle, | ||
| startThreadRename, | ||
| renamingInputRef, | ||
| renamingCommittedRef, | ||
| confirmingArchiveThreadKey, | ||
|
|
@@ -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; | ||
|
|
@@ -558,6 +568,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP | |
| isSelected, | ||
| })} relative isolate`} | ||
| onClick={handleRowClick} | ||
| onDoubleClick={handleRowDoubleClick} | ||
| onKeyDown={handleRowKeyDown} | ||
| onContextMenu={handleRowContextMenu} | ||
| > | ||
|
|
@@ -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; | ||
|
|
@@ -801,6 +813,7 @@ const SidebarProjectThreadList = memo(function SidebarProjectThreadList( | |
| renamingThreadKey, | ||
| renamingTitle, | ||
| setRenamingTitle, | ||
| startThreadRename, | ||
| renamingInputRef, | ||
| renamingCommittedRef, | ||
| confirmingArchiveThreadKey, | ||
|
|
@@ -852,6 +865,7 @@ const SidebarProjectThreadList = memo(function SidebarProjectThreadList( | |
| renamingThreadKey={renamingThreadKey} | ||
| renamingTitle={renamingTitle} | ||
| setRenamingTitle={setRenamingTitle} | ||
| startThreadRename={startThreadRename} | ||
| renamingInputRef={renamingInputRef} | ||
| renamingCommittedRef={renamingCommittedRef} | ||
| confirmingArchiveThreadKey={confirmingArchiveThreadKey} | ||
|
|
@@ -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; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mobile sidebar closes before renameMedium Severity On mobile, the first tap of a double-tap still runs 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; | ||
|
|
@@ -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); | ||
|
|
@@ -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; | ||
| } | ||
|
|
||
|
|
@@ -1991,6 +2014,7 @@ const SidebarProjectItem = memo(function SidebarProjectItem(props: SidebarProjec | |
| markThreadUnread, | ||
| memberProjectByScopedKey, | ||
| project.cwd, | ||
| startThreadRename, | ||
| ], | ||
| ); | ||
|
|
||
|
|
@@ -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} | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium
t3code/apps/web/src/components/Sidebar.tsx
Line 595 in ba0abbd
Double-clicking inside the rename input to select text bubbles
dblclicktoSidebarMenuSubButton, triggeringhandleRowDoubleClickwhich resetsrenamingTitletothread.titleand discards the user's edits. The input'sonClickhandler only stopsclickpropagation, notdblclick. Consider addingonDoubleClick={stopPropagation}to the rename input to prevent the event from reaching the row button.🤖 Copy this AI Prompt to have your agent fix this: