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

Error as array #18

Open
wants to merge 4 commits into
base: release/v0.3.0
Choose a base branch
from
Open

Error as array #18

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
794eef6
better fileSizeError custom message, updated readme
dmmcinty Apr 9, 2018
e4d0f64
display both errors if neccesary
dmmcinty Apr 10, 2018
2661280
un-prettier things
dmmcinty Apr 10, 2018
bb7258a
Looping thru errors
dmmcinty Apr 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ value: PropTypes.shape({
filename: PropTypes.string, // Original filename from uploaded file
filetype: PropTypes.string, // Original MIME type from uploaded file
src: PropTypes.string, // Original DataURL from the FileReader.result
error: PropTypes.string, // Error returned from fileSize/fileType validators
error: PropTypes.array, // Error returned from fileSize/fileType validators
}).isRequired,
```

Expand All @@ -111,7 +111,7 @@ customMessage: PropTypes.shape({
instructions: PropTypes.string, // default: 'Drag-n-drop a file or click to add an image'
acceptedFileTypes: PropTypes.string, // default: 'Accepted file types: '
maxFileSize: PropTypes.string, // default: 'Max file size: '
fileTypeErrorMessage: PropTypes.string, // default: `File size must be less than $BYTES`
fileTypeErrorMessage: PropTypes.string, // default: 'File size must be less than $BYTES'. maxFileSize value can be referenced with '$BYTES'
fileSizeErrorMessage: PropTypes.string, // default: 'Invalid file type'
})
```
Expand Down
16 changes: 10 additions & 6 deletions src/DropNCrop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class DropNCrop extends Component {
filename: PropTypes.string,
filetype: PropTypes.string,
src: PropTypes.string,
error: PropTypes.string,
error: PropTypes.array,
}),
};

Expand Down Expand Up @@ -74,7 +74,7 @@ class DropNCrop extends Component {
customMessage,
} = this.props;
const fileSizeErrorMessage = customMessage.fileSizeErrorMessage
? customMessage.fileSizeErrorMessage + bytesToSize(maxFileSize)
? customMessage.fileSizeErrorMessage.replace('$BYTES',bytesToSize(maxFileSize))
: null;
const fileSizeValidation = fileSizeLessThan(maxFileSize, fileSizeErrorMessage)(files);
const fileTypeValidation = fileType(allowedFileTypes, customMessage.fileTypeErrorMessage)(files);
Expand All @@ -92,10 +92,11 @@ class DropNCrop extends Component {
};
reader.readAsDataURL(files[0]);
} else {
const errors = [];
if(!fileTypeValidation.isValid) errors.push(fileTypeValidation.message);
if(!fileSizeValidation.isValid) errors.push(fileSizeValidation.message);
onChange({
error: !fileTypeValidation.isValid
? fileTypeValidation.message
: !fileSizeValidation.isValid ? fileSizeValidation.message : null, // TODO: Update error state to be an array to handle both messages if necessary
error: errors
});
}
};
Expand Down Expand Up @@ -166,7 +167,10 @@ class DropNCrop extends Component {
key="dropzone-validation"
className="dropzone-validation"
>
{value && value.error}
<p>{value && value.error[0]}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

When displaying multiple error messages like this it's more flexible to just loop through all the errors and output them.

{value && value.error && value.error.map(error => <p>{error}</p>}

Also we should have scott review this to make sure he's cool with multiple errors being displayed inside p tags here.

Copy link

Choose a reason for hiding this comment

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

I think I'm okay with this? Would love to see what it looks like all validated out. @dmmcinty send me a screenshot ??

Copy link
Author

Choose a reason for hiding this comment

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

Updated the line

{value.error[1]
? <p>{value && value.error[1]}</p>
:null}
</div>
: null}
</Dropzone>}
Expand Down