fix: open file picker when Browse files is clicked in Dropper (#1261)#1395
fix: open file picker when Browse files is clicked in Dropper (#1261)#1395kairavb wants to merge 2 commits into
Conversation
Browse files stopped propagation and called onBrowse(), which is a no-op outside a wired VS Code host. Use openFilePicker() instead so the button matches click-to-upload on the drop zone. Co-authored-by: Cursor <cursoragent@cursor.com>
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID. Do not edit or delete. |
📝 WalkthroughWalkthroughThe browse button now renders from a boolean prop and opens the internal file picker directly. The container no longer passes a native-dialog callback and instead controls browse-button visibility with a parent-window check. ChangesDropper browse button flow
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dropper-ui/src/components/DropZone.tsx (1)
162-170: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the
onBrowsenative-dialog path for this button
The button is only shown whenonBrowseis provided for the VS Code/Cursor webview case, but it now always callsopenFilePicker(). That makes therequestFileDialogfallback unreachable from the browse button; useonBrowse()when present and fall back toopenFilePicker()otherwise.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dropper-ui/src/components/DropZone.tsx` around lines 162 - 170, The Browse files button in DropZone.tsx currently always calls openFilePicker(), which bypasses the native-dialog path exposed by onBrowse. Update the button’s onClick handler in the browse button render block to call onBrowse() when that prop is available and only fall back to openFilePicker() otherwise, so the VS Code/Cursor webview flow still uses requestFileDialog through onBrowse.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/dropper-ui/src/components/DropZone.tsx`:
- Around line 162-170: The Browse files button in DropZone.tsx currently always
calls openFilePicker(), which bypasses the native-dialog path exposed by
onBrowse. Update the button’s onClick handler in the browse button render block
to call onBrowse() when that prop is available and only fall back to
openFilePicker() otherwise, so the VS Code/Cursor webview flow still uses
requestFileDialog through onBrowse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f90714c3-2d68-47a6-a3d6-68d8047ca4ac
📒 Files selected for processing (1)
apps/dropper-ui/src/components/DropZone.tsx
kairavb
changed the title
dsapandora
left a comment
dsapandora
left a comment
There was a problem hiding this comment.
Nice small, surgical fix — exactly the kind of one-liner that's easy to review and easy to trust given the manual iframe repro you did.
Swapping onBrowse() for openFilePicker() on the Browse files button makes sense given the bug: onBrowse was presumably meant as a host-callback hook that's a no-op unless something's wired up outside the webview, so routing through the same openFilePicker() used by the drop zone click is the right fix and keeps both entry points consistent.
One thing worth a quick check before merging: is onBrowse (and the DropZoneProps.onBrowse prop) still used anywhere else in this component or by callers? If this was its only call site, it's now dead code and worth either removing the prop or leaving a note on why it's kept around (e.g. for a future host integration). Not a blocker, just don't want an unused prop/lint warning to sneak in.
Replace the onBrowse prop with showBrowseButton now that Browse files uses openFilePicker(). Drop the unused VS Code native dialog helper. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dropper-ui/src/components/DropperContainer.tsx (1)
253-254: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the orphaned
nativeFilesSelectedlistener
DropperContainer.tsxstill listens forwindow.parentmessages of typenativeFilesSelected, but the removedrequestNativeFileDialogpath was the only sender from this component. With no in-repo emitter for that message, the effect is dead code and should be removed with the rest of the browse-dialog cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dropper-ui/src/components/DropperContainer.tsx` around lines 253 - 254, The `DropperContainer` message listener still handles `nativeFilesSelected` even though the related `requestNativeFileDialog` flow was removed, so this effect is now dead code. Remove the orphaned `window.parent` message handling from `DropperContainer.tsx`, including any `useEffect`/handler logic and cleanup tied to `nativeFilesSelected`, and keep only the remaining browse-dialog behavior that is still referenced by this component.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/dropper-ui/src/components/DropperContainer.tsx`:
- Around line 253-254: The `DropperContainer` message listener still handles
`nativeFilesSelected` even though the related `requestNativeFileDialog` flow was
removed, so this effect is now dead code. Remove the orphaned `window.parent`
message handling from `DropperContainer.tsx`, including any `useEffect`/handler
logic and cleanup tied to `nativeFilesSelected`, and keep only the remaining
browse-dialog behavior that is still referenced by this component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3c74470b-7932-4a6d-928e-56f57e18cde1
📒 Files selected for processing (2)
apps/dropper-ui/src/components/DropZone.tsxapps/dropper-ui/src/components/DropperContainer.tsx
|
Addressed (grep -R "onBrowse" returns no matches or any search) — onBrowse was no longer called after switching the button to openFilePicker(). Replaced it with a showBrowseButton flag and removed the unused VS Code native-dialog helper. |
|
Thanks for investigating the Dropper Browse-files behavior, @kairavb. The underlying issue is now addressed by merged PR #1235, which provides the native file-dialog round trip used by the embedded VS Code flow. This PR would replace that callback path and risk regressing uploads, so it should be closed as superseded with your contribution credited. |
4 participants
Browse files stopped propagation and called onBrowse(), which is a no-op outside a wired VS Code host. Use openFilePicker() instead so the button matches click-to-upload on the drop zone.
Summary
onBrowse()openFilePicker()so the button opens the same native file picker as clicking the drop zone.Type
Testing
/tmp/dropper-iframe.html), Browse files now opens picker./builder dropper-ui:build --forcepassesChecklist
developLinked Issue
Fixes #1261
Summary by CodeRabbit