-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
Make sure we use the standard API for file drops if it's available #38
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR but you bundled too many opinionated changes into one PR.
I would have merged the reversal, but I don't want to change the return type or code style.
if (files.length === 0) return | ||
|
||
for (var i = 0; i < files.length; ++i) { | ||
files[i].fullPath = '/' + files[i].name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't necessary to change the code style here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was, a FileList
isn't an array, it doesn't have a forEach
method.
@@ -110,7 +110,19 @@ function dragDrop (elem, listeners) { | |||
} | |||
|
|||
// file drop support | |||
if (e.dataTransfer.items) { | |||
if (e.dataTransfer.files) { | |||
var files = e.dataTransfer.files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the return type here is a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's also the only reason to make this change, if the return type doesn't change, this library can't be used to transfer a file drag and drop to a file input
I merged just the |
The change makes the |
@benjamin-demarteau I don't understand your use case. Can you explain it more completely? |
I am using uppy to handle file upload, when dropping files into a drop zone, it registers the files to upload, but it doesn't add the files dropped to the html input, which means there is no user feedback (that and other problems when selecting the same file multiple times). It is possible to add files to an Since uppy uses this library to handle the drag and drop events, before contributing that change, I need this library to provide a Reference: https://html.spec.whatwg.org/multipage/input.html#common-input-element-apis
|
Understood. I decided to change the API to return a |
Thanks a bunch for this change! |
There was a regression, so I need to change the API once again. See: #39 |
As there are no tests, I do not know if this breaks anything. I assume the code was written so that both code path provide the same object.
The new code doesn't transform the
files
list to an array anymore as having aFileList
instance is much more useful.