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.

Sign up for GitHub

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

Jump to bottom

Show rejected styles when dragging multiple files and multiple==false #410

Merged
merged 6 commits into from Apr 26, 2017
Merged

Show rejected styles when dragging multiple files and multiple==false #410

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8e8be7c
Add fix for 409. Show the error class when multiple files are being …
plee-nm Apr 19, 2017
602eb30
Remove the negation of the variable name and remove the ternary state…
plee-nm Apr 20, 2017
f0d3593
remove multiple param to getDataTransferItems
plee-nm Apr 20, 2017
75e03cc
Add to rejected files when multiple files are dropped in multiple=false
plee-nm Apr 20, 2017
91b0373
break comment into multi line, word tests better with should
plee-nm Apr 24, 2017
b90171b
Merge branch 'master' into master
okonet Apr 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/index.js
View file
Open in desktop
Expand Up @@ -79,11 +79,14 @@ class Dropzone extends React.Component {
this.dragTargets.push(e.target);
}

const allFilesAccepted = this.allFilesAccepted(getDataTransferItems(e, this.props.multiple));
// validate all files regardless if they are in multi file mode.
const files = getDataTransferItems(e, true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Shouldn't it be aligned with https://github.com/okonet/react-dropzone/pull/410/files#diff-1fdf421c05c1140f6d71444ea2b27638R133?
  2. true is default argument, so you can skip it

const allFilesAccepted = this.allFilesAccepted(files);
const isMultipleNotAllowed = this.props.multiple ? false : files.length > 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Let's remove negation from the variable name please
  2. This can be rewritten as !this.props.multiple || files.length > 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the negation with the new changes.


this.setState({
isDragActive: allFilesAccepted,
isDragReject: !allFilesAccepted
isDragReject: !allFilesAccepted || isMultipleNotAllowed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can allFilesAccepted be set accordingly before so this code remains intact?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this means exactly. I just pushed a change so let me know if I need to update it.

});

if (this.props.onDragEnter) {
Expand Down
2 changes: 1 addition & 1 deletion src/index.spec.js
View file
Open in desktop
Expand Up @@ -369,7 +369,7 @@ describe('Dropzone', () => {
);
dropzone.simulate('dragEnter', { dataTransfer: { files: images } });
expect(dropzone.state().isDragActive).toEqual(true);
expect(dropzone.state().isDragReject).toEqual(false);
expect(dropzone.state().isDragReject).toEqual(true);
});

it('should expose state to children', () => {
Expand Down