Sign up
View stargazersSign up

fix(web): sidebar search results can be dragged onto the composer#28016

merged3
main
GitHub fork: fix/issue-28015
opened10d agobyRroot-daemon
2 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

Sidebar search results can't be dragged onto the composer, even though rows in the regular sidebar list can be dropped there as thread context. Fixes #28015.

Change

  • Wrap the search results list in its own DndContext. It reuses the sidebar's existing SidebarPointerSensor, along with its context-drag move, drop and finish handlers, and records the dragged key on drag start. Auto-scroll is off because search results can't be reordered.
  • SidebarSearchResultRow registers with useDraggable and spreads only its listeners onto the row button. It doesn't take dnd-kit's attributes, so the row keeps its role="option" listbox semantics.
  • pointerOutsideThreadList measures whichever list is mounted. Search results replace the thread list, so before this change no context drag could start from search.

A plain click still opens the thread (a drag only starts past the existing 6px threshold), and dragging one row of a multi-selection still brings the whole selection, as in the main list. Search only returns live, unarchived thread shells, so the composer's readThreadShell lookup resolves every dropped result.

Scope and approval

A small, focused fix to one entry point of an existing capability, dragging a thread onto the composer. The composer guide promises that drag on web and desktop. Bug report: #28015. Related but separate: #26676.

Verification

