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 4 commits
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: 1 addition & 6 deletions src/getDataTransferItems.js
View file
Open in desktop
@@ -1,4 +1,4 @@
export default function getDataTransferFiles(event, isMultipleAllowed = true) {
export default function getDataTransferFiles(event) {
let dataTransferItemsList = [];
if (event.dataTransfer) {
const dt = event.dataTransfer;
Expand All @@ -12,11 +12,6 @@ export default function getDataTransferFiles(event, isMultipleAllowed = true) {
} else if (event.target && event.target.files) {
dataTransferItemsList = event.target.files;
}

if (dataTransferItemsList.length > 0) {
dataTransferItemsList = isMultipleAllowed ? dataTransferItemsList : [dataTransferItemsList[0]];
}

// Convert from DataTransferItemsList to the native Array
return Array.prototype.slice.call(dataTransferItemsList);
}
14 changes: 0 additions & 14 deletions src/getDataTransferItems.spec.js
View file
Open in desktop
Expand Up @@ -73,20 +73,6 @@ describe('getDataTransferFiles', () => {
expect(res).toHaveLength(2);
});

it('should return first file if isMultipleAllowed is false', () => {
const event = {
target: {
files
},
dataTransfer: {
files
}
};
const res = getDataTransferFiles(event, false);
expect(res).toHaveLength(1);
expect(res[0].name).toEqual(files[0].name);
});

it('should not mutate data', () => {
const event = {
dataTransfer: {
Expand Down
13 changes: 10 additions & 3 deletions src/index.js
View file
Open in desktop
Expand Up @@ -79,11 +79,13 @@ class Dropzone extends React.Component {
this.dragTargets.push(e.target);
}

const allFilesAccepted = this.allFilesAccepted(getDataTransferItems(e, this.props.multiple));
const files = getDataTransferItems(e);
const allFilesAccepted = this.allFilesAccepted(files);
const isMultipleAllowed = this.props.multiple || files.length <= 1;

this.setState({
isDragActive: allFilesAccepted,
isDragReject: !allFilesAccepted
isDragReject: !allFilesAccepted || !isMultipleAllowed
});

if (this.props.onDragEnter) {
Expand Down Expand Up @@ -127,7 +129,7 @@ class Dropzone extends React.Component {

onDrop(e) {
const { onDrop, onDropAccepted, onDropRejected, multiple, disablePreview } = this.props;
const fileList = getDataTransferItems(e, multiple);
const fileList = getDataTransferItems(e);
const acceptedFiles = [];
const rejectedFiles = [];

Expand Down Expand Up @@ -156,6 +158,11 @@ class Dropzone extends React.Component {
}
});

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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please split the comment into 2 lines

rejectedFiles.push(...acceptedFiles.splice(1));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be rewritten with array destructuring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change acceptedFiles to a let so it can be redefined and then do something like

const [accepted, ...rejected] = acceptedFiles;
rejectedFiles.push(...rejected);
acceptedFiles = [accepted];

Or just leave it as it is?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's leave as is. Thanks for clarifying.

}

if (onDrop) {
onDrop.call(this, acceptedFiles, rejectedFiles, e);
}
Expand Down
46 changes: 30 additions & 16 deletions 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 Expand Up @@ -455,44 +455,58 @@ describe('Dropzone', () => {
expect(dropzone.state().isDragReject).toEqual(false);
});

it('should expose acceptedFiles and rejectedFiles to children', () => {
it('multiple false valid files adds to rejected files on a multple drop', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we still keep shoulds in tests?

const dropzone = mount(
<Dropzone
accept="image/*"
onDrop={dropSpy}
multiple={false}
>
{({ acceptedFiles, rejectedFiles }) => JSON.stringify([acceptedFiles, rejectedFiles])}
</Dropzone>
/>
);
dropzone.simulate('drop', { dataTransfer: { files: images } });
expect(dropzone.text()).toEqual(JSON.stringify([images.slice(0, 1), []]));
dropzone.simulate('drop', { dataTransfer: { files } });
expect(dropzone.text()).toEqual(JSON.stringify([[], files.slice(0, 1)]));
const rejected = dropSpy.firstCall.args[0];
expect(rejected.length).toEqual(1);
});

it('should take all dropped files if multiple is true', () => {
it('multiple false with invalid are added files to rejected', () => {
const dropzone = mount(
<Dropzone
accept="image/*"
onDrop={dropSpy}
multiple
multiple={false}
/>
);
dropzone.simulate('drop', { dataTransfer: { files: images } });
expect(dropSpy.firstCall.args[0]).toHaveLength(2);
expect(dropSpy.firstCall.args[0][0].name).toEqual(images[0].name);
expect(dropSpy.firstCall.args[0][1].name).toEqual(images[1].name);
dropzone.simulate('drop', { dataTransfer: { files: images.concat(files) } });
const rejected = dropSpy.firstCall.args[1];
expect(rejected.length).toEqual(2);
});

it('should take first file out of dropped files if multiple is false', () => {
it('multiple false allows single files to be dropped', () => {
const dropzone = mount(
<Dropzone
accept="image/*"
onDrop={dropSpy}
multiple={false}
/>
);

dropzone.simulate('drop', { dataTransfer: { files: [images[0]] } });
const [accepted, rejected] = dropSpy.firstCall.args;
expect(accepted.length).toEqual(1);
expect(rejected.length).toEqual(0);
});

it('should take all dropped files if multiple is true', () => {
const dropzone = mount(
<Dropzone
onDrop={dropSpy}
multiple
/>
);
dropzone.simulate('drop', { dataTransfer: { files: images } });
expect(dropSpy.firstCall.args[0]).toHaveLength(1);
expect(dropSpy.firstCall.args[0]).toHaveLength(2);
expect(dropSpy.firstCall.args[0][0].name).toEqual(images[0].name);
expect(dropSpy.firstCall.args[0][1].name).toEqual(images[1].name);
});

it('should set this.isFileDialogActive to false', () => {
Expand Down