Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Drag & Drop JS runtime #3516

Conversation

jakubpeleska
Copy link
Contributor

@jakubpeleska jakubpeleska commented May 28, 2024

Description

This PR completes the drag-and-drop support, which was previously implemented in:

Changes

  • drag and drop refactor - draganddrop.js
  • fix for CSS class wails-drop-target-active
  • add debounce on dragleave to avoid flickering of the wails-drop-target-active class

This implementation provides developers with enough functionality to start using this feature in production. Therefore, we believe this commit can conclude all previous efforts and mark the drag-and-drop feature as ready for production.

How Has This Been Tested?

Fix was tested on a dummy project available on https://github.com/beam-transfer/wails-drag-and-drop

  • Windows
  • macOS
  • Linux

Video

  • red area - drop zone "--wails-drop-target": "drop"
  • yellow area - drop zone disabled "--wails-drop-target": "none"
  • teal area - element without any special treatment
drag-and-drop-demonstration.mp4

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have updated website/src/pages/changelog.mdx with details of this PR
  • My code follows the general coding style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

lyimmi and others added 5 commits February 6, 2024 17:18
* 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

* 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)
 into feature/1090_native_drag_and_drop_for_file_and_folder
@pavelbinar
Copy link
Contributor

Hi @lyimmi , @APshenkin please take a look.

Copy link
Member

@leaanthony leaanthony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to loop back to this 🙏
What do you think the challenges for this approach might be for a multi-window application?

@APshenkin
Copy link
Contributor

@jakubpeleska @pavelbinar
This looks great. Thank you for finishing this!

Hope to see this released in v2 soon!

@jakubpeleska
Copy link
Contributor Author

@leaanthony Thanks for taking a look.

Thanks for taking the time to loop back to this 🙏 What do you think the challenges for this approach might be for a multi-window application?

As for the multi-window challenges, I can't really speak for the native implementations. However, the runtime JS should work out of the box, assuming that there are separate JS runtimes for each window. That being said, I did not test it, so I can't be sure. @APshenkin and @lyimmi might have more insight into the challenges of native implementations.

@pavelbinar pavelbinar force-pushed the feature/1090_native_drag_and_drop_for_file_and_folder branch from 0f84d4a to 600997b Compare June 3, 2024 11:14

// collecting all file paths
NSString *files_str = @"";
NSMutableArray *files_strs = [[NSMutableArray alloc] init];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be released?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leaanthony @jakubpeleska

Here we iterate over NSURL array to convert them to just strings, then join then all in one string.

So files_strs is needed. Only if someone have better know how to do this mapping, as I'm not Objective-C expert 😅.

However files_str is redundant, as it's not used. You can remove it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the [alloc] would require a [release]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes you are right, sorry misunderstood you 😅

@leaanthony leaanthony merged commit 022a5ff into wailsapp:master Jun 10, 2024
8 checks passed
@leaanthony
Copy link
Member

leaanthony commented Jun 10, 2024

Thanks all involved in this monumental effort! Anyone fancy writing a small guide on how to use it? 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants