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

add html handling in native types #2949

Merged
merged 3 commits into from
Feb 9, 2021

Conversation

maxired
Copy link
Contributor

@maxired maxired commented Jan 5, 2021

This PR solves #2928 and allows to drag and drop HTML elements such as images from another tab.

This PR does not implements any custom logic to deal with the HTML data in itself.

@codecov
Copy link

codecov bot commented Jan 5, 2021

Codecov Report

Merging #2949 (5762f72) into main (cab86e2) will decrease coverage by 0.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2949      +/-   ##
==========================================
- Coverage   46.96%   46.94%   -0.02%     
==========================================
  Files          79       79              
  Lines        2291     2294       +3     
  Branches      501      501              
==========================================
+ Hits         1076     1077       +1     
- Misses       1215     1217       +2     
Impacted Files Coverage Δ
packages/backend-html5/src/HTML5BackendImpl.ts 20.52% <0.00%> (-0.07%) ⬇️
...d-html5/src/NativeDragSources/nativeTypesConfig.ts 16.66% <0.00%> (-3.34%) ⬇️
packages/backend-html5/src/NativeTypes.ts 100.00% <100.00%> (ø)

@maxired
Copy link
Contributor Author

maxired commented Jan 15, 2021

@darthtrevino do you have any opinion about this ?

@darthtrevino
Copy link
Member

I apologize, I've been a beet slammed at work. Just taking a quick look - Is the preventDefault() necessary()? At some point we removed some of that because we didn't want to interfere with custom handling of native events. Also, can you put together an example on the docsite using this new type? You can base it off of the existing native types example or just extend it if you prefer

@maxired
Copy link
Contributor Author

maxired commented Jan 20, 2021

Thanks for your answer. Yes I will try to setup an example.

Without the preventDefault, the browser opens the content in a new tab in that case. I am not very familiar with this code base, so maybe it can be put somewhere else? As a side not, I did not saw side effects on other handlers.

@maxired
Copy link
Contributor Author

maxired commented Jan 26, 2021

@darthtrevino I've just added an example, please let me know if this looks good to you.

@maxired
Copy link
Contributor Author

maxired commented Feb 9, 2021

@darthtrevino sorry to bother you. What is the status on this one ?

@darthtrevino darthtrevino merged commit cd4f4f1 into react-dnd:main Feb 9, 2021
@darthtrevino
Copy link
Member

darthtrevino commented Feb 9, 2021 via email

@inukshuk
Copy link
Contributor

This PR raises a problem: dragged images, URLs, and links etc. typically have multiple data transfer items with type text/html and text/uri-list and even text/plain. Right now it seems that matchNativeItemType returns just the first match: that means for example that dragged images or URLs are now only matched as type HTML, because it was added higher up in the native types config. For this reason, this PR makes it impossible (I believe?) to register drop targets for URLs -- unless you register for type HTML and then parse out the URL from the HTML data.

darthtrevino pushed a commit that referenced this pull request Feb 3, 2022
* add html handling in native types

* add release strategy

* add documentation
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.

None yet

3 participants