Skip to content

Fix missing file info when dragging native files#3262

Merged
darthtrevino merged 5 commits into
react-dnd:mainfrom
jgonera:jgonera-hover-files
Feb 3, 2022
Merged

Fix missing file info when dragging native files#3262
darthtrevino merged 5 commits into
react-dnd:mainfrom
jgonera:jgonera-hover-files

Conversation

@jgonera

@jgonera jgonera commented Jul 19, 2021

Copy link
Copy Markdown
Contributor

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

jgonera added 2 commits July 18, 2021 15:21
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

jgonera commented Sep 27, 2021

Copy link
Copy Markdown
Contributor Author

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

@mitochondrion

Copy link
Copy Markdown

Would love to see this get merged...

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

jgonera commented Feb 3, 2022

Copy link
Copy Markdown
Contributor Author

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

ahoisl commented Mar 8, 2022

Copy link
Copy Markdown

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
Copy Markdown

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