Manual check in a real web client (vp run dev, Chromium through T3's Browser panel). I seeded a project and three Replay threads, searched the sidebar for "billing", and dragged the one result onto the composer.

Before (main)After (this branch)
beforeafter
The drag never starts and the composer stays empty. RecordingThe result drops in as a "Refactor billing webhook" thread chip. Recording
  • To get the "before" state, I hot-swapped main's versions of the two files into the same dev server and repeated the drag.
  • Clicking a search result still opens that thread (searched "flaky", clicked, and the route and breadcrumb switched to it).
  • Dragging from the regular sidebar list still drops a chip as before.
  • On drop, React logs "flushSync was called from inside a lifecycle method". The existing main-list drag logs the same error, so it comes from the composer's insert path and this change doesn't introduce it.
  • vp run check:changed: format, lint, knip, typecheck (2 packages) and the React compiler check pass. In the test step, 124 tests across 2 files pass, plus 2 tests in another run.
  • Not run: the browser test files in that step. Playwright doesn't support the author's OS ("Playwright does not support chromium on ubuntu26.04-x64"), so CI will be the first run.

🤖 Generated with Claude Code (Claude Opus 5.5)

Checks

23 successful · 13 skipped
All checks passed
8047432

Added 1 commit

  • fix(web): sidebar search results can be dragged onto the composerfe403d6
CcoderabbitaiBot10d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between fe403d6044d2f18642b8287897f5dd9a147d518e and 804743213315e1650761f5ac4ffc629c867fcead.

External image Files selected for processing: 2
  • apps/web/src/components/Sidebar.tsx
  • apps/web/src/components/sidebar/SidebarThreadRow.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

Sidebar search results can now be dragged as chat context. The search-results list uses the sidebar drag sensors and serves as a fallback boundary for context-drag detection.

Changes: Sidebar Search Result Dragging

Layer / File(s)Summary
Register and detect search-result drags
apps/web/src/components/sidebar/SidebarThreadRow.tsx, apps/web/src/components/Sidebar.tsx		Search result rows register with useDraggable and attach drag listeners to their buttons. The sidebar wraps search results in a DndContext, records the active row key at drag start, and checks the results list as a fallback boundary when the regular thread list is absent.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to 80474

No merge-blocking issue was established for search-result dragging.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main change: enabling sidebar search results to be dragged onto the composer.
Description checkPassedThe description covers the problem, implementation, scope and approval basis, verification steps, observed results, screenshots, recordings, test results, and limitations.
Linked Issues checkPassedThe changes satisfy #28015. Sidebar.tsx renders search results inside a DndContext, reuses the sidebar drag handlers, records the active thread key, and measures the mounted search-results list. `…
Out of Scope Changes checkPassedThe diff changes only apps/web/src/components/Sidebar.tsx and apps/web/src/components/sidebar/SidebarThreadRow.tsx. Each change supports the search-result drag path in #28015. The changes do not i…
Full details: Linked Issues check

Explanation

The changes satisfy #28015. Sidebar.tsx renders search results inside a DndContext, reuses the sidebar drag handlers, records the active thread key, and measures the mounted search-results list. SidebarSearchResultRow uses useDraggable and keeps its listbox semantics. These changes enable search-result drops into the composer and preserve click behavior. The linked issue has no separate coding-test requirement.

Full details: Out of Scope Changes check

Explanation

The diff changes only apps/web/src/components/Sidebar.tsx and apps/web/src/components/sidebar/SidebarThreadRow.tsx. Each change supports the search-result drag path in #28015. The changes do not implement the separate folder-row work in #26676 or add unrelated behavior.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Mmaria9d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

Verified unchanged head fe403d6044d2f18642b8287897f5dd9a147d518e on Linux using the repo Playwright dependency and /usr/bin/google-chrome against vp run dev.

  • Reproduced on github/main (54c5f25249) in a separate worktree after vp i: searching Billing and dragging Billing Source Verification onto Context Destination Thread leaves the composer empty.
  • Created a real git repository, added its project through the UI, and submitted real Codex gpt-6.1-sol turns through the composer. No replay provider, mocks, fixtures, or demo data.
  • Stopped main before starting the exact PR head after vp i. Copied the stopped verification home into the isolated PR home for matching conversations. The same search-result drag inserts a thread chip and keeps the destination open.
  • Submitted that chip with a question whose answer existed only in the source conversation. Codex invoked Read thread for Billing Source Verification and returned amber otter 742 in the destination. This verifies the client, server, thread-reference tool, and real provider path.
  • Nearby behavior: clicking a search result opens its thread; regular sidebar dragging still inserts a context chip; repeated drops of the same search result leave one chip.
  • vp run check:changed passed through blacksmith-check: format, lint, both knip steps, affected typechecks, 128 tests across four files (including both browser files), and two selected React compiler coverage cases. Installed missing Chromium on the runner before the successful rerun. Existing CI reports 26 successful checks.

Before, main:

main: search-result drag leaves destination composer empty

After, exact PR head with matching conversations:

pr: search-result drag inserts Billing Source Verification context chip

real codex reads the attached source thread and returns its phrase

search-result drag interaction, 6.68 seconds

Inspected captures and verified uploaded video playback and seeking in the T3 media player. No code changes or pushes were needed.

Not verified

Native desktop shell behavior on macOS/Windows, native iOS/Android clients, relay/tunnel and cross-environment drops, multi-selection dragging, and providers other than Codex were not exercised. This pass covered the Linux web client and local server path.

merge conflict update

merged current main into 804743213315e1650761f5ac4ffc629c867fcead without rebasing or force-pushing. kept both the search-results ref and main's scroll-container ref. the diff remains the original two files, 73 additions and 52 deletions. no other feature changes.

blacksmith vp run check:changed github/main passed format, lint, both knip passes, affected typechecks, 134 tests across four sidebar files, and two compiler cases. two independent read-only reviews approved this head.

final-head linux web verification with replay conversations: search-result dragging inserts a thread chip while keeping the destination open; clicking the result opens its source. original real-provider verification above remains historical evidence.

final head: search-result context chip in destination composer

final head: search-result drag inserts context chip

inspected the captures and verified uploaded recording playback and seeking. ci initially failed the unrelated PDF-preview test keeps workspace PDF scroll and zoom after reloads, which also failed on main in run 41503034996. the failed-job rerun passed without code changes. all 29 checks are green, and git.cafe reports no conflicts on this head.

not reverified: native clients, remote connections, or a real-provider turn on the merged head.

comment media was reopened, but github did not expose the uploaded embed to the unauthenticated browser; its rendered comment embed remains unverified. the uploaded video itself was opened and playback and seeking passed.

Added 2 commits

  • chore: merge main into sidebar search drag fix282e523
  • chore: merge current main into sidebar search drag fix8047432