feat: support dropping file paths into composer#1722
Conversation
|
| Filename | Overview |
|---|---|
| packages/app/src/file-drops/file-drop-paths.ts | New module classifying dropped paths by extension; logic is correct but getFileExtension is duplicated from file-types.ts and IMAGE_MIME_BY_EXTENSION partially duplicates RASTER_IMAGE_MIME_TYPE_BY_EXTENSION. |
| packages/app/src/file-drops/file-drop-paths.test.ts | Tests cover the public module interface (extraction, classification, mime resolution, text formatting) with clear setup/assert shape and no banned patterns. |
| packages/app/src/hooks/use-file-drop-zone.ts | Adds onTextDropped wiring to both IPC and DOM drop paths; the DOM path silently drops non-image files when onTextDropped is absent but onGenericFilesDropped is present (existing consumers like adaptive-modal-sheet.tsx are affected). |
| packages/app/src/components/file-drop-zone.tsx | Adds onTextDropped prop and passes it through to useFileDropZone; otherwise removes dead comments. |
| packages/app/src/panels/agent-panel.tsx | Adds handleTextDropped callback and passes onTextDropped to FileDropZone; simplifies handleGenericFilesDropped to filter only web-file items, consistent with the new routing. |
| packages/app/src/composer/draft/workspace-tab.tsx | Adds handleTextDropped wired to onTextDropped; same pattern as the other two surfaces. |
| packages/app/src/screens/new-workspace-screen.tsx | Adds handleTextDropped and wires onTextDropped to FileDropZone; comment on handleClearDraft improved to English. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[File Drop Event] --> B{Electron IPC or DOM?}
B -->|Desktop IPC| C[resolveDroppedFilePaths]
B -->|DOM fallback| D[extractDroppedFilePaths\n+ resolveDroppedFilePaths]
C --> E{droppedPaths.text?}
D --> F{droppedPaths.text?}
E -->|yes| G{onTextDropped registered?}
G -->|yes| H[onTextDropped fires]
G -->|no| I[drop silently lost]
E -->|no| J{onTextDropped absent + onGenericFilesDropped?}
J -->|yes| K[onGenericFilesDropped fires]
F -->|yes| M{onTextDropped registered?}
M -->|yes| N[onTextDropped fires]
M -->|no| O[drop silently lost]
F -->|no| P[onGenericFilesDropped fires]
E --> Q[imagePaths → onFilesDropped]
F --> R[isRasterImageFile → onFilesDropped]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[File Drop Event] --> B{Electron IPC or DOM?}
B -->|Desktop IPC| C[resolveDroppedFilePaths]
B -->|DOM fallback| D[extractDroppedFilePaths\n+ resolveDroppedFilePaths]
C --> E{droppedPaths.text?}
D --> F{droppedPaths.text?}
E -->|yes| G{onTextDropped registered?}
G -->|yes| H[onTextDropped fires]
G -->|no| I[drop silently lost]
E -->|no| J{onTextDropped absent + onGenericFilesDropped?}
J -->|yes| K[onGenericFilesDropped fires]
F -->|yes| M{onTextDropped registered?}
M -->|yes| N[onTextDropped fires]
M -->|no| O[drop silently lost]
F -->|no| P[onGenericFilesDropped fires]
E --> Q[imagePaths → onFilesDropped]
F --> R[isRasterImageFile → onFilesDropped]
Reviews (2): Last reviewed commit: "feat: support dropping file paths into c..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }), | ||
| }); | ||
| const agentInputText = agentInputDraft.text; | ||
| const setAgentInputText = agentInputDraft.setText; | ||
| const handleTextDropped = useCallback( | ||
| (text: string) => { | ||
| setAgentInputText( | ||
| appendDroppedFilePathText({ | ||
| currentText: agentInputText, | ||
| droppedText: text, | ||
| }), | ||
| ); | ||
| }, | ||
| [agentInputText, setAgentInputText], | ||
| ); |
There was a problem hiding this comment.
handleTextDropped callback duplicated across three call sites
The same 10-line useCallback + appendDroppedFilePathText pattern is copy-pasted into agent-panel.tsx, workspace-tab.tsx, and new-workspace-screen.tsx. Per the codebase's own rule: extract when called from at least two unrelated call sites. A small hook like useFileDropText(setText) would own the pattern once and keep all three callers to a single line. The duplication is benign today but will quietly drift if appendDroppedFilePathText's calling convention ever changes.
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
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!
|
|
||
| const draftText = draftInput.text; | ||
| const handleTextDropped = useCallback( | ||
| (text: string) => { | ||
| setDraftText( | ||
| appendDroppedFilePathText({ | ||
| currentText: draftText, | ||
| droppedText: text, | ||
| }), | ||
| ); | ||
| }, | ||
| [draftText, setDraftText], |
There was a problem hiding this comment.
currentText captured in closure rather than using a functional updater
handleTextDropped closes over draftText, making draftText a useCallback dependency. Every keystroke recreates the callback. The ref-update useEffect in useFileDropZone then fires after each re-render to stay in sync, adding a tiny staleness window. The same pattern exists in agent-panel.tsx and new-workspace-screen.tsx. If setDraftText supports a functional updater (prev) => nextValue, preferring it would remove draftText from the deps array and produce a stable callback with no staleness risk.
Fitzzzzzz
force-pushed
the
codex/file-drop-path-text
branch
from
6080618 to
24e7bae
Compare
1 participant
Summary
Verification
npx vitest run packages/app/src/file-drops/file-drop-paths.test.ts --bail=1npm run lint -- packages/app/src/hooks/use-file-drop-zone.ts packages/app/src/components/file-drop-zone.tsx packages/app/src/panels/agent-panel.tsx packages/app/src/composer/draft/workspace-tab.tsx packages/app/src/screens/new-workspace-screen.tsx packages/app/src/file-drops/file-drop-paths.ts packages/app/src/file-drops/file-drop-paths.test.tsnpm run build:app-depsnpm run typecheck --workspace=@getpaseo/appnpm run build:servergit diff --check