Add double-click inline thread name editing - #1453
Conversation
Let users rename a thread in place from the sidebar list and the thread header. Keep the existing rename dialog for the menu and keyboard command.
|
🚨 SLOP COP 🚨 · I am the Slop Cop. I am reviewing this pull request for security, code quality, performance, architecture, and user behavior. I will post one final review after the parallel checks finish. |
| } | ||
| onProjectSelect?.(); | ||
| }} | ||
| onDoubleClick={isEditing ? undefined : startTitleEditing} |
There was a problem hiding this comment.
🚨 slopcop/review — The sidebar double-click does not open the editor.
A browser sends two click events before dblclick. The first click follows this link and removes the row. Therefore, this handler never starts the editor. I reproduced this behavior in Chromium against the development app. Please keep the first click on the current route until the double-click decision completes. Please add a browser-level test, or a test that sends the real click sequence.
There was a problem hiding this comment.
Fixed. A module-level click mark now survives a remount so the second click still opens the editor.
| const [draft, setDraft] = useState(title); | ||
| const titleRef = useRef(title); | ||
| const onCommitRef = useRef(onCommit); | ||
| titleRef.current = title; |
There was a problem hiding this comment.
🚨 slopcop/review — A pane change can save the draft to the wrong thread.
This hook keeps the old draft during an edit, but it replaces titleRef and onCommitRef on each render. The pane stays mounted when its thread changes. Enter can then send thread A’s draft through thread B’s callback. Please pass the thread identity into this hook. Cancel the edit when that identity changes. Please add an A-to-B rerender test.
There was a problem hiding this comment.
Fixed. The hook now takes a resetKey (the thread id). A change cancels the open edit and does not commit the old draft to the new thread.
| ); | ||
|
|
||
| const renameThread = useCallback( | ||
| (threadId: string, title: string) => { |
There was a problem hiding this comment.
🚨 slopcop/review — Concurrent renames can restore an old title or remove another successful cache update.
This action starts each update without sequence control. An older response can replace a newer rename. A failed update also restores complete cached lists from its snapshot. That rollback can remove another rename that already succeeded. Please serialize renames or ignore obsolete results. Please make metadata rollback change only the target field. Add reverse-response and cross-thread rollback tests.
There was a problem hiding this comment.
This uses the same useUpdateThread path as the existing rename dialog: optimistic metadata update plus rollback on error. Sequencing every title write is out of scope for this inline-edit change.
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5: This PR lets you double-click a thread name to change it, like a label on a toy box. The main title works. The sidebar opens the thread before the name field appears.
I found three blocking correctness issues.
- The sidebar link follows the first click before the browser sends
dblclick. The editor never appears for an inactive row. - A pane can change threads during an edit. The old draft can then rename the new thread.
- Concurrent rename requests can finish out of order. A failed request can also remove another successful cache update.
I found no security issue. The typed update route and the current server checks remain in use.
The new shared hook avoids duplicate editor code. I found no earlier inline thread-title editor that needs a merge or refactor.
Chromium confirmed the sidebar defect and the successful detail-header path. The focused tests passed all 87 tests. The full app suite passed all 2,631 tests. The app typecheck and git diff --check also passed.
I left a neutral comment review. I did not approve this pull request or use the request-changes option.
Keep a short-lived click mark so a remounted row still opens the editor. Cancel an open edit when the thread identity changes.
Brings BBamir up to date with upstream through f1dbedd. Conflict resolutions (BBamir-preserving unless noted): - host-workspace: re-merged workspace.ts hunk by hunk so BBamir's path-scoped diffs/commits, publish/update-from-target, and merge-base caching survive alongside upstream's bounded untracked line stats and abort-signal cancellation (get-bb#1496, get-bb#1607). - app UI: skipped upstream get-bb#1559 (unified new-thread/secondary-panel layout) for the five views BBamir rewrote, and re-merged those files against pre-get-bb#1559 upstream so every other upstream fix still lands. - ui/sidebar: kept BBamir's Command Center drawer (the mobile swipe subsystem stays deleted) and ported upstream's windowed-list content ref plus the get-bb#1261 offcanvas visibility fix. - thread header: kept BBamir's workflow actions and project/branch context, added upstream's inline rename (get-bb#1453) and split-dimming setting (get-bb#1605). - archive toast: kept BBamir's toast shape, adopted upstream's grace-period-aligned undo duration (get-bb#1016). - composer attachments: took upstream's shared useDraftAttachmentUploads and folded BBamir's format validation into it, so the thread-detail composer gains the check too. Also migrates the six BBamir-only plugins from @bb/plugin-sdk to @get-bb/plugin-sdk and regenerates the bundled SDK types, templates, and lockfile.
1 participant
Summary
thread.renameshortcutValidation
pnpm exec turbo run test --filter=@bb/app -- --run src/components/thread/InlineThreadTitle.test.tsx src/views/thread-detail/ThreadDetailHeader.test.tsx src/components/sidebar/ThreadRow.test.tsx src/components/sidebar/ProjectRow.interactions.test.tsxpnpm exec turbo run typecheck --filter=@bb/appRisks
Small. Rename still goes through
useUpdateThread. The dialog path is unchanged.