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

Reject all if multiple prop is false and multiple files are dropped #458

Closed
1 of 2 tasks
dyelax opened this issue Jul 18, 2017 · 9 comments · Fixed by #641
Closed
1 of 2 tasks

Reject all if multiple prop is false and multiple files are dropped #458

dyelax opened this issue Jul 18, 2017 · 9 comments · Fixed by #641

Comments

@dyelax
Copy link

dyelax commented Jul 18, 2017

Do you want to request a feature or report a bug?

  • I found a bug
  • I want to propose a feature

What is the current behavior?

When the multiple prop is false, and the user drops multiple files, the first file is accepted and the rest are rejected.

What is the expected behavior?

I would expect all images to be rejected in this case. It's bad UI to show a general error when one file (pseudo-randomly) will succeed.

Happy to make a PR for this if you are open to changing the interface.

@okonet
Copy link
Collaborator

okonet commented Jul 19, 2017

Duplicate of #409. This was done in #410

@okonet okonet marked this as a duplicate of #409 Jul 19, 2017
@okonet okonet closed this as completed Jul 19, 2017
@dyelax
Copy link
Author

dyelax commented Jul 25, 2017

This isn't exactly a duplicate of #409. That issue is talking about the css class showing. This works fine for me. I'm suggesting that when multiple files are dropped and multiple=false, none of the files should go to accepted (all of them should be rejected), while currently one of the files that are dropped will still be accepted

@okonet
Copy link
Collaborator

okonet commented Aug 1, 2017

Hmm, this actually makes sense to me. As I understand this will require to modify this line https://github.com/okonet/react-dropzone/pull/410/files#diff-1fdf421c05c1140f6d71444ea2b27638R85 and remove the second condition + update all related tests.

Am I right?

This will be a breaking change, though. But I think this behaviour is more logical. Do you mind creating a PR for that?

@DeFuex
Copy link

DeFuex commented Oct 3, 2017

Hi @dyelax @okonet , seems like the problem is on that specific line here

if (!multiple) {
      // if not in multi mode add any extra accepted files to rejected.
      // This will allow end users to easily ignore a multi file drop in "single" mode.
      rejectedFiles.push(...acceptedFiles.splice(1))
    }

it seems like this behaviour was added on purpose, but with the changed css styling in ticket #409 this seems more like a bug due to the described behaviour of you @dyelax ....if wished i create a PR and set rejectedFiles.push(...acceptedFiles.splice(0)), then every file in multiple=false mode should be declined. i tested it with react dev tools in the browser.

Edit: ok, seems like that splice(0) is not the solution...if set 0 it doesnt work on single file uploads, but anyways. ill try to fix it.

@duro
Copy link

duro commented Dec 16, 2017

On top of this, there also seems to be a miss match in behavior with multiple=true

Right now, if I have multiple=true, and some of the files I drop are OK by my accept settings, but some are not, the acceptable files are sent to onDrop, and the rejected files are not.

As far the passed props isDragAccept and isDragReject goes, there is no way to accurately display this to the end user.

Right now the way it works is that if any of the files are rejected, isDragReject === true and isDragAccept === false. But that's not exactly what is happening.

What I would prefer to see if that there be some way, before the drop actually happens, to present this visually to the end user (since I think it's fine that they may have mistakenly included an unacceptable file, but some that were acceptable).

In this case I would expect either isDragAccept and isDragReject to both be true, that way I can message to the user that their drop will go through, but some of the files will be rejected.

But right now, I end up messaging to them that their selection is rejected, even though if they actually did drop, some of the files would pass through.

Am I making sense? It's late here. 😴

@DeFuex
Copy link

DeFuex commented Dec 16, 2017

Hi @duro, the correct behaviour of mutliple=true and multiple=false should actually be fixed now in #511 ....the reason why it isn't merged yet is because currently there is mutable behaviour with the arrays handling accepted and rejected files, which should be actually immutable (...cause reasons)

Can you further clarify what you mean with your points? Do you mean something like a third flag option that allows a user to drop files which could be accepted and rejected as drops depending on a "type" they have? This sounds like an extra feature to me... @okonet

@okonet
Copy link
Collaborator

okonet commented Dec 19, 2017

@duro I'm not sure I'm following. The signature of the onDrop function looks like

function onDrop(acceptedFiles, rejectedFiles) {
  // do stuff with files...
}

so it's up to you how you react if rejectedFiles are present.

@duro
Copy link

duro commented Dec 20, 2017

@okonet The main problem I'm running into is that there is no way to know if there are both accepted and rejected files until the user has finished the drop. I want to be able to warn the user that they have some rejected (and some accepted) files on dragEnter before they release the mouse and onDrop is triggered.

@okonet
Copy link
Collaborator

okonet commented Dec 20, 2017

This is not possible because of browser limitations (i.e. "security" reasons).

okonet pushed a commit that referenced this issue Aug 17, 2018
Closes #458

BREAKING CHANGE

This can break applications that rely on the current behavior.
okonet pushed a commit that referenced this issue Aug 17, 2018
If multiple files were dropped and `multiple == false`,
react-dropzone will reject all files. Previously it would
accept one file and reject the rest.

Closes #458

BREAKING CHANGE

This can break applications that rely on the current behavior.
okonet pushed a commit that referenced this issue Aug 17, 2018
If multiple files were dropped and `multiple == false`,
react-dropzone will reject all files. Previously it would
accept one file and reject the rest.

BREAKING CHANGE: This can break applications that rely on the current behavior.

Closes #458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants