Support file tree path drops into terminal tabs#296
Conversation
✅ Deploy Preview for grand-selkie-225f87 canceled.
|
|
Warning Review limit reached
More reviews will be available in 27 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR adds support for dragging file-tree paths into terminal tabs/panes and writing those paths into the target PTY session.
Changes:
- Added shared file-tree terminal drop payload helpers.
- Wired file-tree drag start/end handling to dispatch terminal drop events.
- Added terminal tab/pane drop targets and tests for payload/drop handling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/shared/lib/fileTreeTerminalDrop.ts |
Defines the drag payload format, serialization helpers, target lookup, and terminal input formatting. |
src/shared/lib/fileTreeTerminalDrop.test.ts |
Adds coverage for payload round-tripping and path formatting. |
src/features/projects/FileTreePanel.tsx |
Creates file-tree drag payloads and dispatches custom terminal drop events on drag end. |
src/features/projects/FileTreePanel.test.tsx |
Adds coverage for click suppression and drag payload generation. |
src/features/terminal/TerminalTabs.tsx |
Registers terminal tabs/pane as drop targets and writes dropped paths to PTY sessions. |
src/features/terminal/TerminalTabs.test.tsx |
Adds terminal drop behavior coverage. |
src/features/terminal/TabStrip.tsx |
Exposes tab element refs so terminal tabs can be marked as drop targets. |
src/test/setup.ts |
Adds a writeToPty mock for tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fireEvent.dragOver(terminalTab, { dataTransfer }); | ||
| fireEvent.drop(terminalTab, { dataTransfer }); |
| return paths | ||
| .filter((path) => path.length > 0) | ||
| .join(" "); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1abc51a1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function formatTerminalPathInput(paths: readonly string[]) { | ||
| return paths | ||
| .filter((path) => path.length > 0) | ||
| .join(" "); |
There was a problem hiding this comment.
When any dropped file or directory contains spaces, this formatter writes the raw absolute path into the PTY and separates paths with a single space, so a normal shell command like cat /root/path with spaces/file.ts is parsed as multiple arguments instead of one path. This affects common project/file names with spaces; quote or shell-escape each path before joining so the terminal receives usable path arguments.
Useful? React with 👍 / 👎.
| if (payload) { | ||
| const target = getFileTreeTerminalDropTargetAtPoint( | ||
| event.clientX, | ||
| event.clientY, | ||
| ); | ||
| target?.dispatchEvent( |
There was a problem hiding this comment.
This dispatches the terminal-drop event for any dragend that finishes over a terminal target, even if the user canceled the drag (for example by pressing Escape while hovering over the terminal); MDN documents dropEffect === "none" on dragend for canceled drags (https://developer.mozilla.org/en-US/docs/Web/API/HTML_Drag_and_Drop_API/Drag_operations#finishing_the_drag). In that scenario the path is still written into the PTY despite no drop being performed, so the handler needs to distinguish a real accepted drop from cancellation.
Useful? React with 👍 / 👎.
| fireEvent.dragOver(terminalTab, { dataTransfer }); | ||
| fireEvent.drop(terminalTab, { dataTransfer }); |
There was a problem hiding this comment.
This test fires native dragOver/drop events, but the component added in this change only listens for FILE_TREE_TERMINAL_DROP_EVENT via addEventListener, not for the browser drop event. As written, handleTerminalPathDrop never runs and writeToPtyMock remains uncalled, so the new test fails unless it dispatches the same custom event that FileTreePanel sends on drag end.
Useful? React with 👍 / 👎.
Summary
Validation