Skip to content

fix(scope): prevent browser default drop from creating empty rounds#254

Open
beruro wants to merge 7 commits into
developfrom
fix/issue-250-drag-file-empty-round
Open

fix(scope): prevent browser default drop from creating empty rounds#254
beruro wants to merge 7 commits into
developfrom
fix/issue-250-drag-file-empty-round

Conversation

@beruro

@beruro beruro commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • When an OS file was dragged onto the chat input, the browser's default contenteditable drop behaviour (inserting the filename as raw text) was not suppressed — allowing browser-injected text to corrupt the editor and, in certain timing windows, produce an empty conversation round.
  • ComposerInput/index.tsx: handleDropEvent now unconditionally calls event.preventDefault() before invoking createDropHandler. OS file drops are already handled by GlobalDragDropdroppedFilesAtomuseDroppedFilesConsumer; the browser must not also insert raw text.
  • useContainerDrag.ts: handleContainerDrop now calls event.preventDefault() + event.stopPropagation() for all drops on the composer container, stopping the event before it can reach the inner InputEditor div or the contenteditable host. Internal/reference drops are still forwarded to handleDrop as before.

Closes #250

Root Cause

createDropHandler (in pasteHandlers.ts) only handles reference pill drags and returns false for everything else — without calling event.preventDefault(). The event propagated to the contenteditable host where the browser inserted the file name as text. This corrupted the editor state and, when combined with timing around useDroppedFilesConsumer updating hasContentRef, could produce an empty conversation round.

Changes

File Change
src/components/ComposerInput/index.tsx Always call event.preventDefault() in handleDropEvent
src/engines/ChatPanel/InputArea/hooks/useContainerDrag.ts Unconditionally preventDefault + stopPropagation in handleContainerDrop; use isInternalDropType helper
src/engines/ChatPanel/InputArea/hooks/containerDropHelpers.ts New — extracted isInternalDropType pure helper
src/components/ComposerInput/__tests__/dropHandler.test.ts New — 6 unit tests for createDropHandler OS-file-drop behaviour
src/engines/ChatPanel/InputArea/hooks/__tests__/containerDropHelpers.test.ts New — 12 unit tests for isInternalDropType
src/engines/ChatPanel/InputArea/hooks/__tests__/TEST_CASES.md New — manual QA test cases

Test Plan

  • pnpm test passes (18 new tests, 0 failures)
  • TypeScript type check passes
  • Lint passes (pre-commit hook)
  • Manual: drag a .ts file from Finder onto the chat input → only a pill is inserted, no new empty conversation round appears
  • Manual: drag a file onto an active session → pill inserted into queue composer, no accidental submit
  • Manual: internal file-tree drag still works (pill inserted via handleDrop)
  • Manual: reference drag (PR/issue pill) still inserts correctly

beruro added 7 commits July 3, 2026 12:57
When an OS file is dragged onto the chat input, the browser's default
contenteditable drop behaviour (inserting the file name as raw text) was
not being suppressed. This corrupted the editor state and, in certain
timing windows, triggered an empty conversation round.

Two-part fix:
1. `ComposerInput/index.tsx` – `handleDropEvent` now calls
   `event.preventDefault()` unconditionally before invoking
   `createDropHandler`. OS file drops are already handled by GlobalDragDrop
   via the Jotai `droppedFilesAtom` → `useDroppedFilesConsumer` pill path;
   letting the browser also insert raw text was the defect.
2. `useContainerDrag.ts` – `handleContainerDrop` now calls
   `event.preventDefault()` + `event.stopPropagation()` for ALL drop events
   on the composer container, not only for internal/reference drags. Only
   internal-type drops (file-tree, application/x-file-reference, reference
   drag) are forwarded to the `handleDrop` handler.

Extracted `isInternalDropType` into `containerDropHelpers.ts` and added
unit tests covering both helpers. Added `TEST_CASES.md` for manual QA.

Closes #250

