Fix backend drag and drop on Linux #3949
Conversation
WalkthroughThe changes in this pull request focus on enhancing the drag-and-drop functionality within the application. Key modifications include the introduction of an Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v2/internal/frontend/runtime/desktop/draganddrop.js(2 hunks)v2/internal/frontend/runtime/runtime_debug_desktop.js(1 hunks)v2/pkg/runtime/draganddrop.go(1 hunks)
🔇 Additional comments (2)
v2/pkg/runtime/draganddrop.go (1)
14-16: LGTM! Event emission coordinates frontend initialization.
The addition of EventsEmit for "wails:init-file-drop" ensures proper initialization sequence between backend and frontend.
Let's verify the event handling coordination:
✅ Verification successful
Event coordination between backend and frontend is properly implemented
The verification confirms proper event handling coordination:
- Backend emits "wails:init-file-drop" event in
draganddrop.go - Frontend listens for this event using
EventsOncein bothdraganddrop.jsandruntime_debug_desktop.js - Upon receiving the event, frontend registers drag-and-drop event listeners (
dragover,dragleave,drop) only if the feature flag is enabled
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify event handling coordination between backend and frontend
# Test: Search for event handler registration. Expect: Event emission in backend and handler in frontend.
# Check backend event emission
rg -A 2 "wails:init-file-drop"
# Check frontend event handling
ast-grep --pattern 'EventsOnce("wails:init-file-drop", $_)'
Length of output: 2254
v2/internal/frontend/runtime/desktop/draganddrop.js (1)
247-254: LGTM! Well-structured event handling initialization.
The implementation:
- Uses EventsOnce for one-time registration
- Properly checks enableWailsDragAndDrop flag
- Reuses existing handler functions
| EventsOnce("wails:init-file-drop", () => { | ||
| if (window.wails.flags.enableWailsDragAndDrop) { | ||
| window.addEventListener("dragover", onDragOver); | ||
| window.addEventListener("dragleave", onDragLeave); | ||
| window.addEventListener("drop", onDrop); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Remove duplicate event handler registration.
This event handler registration is identical to the one in draganddrop.js. Having duplicate handlers could cause issues and makes maintenance more difficult.
Apply this diff to remove the duplicate code:
- EventsOnce("wails:init-file-drop", () => {
- if (window.wails.flags.enableWailsDragAndDrop) {
- window.addEventListener("dragover", onDragOver);
- window.addEventListener("dragleave", onDragLeave);
- window.addEventListener("drop", onDrop);
- }
- });
|
@atterpac - any thoughts on this? |
|
Any chance this could be merged soon? Thanks. |
|
Ok I am seeing one issue with this PR so far, which is that it doesn't seem to work properly with a built binary (
I don't get why this would happen. Maybe related to #3563? It would be good if someone else could confirm this. |
|
I can try to look at this this week at some point. Perhaps @atterpac could look at it if he has time. |
I can look into it |
Description
This PR fixes #3686 by copying a dead PR, #3704.
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version: Arch Linux latest
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRI don't fully understand this PR (hopefully you do instead) but the basic reason why it works is because it registers event listeners that call
e.PreventDefault()so the default file drop behaviour is stopped.This requires doc updates because callers need to know that backend file drop handlers should be registered in
OnDomReady, notOnStartup.Summary by CodeRabbit
New Features
Bug Fixes