implement native drag and drop for macOS #3250
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 WalkthroughThis update introduces enhanced drag-and-drop functionality for macOS within the Wails framework, adding support for enabling or disabling drag-and-drop in both the general application context and specifically within the WebView component. It also introduces a custom Changes
Related 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 (
|
|
|
||
| self.webview = [WKWebView alloc]; | ||
|
|
||
| self.webview = [WailsWebView alloc]; |
There was a problem hiding this comment.
The important change is here. We use our extended implementation of WKWebkit here
|
Thanks for this @APshenkin 🙏 I'd be keen to hear @pavelbinar and @lyimmi's thoughts. Awesome work man! |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (15)
- v2/internal/frontend/desktop/darwin/Application.h (1 hunks)
- v2/internal/frontend/desktop/darwin/Application.m (2 hunks)
- v2/internal/frontend/desktop/darwin/WailsContext.h (3 hunks)
- v2/internal/frontend/desktop/darwin/WailsContext.m (24 hunks)
- v2/internal/frontend/desktop/darwin/WailsWebView.h (1 hunks)
- v2/internal/frontend/desktop/darwin/WailsWebView.m (1 hunks)
- v2/internal/frontend/desktop/darwin/frontend.go (1 hunks)
- v2/internal/frontend/desktop/darwin/window.go (2 hunks)
- v2/internal/frontend/desktop/linux/window.go (1 hunks)
- v2/internal/frontend/runtime/desktop/draganddrop.js (5 hunks)
- v2/internal/frontend/runtime/runtime_prod_desktop.js (1 hunks)
- v2/pkg/options/options.go (3 hunks)
- website/docs/reference/options.mdx (3 hunks)
- website/docs/reference/runtime/draganddrop.mdx (1 hunks)
- website/src/pages/changelog.mdx (1 hunks)
Files skipped from review due to trivial changes (1)
- website/docs/reference/runtime/draganddrop.mdx
Additional comments: 24
v2/internal/frontend/desktop/darwin/WailsWebView.h (1)
- 1-14: LGTM!
v2/internal/frontend/desktop/darwin/Application.h (1)
- 20-20: LGTM!
v2/internal/frontend/desktop/darwin/WailsContext.h (3)
- 13-13: LGTM!
- 36-36: LGTM!
- 68-68: LGTM!
v2/internal/frontend/desktop/darwin/WailsWebView.m (1)
- 1-120: LGTM!
v2/internal/frontend/runtime/desktop/draganddrop.js (4)
- 13-13: LGTM!
- 81-81: LGTM!
- 103-103: LGTM!
- 125-126: LGTM!
v2/pkg/options/options.go (3)
- 100-100: LGTM!
- 156-158: LGTM!
- 203-203: LGTM!
v2/internal/frontend/desktop/darwin/window.go (2)
- 86-88: LGTM!
- 123-123: LGTM!
v2/internal/frontend/desktop/darwin/frontend.go (1)
- 367-371: LGTM!
v2/internal/frontend/runtime/runtime_prod_desktop.js (1)
- 1-1: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [104-105]
The conditional checks for
appoptions.DragAndDropbeing notnilbefore accessing its properties are correct and prevent potential null reference exceptions.v2/internal/frontend/desktop/linux/window.go (1)
- 104-105: The conditional checks for
appoptions.DragAndDropbeing notnilbefore accessing its properties are correct and prevent potential null reference exceptions.v2/internal/frontend/desktop/darwin/Application.m (1)
- 14-20: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [17-30]
The addition of
enableDragAndDropanddisableWebViewDragAndDropparameters to theCreatefunction inWailsContextcorrectly extends its functionality to support configurable drag and drop behavior.v2/internal/frontend/desktop/darwin/WailsContext.m (2)
- 155-177: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [139-264]
The
CreateWindowmethod has been significantly modified to support drag and drop functionality. Ensure that theenableDragAndDropanddisableWebViewDragAndDropparameters are correctly used to configure theWailsWebViewinstance. Also, verify that theWailsWebViewinitialization and configuration are correctly implemented.
- 436-446: The method signature for
webView:runOpenPanelWithParameters:initiatedByFrame:completionHandler:has been updated. Confirm that this change aligns with the correct method signature as per the WebKit documentation and that the implementation correctly handles the open panel functionality.website/docs/reference/options.mdx (2)
- 67-72: The documentation correctly reflects the conversion of the
DragAndDropfield to a pointer and the renaming of theDisablefield toDisableWebViewDrop. Ensure that the example code and descriptions accurately represent the new structure and naming conventions.- 558-564: The documentation for the
DisableWebViewDropfield is clear and accurately describes its purpose. Ensure that the default value and behavior are correctly documented and that the example code reflects the new naming convention.website/src/pages/changelog.mdx (1)
- 26-26: Ensure the changelog entry for macOS drag & drop support is consistent with the format and style of previous entries.
| #import "WailsContext.h" | ||
| #import "WailsAlert.h" | ||
| #import "WailsMenu.h" | ||
| #import "WailsWebView.h" |
There was a problem hiding this comment.
Missing newline after the import statement for WailsWebView.h for consistency with other import statements.
#import "WailsWebView.h"
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| #import "WailsWebView.h" | |
| #import "WailsWebView.h" | |
|
@APshenkin This is great 🎉 |
|
Nice work @APshenkin! I tested it on linux it works as expected. On windows the Thanks for catching my mistakes in the JS files! On the issue of loosing some functionality when D&D is set to disabled/enabled, I was thinking about that before and yes the cause of it is that all drop events are hidden from the webview. The same thing happens when the When someone wants to have the original behavior as well, I think in that case they should handle the drop events manually to have a greater level of control over it. Maybe we should document this? @pavelbinar @leaanthony |
|
@lyimmi thank you for the hint!. For some reason my IDE didn't replace it for Windows. Fixed |
|
Hi, guys. Thanks, @APshenkin and @lyimmi for a great job on the PR. I work with @pavelbinar and today, we were able to test it on MacOS, and it works well. The only issue I found is that the removal of the |
|
@APshenkin, @jakubpeleska, @pavelbinar We could use the "counter method" for this. Add a new flag in const flags = {
registered: false,
defaultUseDropTarget: true,
useDropTarget: true,
prevElement: null,
dragCount: 0, <-- this
};Add a new listener: function onDragEnter(e) {
if (!window.wails.flags.enableWailsDragAndDrop) {
return;
}
e.preventDefault();
if (!flags.useDropTarget) {
return;
}
flags.dragCount++;
}
...
window.addEventListener('dragenter', onDragEnter);
...
window.removeEventListener('dragenter', onDragEnter);Zero the counter on drop: function onDrop(e) {
if (!window.wails.flags.enableWailsDragAndDrop) {
return;
}
e.preventDefault();
if (!flags.useDropTarget) {
return;
}
flags.dragCount = 0;
...Decrement the counter on leave and clear the class when the counter is zero: function onDragLeave(e) {
if (!window.wails.flags.enableWailsDragAndDrop) {
return;
}
e.preventDefault();
if (!flags.useDropTarget) {
return;
}
flags.dragCount--;
let targetElement = document.elementFromPoint(e.x, e.y);
let cssDropValue = window.getComputedStyle(targetElement).getPropertyValue(window.wails.flags.cssDropProperty);
if (cssDropValue) {
cssDropValue = cssDropValue.trim();
}
if (flags.prevElement && (cssDropValue !== window.wails.flags.cssDropValue || flags.dragCount === 0)) {
targetElement.classList.remove("wails-drop-target-active");
flags.prevElement.classList.remove("wails-drop-target-active");
// remove element as otherwise we will not update the class on the next dragover
flags.prevElement = null;
}
}With this the class is reset correctly, I don't really like it but it works. Screencast.from.2024-02-12.16.54.40.webm |
|
I tried @lyimmi solution with macOS and for some reasons it didn't work. So I came up with other way to solve it. Seems it works (at least on macOS 😅) and do this without counter which I think was the reason (as if something will register several times, then counter drop will not work). Also I covered the case when you have nested elements in your Code for this is a bit ugly, but I didn't come up with something better than checking coordinates... Can you guys check that it works on your side too? Screen.Recording.2024-02-14.at.02.19.38.mov |
|
Hi everyone, Screen.Recording.2024-02-16.at.16.05.36.mov |
|
Hi guys, I would suggest the following: The primary goal of this PR is the low-level implementation of drag and drop - providing the URLs for the files and folders and registering the events for such actions. The issues and implementation of the front-end are an addition. In our specific case, it is all we need, because we will still listen for the entire window (we do not need drop zones). Also, I can imagine other edge cases when you have multiple drop zones and you need to identify which one was used - this is not currently supported. My point is: What do you think? |
|
I'm ok with just the events without the runtime part. Edit: I mean the drop target part. Windows still needs Javascript to catch the drop. |
|
Agree with @pavelbinar. Is there a way to separate the 2 concerns? Perhaps hooks? We could also add the ui part later as a flag or document it. Thoughts? |
|
@leaanthony it can be separated relatively simply. We should register an event listener on the window for the drop for windows, that only handles the path resolution by calling Simplified to something like this, it could be moved to the main.js file under other listeners. This should be enough for windows. function onDrop(e) {
if (!window.wails.flags.enableWailsDragAndDrop) {
return;
}
e.preventDefault();
if (window.chrome?.webview?.postMessageWithAdditionalObjects != null) {
// process files
let files = [];
if (e.dataTransfer.items) {
files = [...e.dataTransfer.items].map((item, i) => {
if (item.kind === 'file') {
return item.getAsFile();
}
});
} else {
files = [...e.dataTransfer.files];
}
chrome.webview.postMessageWithAdditionalObjects(`file:drop:${e.x}:${e.y}`, files);
}
}
window.addEventListener('drop', onDrop);In addition I would start with |
|
maybe then merge this PR to the branch and then someone can pick up separation of JS part? I'm not too familiar with Windows JS magic 😅 |
leaanthony
merged commit 318628d
into
wailsapp:feature/1090_native_drag_and_drop_for_file_and_folder
|
🚀 |
* 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>
5 participants
Description
Native drag and drop with full file path for macOS. #1090
Also changed few things:
– changed
DragAndDropto be a link in app options– renamed
DragAndDrop.DisabletoDragAndDrop.DisableWebViewDropto be more clear about what we are disabling–
EventsOffimport was missing draganddrop.js– Fixed setting class property when moving hover to and away target element
1 UX nuance that I found: if you have Native drag and drop disabled and you try to drop file somewhere, where it's not allowed in UI, you can see animation where element "return back". When you enable native drag and drop you loose this effect. As well that some additional icons are not displayed (e.g. if you drop to
inputfield). I think this is because we setdropanddragoverlisteners for entire window indraganddrop.jsherewails/v2/internal/frontend/runtime/desktop/draganddrop.js
Line 183 in f9df564
This is minor thing and I think we can go with it.
Type of change
Please delete options that are not 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.Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
DragAndDropoption structure.