Pre-commit hook ran. Total eslint: 1, total circular: 0
- SessionFilterButton: dropdown rows → DropdownItem, trigger → IconButton
- FileHeaderMoreMenu: 8 menu rows → DropdownItem, remove MenuItemContent helper
- McpTable: bulk action bar → Button size=small variant=tertiary
- ProjectForm / ReviewerConfigSection: <select>/<textarea>/<input> → DS Select/Textarea/Input
- KiroSessionSetup: 5 icon-only buttons → IconButton / Button ghost
- BottomPanelHeader: delete 127-line commented dead code block
- DB connection fields: <input> → DS Input, extract AddConnectionFormField wrapper
- Extract shared DateRangePills component from duplicate filter sidebars

Pre-commit hook ran. Total eslint: 1, total circular: 0
- status.rs: add STATUS_CACHE (2s TTL, keyed on HEAD SHA) to skip redundant
  git-status subprocess spawns on rapid frontend polls
- numstat.rs: add NUMSTAT_CACHE (500ms TTL) to skip repeated libgit2 diff
  walks when the sidebar re-renders without any working-tree change
- watcher.rs: convert polling thread from std::thread::spawn to tokio::spawn +
  tokio::time::sleep so the poller lives on the async executor instead of
  blocking a dedicated OS thread
- useRepoStatusListener / useWorkspaceGitStatus / useFileTree: add
  document.hidden guard before processing repo:status_updated events so
  background tabs do not trigger redundant React state updates

Pre-commit hook ran. Total eslint: 1, total circular: 0
- Memoize provider values in Files/Terminal/Browser workstation contexts
  so consumers don't re-render on every Provider render.
- Extract static inline style to module const (VIRTUALIZED_BODY_STYLE)
  and memoize dynamic container style in ChatHistory.
- Stabilize WorkstationTabBar leadingSlot as a module-level element.
- Wrap ChatBubble primitives (Avatar/Header/Body/Layout) and
  AgentChatItemDefault with React.memo.
- Optimize VirtualizedSessionList: memoized SessionRow, stable
  itemContent via Virtuoso (index, data) signature, memoized style
  and formatted timestamp.
- Stabilize VirtualizedStickyTree itemContent / computeItemKey with
  useCallback so Virtuoso's internal memo can hit.
- Memoize FileRow in ChangedFilesList.
- Adopt React 19 useDeferredValue on filteredFiles in SourceControlContent
  so the filter input stays responsive while the virtualized tree
  recomputes at lower priority.

Pre-commit hook ran. Total eslint: 1, total circular: 0
- Fix off-by-one in terminal pill label: call `.trimEnd()` before
  `.split("\n")` so trailing newlines don't inflate lineCount
  (useAtMention, useAddToAgentInsertion, useComposerInput)
- Rename `checkout` → `checkoutRaw` in branchOps / GitOperationsService
  to make the unguarded path explicit and prevent accidental UI use
- Two-step create-then-checkout in gitCreateBranchFromCommit: pass
  `checkout: false` to backend create, then route through
  `checkoutWithDialog` so dirty-tree conflicts surface the dialog
- Narrow TextSelectionDropdown primary width 280→180px, submenu 280→240px

Pre-commit hook ran. Total eslint: 1, total circular: 0
- Add lineStart/lineEnd to TerminalSelectionInfo, SelectionState,
  AddToAgentRequest, and __orgiiLastTerminalCopy for end-to-end flow
- terminalHandlers: call getSelectionPosition() and convert 0-based
  xterm rows to 1-based for display (lineStart/lineEnd)
- TerminalCore: thread lineStart/lineEnd through selection state, copy
  handler, and handleAddToChat; add to useEffect dep array; clear on
  selection hide to prevent stale position leakage into copy handler
- useAddToAgentInsertion: use real buffer positions for pill label when
  available, fall back to counting text lines for legacy callers
- pasteHandlers: prefer real lineStart/lineEnd from __orgiiLastTerminalCopy
  when available, fall back to lineCount range for legacy paste paths

Pre-commit hook ran. Total eslint: 1, total circular: 0
Resolve conflicts for PR #254.

Pre-commit hook ran. Total eslint: 1, total circular: 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(scope): Drag file to the chat input will create a new empty round

1 participant