-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Conversation
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
@darthtrevino Would you have a moment to take a look? |
Would love to see this get merged... |
Thank you! |
* 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>
Issue #3318 is similar to this problem and got partly fixed by this change - thank you! Please see this sandbox and watch the output for "getItem list isEmpty": 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 |
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 beundefined
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:
react-dnd/packages/backend-html5/src/HTML5BackendImpl.ts
Line 541 in e8bd643
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
:react-dnd/packages/backend-html5/src/NativeDragSources/nativeTypesConfig.ts
Lines 23 to 24 in e8bd643
Chrome (and apparently Safari too) will empty the
DataTransferItemList
after a given drag event is done so we need to pointNativeDragSource
to a new list on eachdragenter
/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:react-dnd/packages/backend-html5/src/HTML5BackendImpl.ts
Line 655 in e8bd643
Fixes #584