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 missing file info when dragging native files #3262

Merged
merged 5 commits into from Feb 3, 2022

Conversation

jgonera
Copy link
Contributor

@jgonera jgonera commented Jul 19, 2021

Note: The first commit is here just so that it's easier to test this PR in the native file example (item.items[0] should not be undefined when dragging a file with this PR applied). I will remove it before merging if this solution is accepted.

After reading through #584 and a bit of investigation of react-dropzone's source code it seemed that getting the info about dragged files while they're being dragged should be possible. Both react-dnd and react-dropzone as of now use the file info from the initial dragenter event.

react-dropzone:
https://github.com/react-dropzone/react-dropzone/blob/1924fa6cd8bd0fcdbd2165ed726416d3973b0555/src/index.js#L539

react-dnd:

this.beginDragNativeItem(nativeType, dataTransfer as DataTransfer)

The difference lies in react-dropzone making a copy of that information:

https://github.com/react-dropzone/file-selector/blob/d8287918d49a9cb9c25ff08ce9d3a63e089234c2/src/file-selector.ts#L105

While react-dnd just passes the reference to DataTransferItemList:

items: (dataTransfer: DataTransfer): DataTransferItemList =>
dataTransfer.items,

Chrome (and apparently Safari too) will empty the DataTransferItemList after a given drag event is done so we need to point NativeDragSource to a new list on each dragenter/dragover event (see https://bugs.chromium.org/p/chromium/issues/detail?id=137231) which is what this PR is adding.

An alternative approach would be making a copy of this data like react-dropzone does, but this would require bigger, possibly backwards-incompatible changes. Currently, we do update DataTransferItemList once already when a native item is dropped:

this.currentNativeSource?.loadDataTransfer(e.dataTransfer)

Fixes #584

Temporarily, remove before merging.
Chrome (and apparently Safari too) will empty the `DataTransferItemList`
after a given drag event is done so we need to point `NativeDragSource`
to a new list on each dragenter/dragover event.

See https://bugs.chromium.org/p/chromium/issues/detail?id=137231

Fixes react-dnd#584
@jgonera
Copy link
Contributor Author

jgonera commented Sep 27, 2021

@darthtrevino Would you have a moment to take a look?

@mitochondrion
Copy link

Would love to see this get merged...

@darthtrevino darthtrevino merged commit df5386c into react-dnd:main Feb 3, 2022
@jgonera
Copy link
Contributor Author

jgonera commented Feb 3, 2022

Thank you!

@jgonera jgonera deleted the jgonera-hover-files branch February 3, 2022 23:56
darthtrevino added a commit that referenced this pull request Feb 3, 2022
* WIP Add a debugging case for hovering files

Temporarily, remove before merging.

* Load dataTransfer in every dragenter and dragover

Chrome (and apparently Safari too) will empty the `DataTransferItemList`
after a given drag event is done so we need to point `NativeDragSource`
to a new list on each dragenter/dragover event.

See https://bugs.chromium.org/p/chromium/issues/detail?id=137231

Fixes #584

* chore: cut semver

* fix: run prettier

Co-authored-by: Chris Trevino <chtrevin@microsoft.com>
@ahoisl
Copy link

ahoisl commented Mar 8, 2022

Issue #3318 is similar to this problem and got partly fixed by this change - thank you!
However, the file info is still missing in Chrome when you first drag over the ref item and the leave it again.

Please see this sandbox and watch the output for "getItem list isEmpty":
https://codesandbox.io/s/dark-haze-fxw4u3?file=/src/hooks/useFileDrop.ts

Would be great if you could have a look as you already fixed it almost completely 😄

@jlaramie
Copy link

Issue #3318 is similar to this problem and got partly fixed by this change - thank you! However, the file info is still missing in Chrome when you first drag over the ref item and the leave it again.

Please see this sandbox and watch the output for "getItem list isEmpty": https://codesandbox.io/s/dark-haze-fxw4u3?file=/src/hooks/useFileDrop.ts

Would be great if you could have a look as you already fixed it almost completely 😄

I am also seeing this issue when dragging items in and out of the ref

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.

How can I get info about dragging files in canDrop method?
5 participants