Feature/1090 native drag and drop for file and folder#3203
Conversation
|
Important Auto Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe recent updates to the codebase introduce native drag and drop functionality for files and folders, providing absolute paths without reliance on IE11. This enhancement streamlines the process across different browsers and platforms, specifically targeting Windows and Linux systems within the Wails framework, ensuring a more consistent and modern user experience. Changes
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (17)
- v2/internal/frontend/desktop/linux/frontend.go (1 hunks)
- v2/internal/frontend/desktop/linux/window.c (3 hunks)
- v2/internal/frontend/desktop/linux/window.go (1 hunks)
- v2/internal/frontend/desktop/linux/window.h (1 hunks)
- v2/internal/frontend/desktop/windows/frontend.go (5 hunks)
- v2/internal/frontend/dispatcher/dispatcher.go (2 hunks)
- v2/internal/frontend/dispatcher/draganddrop.go (1 hunks)
- v2/internal/frontend/runtime/desktop/draganddrop.js (1 hunks)
- v2/internal/frontend/runtime/desktop/main.js (4 hunks)
- v2/internal/frontend/runtime/runtime_prod_desktop.js (1 hunks)
- v2/internal/frontend/runtime/wrapper/runtime.d.ts (1 hunks)
- v2/internal/frontend/runtime/wrapper/runtime.js (1 hunks)
- v2/pkg/options/options.go (3 hunks)
- v2/pkg/runtime/draganddrop.go (1 hunks)
- website/docs/reference/options.mdx (2 hunks)
- website/docs/reference/runtime/draganddrop.mdx (1 hunks)
- website/src/pages/changelog.mdx (1 hunks)
Additional comments: 28
v2/internal/frontend/dispatcher/draganddrop.go (1)
- 9-37: The implementation of
processDragAndDropMessagecorrectly parses the message, validates the data, and emits awails:file-dropevent with the coordinates and paths.v2/pkg/runtime/draganddrop.go (1)
- 8-33: The functions
OnFileDropandOnFileDropOffare correctly implemented to manage drag and drop event listeners and callbacks.v2/internal/frontend/dispatcher/dispatcher.go (1)
- 49-50: The addition of the case for 'D' messages in
ProcessMessagecorrectly delegates the processing of drag and drop messages to theprocessDragAndDropMessagemethod.website/docs/reference/runtime/draganddrop.mdx (1)
- 1-42: The documentation in
draganddrop.mdxis clear and provides the necessary details for using the drag and drop functionality in the Wails framework.v2/internal/frontend/desktop/linux/window.h (1)
- 109-109: The
SetupWebviewfunction has been correctly updated to accept additional parameters for controlling drag and drop functionality.v2/internal/frontend/runtime/wrapper/runtime.js (1)
- 202-237: The new functions
OnFileDrop,OnFileDropOff,CanResolveFilePaths, andResolveFilePathshave been correctly added to handle drag and drop events and file path resolution.v2/internal/frontend/runtime/desktop/main.js (2)
- 19-19: The import and integration of the
DragAndDropmodule into theruntimeobject are correctly implemented.- 119-122: The function
setCSSDropPropertieshas been correctly added to allow customization of CSS properties for drag and drop.v2/pkg/options/options.go (2)
- 153-164: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [98-161]
The
Appstruct has been correctly updated to include aDragAndDropfield, and theMergeDefaultsfunction has been updated to handle its properties.
- 192-207: The new
DragAndDroptype with properties related to drag and drop behavior is correctly defined and integrated into theAppstruct.v2/internal/frontend/runtime/desktop/draganddrop.js (1)
- 1-217: The functions in
draganddrop.jsfor handling drag and drop events, such asOnFileDropandOnFileDropOff, are correctly implemented.v2/internal/frontend/runtime/wrapper/runtime.d.ts (1)
- 237-249: The TypeScript declarations for the new drag and drop functions are correctly added and match the implementation in
runtime.js.v2/internal/frontend/runtime/runtime_prod_desktop.js (1)
- 1-1: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [104-461]
The changes in
runtime_prod_desktop.jsinclude renaming of variables and functions, reordering of functions, and the addition of drag and drop functionality. The drag and drop functionality is implemented as per the PR objectives, and the code changes are consistent with the expected behavior for enabling drag and drop on the frontend.v2/internal/frontend/desktop/linux/frontend.go (1)
- 445-461: The modifications to the
processMessagefunction infrontend.gocorrectly include JavaScript commands to set CSS drop properties and enable drag and drop functionality. This aligns with the PR objectives to introduce drag and drop capabilities in the Wails framework for Linux.v2/internal/frontend/desktop/linux/window.go (1)
- 104-105: The
NewWindowfunction inwindow.gohas been updated to handle drag and drop options, which is consistent with the PR's goal of adding native drag and drop support. The use ofappoptions.DragAndDrop.Disableandappoptions.DragAndDrop.EnableFileDropensures that the window creation respects the configuration for drag and drop functionality.v2/internal/frontend/desktop/linux/window.c (3)
- 7-7: The inclusion of
<string.h>is appropriate for string manipulation functions used in the drag and drop implementation.- 433-498: The implementation of drag and drop functionality with
onDragDataReceivedandonDragDropfunctions is correct and follows the GTK signal handling conventions.- 500-518: The conditional logic to enable or disable drag and drop behavior in
SetupWebviewis correct and respects thedisableWebViewDragAndDropandenableDragAndDropflags.v2/internal/frontend/desktop/windows/frontend.go (7)
- 19-19: The import of the "unsafe" package is necessary for pointer arithmetic used in the drag and drop feature.
- 465-469: The handling of drag and drop functionality in the
setupChromiumfunction with theAllowExternalDragmethod is correct.- 472-472: The addition of
processMessageWithAdditionalObjectsanddispatchMessagefunctions is appropriate for managing messages and file drops.- 684-692: The JavaScript command to set CSS properties for drag and drop is correctly formatted and executed.
- 713-714: The use of a goroutine to dispatch messages asynchronously is a good practice for non-blocking UI operations.
- 716-768: The
processMessageWithAdditionalObjectsfunction correctly handles the drag and drop messages, extracting file paths and coordinates.- 833-835: The JavaScript command to enable Wails drag and drop functionality is correctly executed during navigation completion.
website/docs/reference/options.mdx (2)
- 67-72: The addition of the
DragAndDropstruct within theOptions.Appstruct is correctly documented and follows the existing documentation style.- 537-588: The documentation for the new
DragAndDropsection is clear, well-structured, and provides all necessary details for the new feature, including default values and behavior on different platforms.website/src/pages/changelog.mdx (1)
- 21-27: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-23]
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- website/src/pages/changelog.mdx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- website/src/pages/changelog.mdx
|
@coderabbitai review |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- v2/internal/frontend/desktop/linux/window.c (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- v2/internal/frontend/desktop/linux/window.c
|
Hi @lyimmi! I was wondering if I could help you add this for MacOS. Reading from your Issue thread: But in this PR we've: I assume a solution would be possible but you haven't looked into it, or you actually meant there's no solution for MacOS? |
|
@risico I meant that, I have no solution for macOS. That would be nice if you could help with the macOS implementation! I have no device to test it on, I tried a macOS VM a few times, but it is painfully slow so I gave up on it for now. |
|
Hi @lyimmi, this is fantastics! Thank you for pushing this through. @leaanthony If this is merged do you plan to put it into v2 or v3? I am asking because my team is considering adding support for Mac (if @lyimmi can guide us a bit), but we would love to see it in production sooner then later. CC: @jakubpeleska |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- website/src/pages/changelog.mdx (1 hunks)
Additional comments: 1
website/src/pages/changelog.mdx (1)
- 25-25: The changelog entry for adding Drag & Drop support for Windows and Linux is clear and well-documented. It properly credits the contributors and is consistent with the format of the rest of the changelog.
|
@pavelbinar - I'm ok with this going into v2. The v3 solution is purely at the window level. Having it at the DOM level would be great and a far better experience. TLDR; Go for it and I'll port to v3 👍 |
|
@lyimmi - did this need any updates to |
|
@leaanthony - no I just checked out the master and everything I needed was already there, didn't touched any dependecies. @pavelbinar - I'm happy to help if I can. |
|
I'm not too familiar how github works regarding pushing changes to a PR from a fork. Should @pavelbinar push changes to my fork? Should I set this PR to draft until the mac side is done? |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (1)
- v2/pkg/runtime/draganddrop.go (1 hunks)
Additional comments: 1
v2/pkg/runtime/draganddrop.go (1)
- 33-36: The method
OnFileDropOffcorrectly removes the event listener for drag and drop. This is a straightforward and effective implementation.
|
Having fun @lyimmi ? 😉 |
|
@leaanthony Yeah. It's pretty good actually. But I'm done now... Sorry. 😁 |
|
@leaanthony - Thank you for the clarification. @lyimmi - I appreciate your willingness to help. We will get in touch within a few weeks (may we connect via email?). I will reach out to you later, once we are more familiar with the code and have some questions. Currently, I am exploring the options and preparing the work. Regarding the PR: |
|
I've created |
…into feature/1090_native_drag_and_drop_for_file_and_folder
f9df564
into
wailsapp:feature/1090_native_drag_and_drop_for_file_and_folder
|
@lyimmi 🎉 @leaanthony perfect 👌🏻 |
* Feature/1090 native drag and drop for file and folder (#3203) * implement basic dnd for linux * implemented windows * progress changed linux handling and added coordinates to drop * progress fix drop coordinates on windows * progress remove log from windows * progress move js * update js after merge * fix event listener registration * fix segfault on non file drag * remove logs, fix coordinates * minor changes, simplify to drop only * rename EnableWails -> EnableFileDrop * add documentation (PR id missing) * add PR id to changelog * fix remove casting from malloc * fix nil check for OnFileDrop's callback * fix nil check for OnFileDrop skip event when nil * add error message for nil callback in OnFileDrop --------- Co-authored-by: lyimmi <lelvente.zambo@gmail.com> Co-authored-by: Lea Anthony <lea.anthony@gmail.com> * implement native drag and drop for macOS (#3250) * implement native drag and drop for macOS * update docs * add to changelog * update docs (macOS is supported) * Fix windows DragAndDrop options * Fix class unset on dragleave for full frame elements * improve class unset (nested elements and borders case) * Fix runtime drop target detection and CSS class assignment * Edit changelog * Fix drag-and-drop options in references * Update v2/internal/frontend/desktop/darwin/WailsWebView.m * Update v2/internal/frontend/desktop/darwin/WailsWebView.m --------- Co-authored-by: Zámbó, Levente <levente.zambo@gmail.com> Co-authored-by: lyimmi <lelvente.zambo@gmail.com> Co-authored-by: Lea Anthony <lea.anthony@gmail.com> Co-authored-by: Andrey Pshenkin <andrey.pshenkin@gmail.com> Co-authored-by: Pavel Binar <pavel@beamtransfer.io>
Description
This PR implement Drag & Drop for files and folders on Windows and Linux.
On both systems the whole window is a drop target and the developer can either use the JS runtime method to handle the drops as described below, or implement the handling them self.
I tried to test all of the cases I can think of but and extensive test by multiple people would be a good idea. Or maybe somehow make it experimental for a while. I don't really know how it wold be possible with the JavaScript code/files.
Fixes #1090
Windows
The windows implementation is based on ayatkyo's work in #2774, with minor changes to align the API with the linux imlementation.
The drop event sends the coordinates and the files to the go side to resolve the file paths and after that a
wails:file-dropevent is fired to notify the listeners in GO and Javascript.Linux
On Linux I have overridden the WebKitGTK signals for drag & drop and the C code sends the coordinates and the file paths via
processMessage()to GO and than the samewails:file-dropevent is fired.macOS
No solution for macOS.
Runtime
GO
This can be used to handle the
wails:file-dropevent with a callback that handles the casting and checking the event'sinterface{}data.Remove the listener.
Javascript
Calls the callback function with the coordinates inside the window where the drag was released and a slice of absolute file paths.
When the
useDropTargetistruein addition to calling the callback when the drop happens, it registers event listeners onthe window that are listening for the drag coordinates and checks if the mouse is over an element that has the
CSSDropProperty style. If the element has the required property
it adds the
wails-drop-target-activeclass to the element's class list and removes it when the mouse moves off of it.This method removes all registered listeners and handlers for drag and drop events.
Without runtime methods
You can simply listen to the
wails:file-dropand handle it as you like.Type of change
How Has This Been Tested?
I tested it on multiple machines. On all systems I tested dropping in files and folders a combination of them slowly and fast, in-and-out. Sorry cannot describe my "method" better.
Test Configuration
Windows:
Linux:
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
New Features
Documentation
DragAndDropconfiguration fields.Enhancements
Bug Fixes