-
-
Notifications
You must be signed in to change notification settings - Fork 33
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 UT to reproduces #106 #108
Conversation
Hmm ... I wonder why checks haven't run 😕 |
They need to be approved, which only maintainers can do. This is for security reasons so that people cannot change workflows in a way that would be considered insecure. |
I think the new UI doesn't have that. ![]() |
Oh interesting, I am still on the old one so I do see it, of course I can't start it as I don't have the permission to do so. |
@buenjybar with #107, you'll now get a promise rejection with:
See diff. So you'll need to handle that. But what that test means, essentially, is that the object you get is not actually a file, see return value:
And there's no way to reach that point with any other event than a // According to https://html.spec.whatwg.org/multipage/dnd.html#dndevents,
// only 'dragstart' and 'drop' has access to the data (source node)
if (type !== 'drop') {
return items;
}
const files = await Promise.all(items.map(toFilePromises)); If this happens during cypress tests, it might be worth seeing if there's something about that which may be causing these issues. Are you using something like https://www.cypress.io/blog/uploading-files-with-selectfile? |
It seems that cypress is using the dispatchEvent with the following event type: so it is never catch by this code :
I checked in this project the items variable just above look like this It is actually a valid File. |
@buenjybar are you triggering a drop event? |
I've also just tried it in react-dropzone-nextjs and I can also see the error. I'll try the patch in #107 and see if that works. |
@buenjybar I also suggest opening a ticket with cypress and inform them of the current behaviour. We expect that the promise resolves to a file, but it looks like it doesn't. We could add a check in our code for It might also be a permission thing or https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts ... |
cypress-io/cypress#669 may also be helpful. |
Obsolete, It has been fixed in 2.1.2 |
What kind of change does this PR introduce?
Did you add tests for your changes?
If relevant, did you update the documentation?
Summary
Explain the motivation for making this change. What existing problem does the pull request solve?
Try to link to an open issue for more information.
Try to reproduce bug #106 (#106)
Does this PR introduce a breaking change?
If this PR introduces a breaking change, please describe the impact and a migration path for existing applications.
Other information