feat: drag-and-drop attachment reordering#709
Conversation
Attachments in the composer strip can now be reordered by dragging them to a new position. Visual feedback: dragged item fades to 40% opacity, drop target shows a dashed outline. Changes: - ChatInput: dragIndex/dragOverIndex state, dragStart/dragOver/ drop/dragEnd handlers, attachment reorder via array splice - main.css: .attachment-drag-wrapper (grab cursor), .attachment- dragging (faded), .attachment-drag-over (dashed outline)
Greptile SummaryThis PR adds drag-and-drop reordering to the attachment strip in the chat composer, plus a handful of correctness fixes bundled in: an
Confidence Score: 4/5Safe to merge with a follow-up fix for the session-resume dedup path in Layout.tsx. The session-resume dedup guard in Layout.tsx correctly prevents a duplicate run from entering state, but then calls setActiveRunId with the newly minted (and discarded) run's ID. In the race condition the guard is designed to handle, the active run would point at an entry that does not exist in runs, leaving the user on a blank or broken chat tab. All other changes — the DnD feature, IPC guard, and contextTokens fixes — look correct. src/renderer/src/screens/Layout/Layout.tsx — the setActiveRunId call after the dedup check needs to use the existing run's ID, not the discarded one. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| onDragOver={(e) => handleDragOver(e, i)} | ||
| onDrop={(e) => handleDrop(e, i)} | ||
| onDragEnd={handleDragEnd} |
There was a problem hiding this comment.
dragOverIndex is only updated when the cursor enters a new wrapper element, so once the cursor exits all attachment wrappers (e.g., moves into the textarea) the outline stays painted on the last-hovered chip for the remainder of the drag. Adding onDragLeave to reset dragOverIndex to null clears it immediately.
| onDragOver={(e) => handleDragOver(e, i)} | |
| onDrop={(e) => handleDrop(e, i)} | |
| onDragEnd={handleDragEnd} | |
| onDragOver={(e) => handleDragOver(e, i)} | |
| onDragLeave={() => setDragOverIndex(null)} | |
| onDrop={(e) => handleDrop(e, i)} | |
| onDragEnd={handleDragEnd} |
| attachment={att} | ||
| onRemove={() => removeAttachment(att.id)} | ||
| /> | ||
| className={`attachment-drag-wrapper${dragOverIndex === i ? " attachment-drag-over" : ""}${dragIndex === i ? " attachment-dragging" : ""}`} |
There was a problem hiding this comment.
When dragOverIndex === dragIndex === i (the cursor re-enters the source element during a drag), the wrapper receives both attachment-dragging (faded) and attachment-drag-over (dashed outline) at the same time. The outline only makes sense for valid other drop targets; guarding the condition with dragIndex !== i prevents the visual conflict.
| className={`attachment-drag-wrapper${dragOverIndex === i ? " attachment-drag-over" : ""}${dragIndex === i ? " attachment-dragging" : ""}`} | |
| className={`attachment-drag-wrapper${dragOverIndex === i && dragIndex !== i ? " attachment-drag-over" : ""}${dragIndex === i ? " attachment-dragging" : ""}`} |
| function handleDrop(e: React.DragEvent, index: number): void { | ||
| e.preventDefault(); | ||
| if (dragIndex === null || dragIndex === index) return; | ||
| setAttachments((prev) => { | ||
| const next = [...prev]; | ||
| const [moved] = next.splice(dragIndex, 1); | ||
| next.splice(index, 0, moved); | ||
| return next; | ||
| }); | ||
| } |
There was a problem hiding this comment.
handleDrop reorders the attachments but leaves dragIndex and dragOverIndex set. The onDragEnd event does fire after onDrop per the HTML5 DnD spec and will clear them, but calling handleDragEnd() directly inside handleDrop makes the cleanup unconditional and symmetrical, and avoids any edge-case flash if onDragEnd is delayed or skipped in unusual browser environments.
| function handleDrop(e: React.DragEvent, index: number): void { | |
| e.preventDefault(); | |
| if (dragIndex === null || dragIndex === index) return; | |
| setAttachments((prev) => { | |
| const next = [...prev]; | |
| const [moved] = next.splice(dragIndex, 1); | |
| next.splice(index, 0, moved); | |
| return next; | |
| }); | |
| } | |
| function handleDrop(e: React.DragEvent, index: number): void { | |
| e.preventDefault(); | |
| if (dragIndex === null || dragIndex === index) return; | |
| setAttachments((prev) => { | |
| const next = [...prev]; | |
| const [moved] = next.splice(dragIndex, 1); | |
| next.splice(index, 0, moved); | |
| return next; | |
| }); | |
| handleDragEnd(); | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…falsy Problem: contextTokens used || (logical OR) which treated 0 as falsy. When backend sent promptTokens=0 (or stopped sending), the gauge kept the stale previous value and never updated. Root Cause: The || operator falls through to previous value when the new value is 0, null, or undefined. But 0 is a valid value (empty context after session switch). Fix: Use ?? (nullish coalescing) so only null/undefined fall through. Also removed || undefined wrapper in usageFromPayload. Files: - useChatIPC.ts L300: || → ?? - useDashboardChatTransport.ts L886: || → ?? - useDashboardChatTransport.ts L596: removed || undefined
…turn contextTokens Problem: cacheHitPct = cacheReadTokens / used where 'used' is the latest turn's prompt tokens. Cache tokens can span multiple turns, so 307k cache / 14k used = 2196% → clamped to 100% — nonsensical '100% hit' display. Fix: pass cumulative promptTokens from Chat.tsx to ContextGauge. Cache % = cacheReadTokens / promptTokens (session total). Falls back to 'used' when promptTokens is unavailable.
When a session is opened twice before the first fetch completes, both async handlers could create separate runs for the same sessionId, resulting in two tabs showing identical content that appears 'mixed'. The resumingRef guard only covers concurrent async calls, but a second click after the first fetch lands but before React commits the state update could still create a duplicate. The setRuns functional updater now checks for an existing run with the same sessionId.
…ort active Root cause: Both useChatIPC and useDashboardChatTransport hooks register IPC event listeners and DB polling simultaneously. When two agent sessions run in parallel, both hooks process events and call setMessages — the duelling state updates can cause messages from one session to leak into another via reconciliation races. Fix: Add enabled flag to useChatIPC, set to false when dashboard transport is handling the chat (dashboardChatEnabled=true). This eliminates the dual-listener conflict while preserving legacy IPC for non-dashboard mode.
Problem
Root Cause
Changes
Verification
|
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
| setRuns((prev) => { | ||
| // Defend against duplicate opens: if another run for this session | ||
| // already landed while we were fetching, switch to it instead. | ||
| const existing = prev.find((r) => r.sessionId === sessionId); | ||
| if (existing) return prev; | ||
| return [...prev, run]; | ||
| }); | ||
| setActiveRunId(run.runId); | ||
| goTo("chat"); |
There was a problem hiding this comment.
When the setRuns functional updater finds an existing run for the session, it correctly discards the newly-minted run — but setActiveRunId(run.runId) is still called with the discarded run's ID, which was never inserted into state. Any downstream lookup of activeRunId will resolve to undefined, leaving the user on a blank or broken chat tab. The dedup comment says "switch to it instead", so existing.runId is the intended target.
The fix requires capturing existing outside the functional updater. One approach: run the dedup check before calling setRuns, read existing from the current snapshot, and use it to decide which ID to activate. Note: runs is captured in the useCallback dependency array, so it is current at the time handleResumeSession fires.
1 participant
Drag-and-drop reorder attachments in the composer strip. Dragged item fades, drop target shows dashed outline.