Add max upload size to image uploader #293
Conversation
There was a problem hiding this comment.
Awesome! I notice some good attention to detail here, really appreciate that @PraggyaJ !
Just a few small comments, and then as you can see you've accidentally made the whole admin components directory executable it seems haha, so just undo that ;).
static/admin.css
Outdated
| @@ -3,6 +3,10 @@ | |||
| color: black; | |||
| } | |||
|
|
|||
| body{ | |||
| @@ -138,6 +148,14 @@ export default class ImageUploader extends BaseComponent { | |||
| this.addImagePreviewUrl(file); | |||
| }); | |||
|
|
|||
| if (tooLargeFileNames.length !== 0) { | |||
| let names = '\n'; | |||
| tooLargeFileNames.forEach((file) => { | |||
There was a problem hiding this comment.
This works, good job! But there's a smoother way we can do it, like this:
const alertMessage = tooLargeFileNames.reduce((currentMessage, filename) => {
return `${currentMessage}\n${file.name} (${sizeCalculation) KB)`;
}, 'The following files exceeded the maximum upload size:\n');See these docs for an explanation of the reduce function
| tooLargeFileNames.forEach((file) => { | ||
| names = names.concat(` ${file.name} (${(file.size / 1024).toFixed(2)} KB)\n`); | ||
| }); | ||
| alert(`The following exceeded the maximum upload size: ${names}`); |
There was a problem hiding this comment.
Maybe just make this the following files... (adding in "files").
Also, it would generally be a lot more natural to add the newline in here instead of as the first part of the string above, so just making it size:\n${names}), but with my above approach you can do it all in one and just do alert(alertMessage) here.
There was a problem hiding this comment.
Instead of alert, let's use a Material UI dialog box. This falls in line with the rest of the Admin Interface and is just a little less jarring than an alert message.
There was a problem hiding this comment.
Yeah you're right, let's keep things pretty! I'll try not to fall into my old habits ;)
| tooLargeFileNames.forEach((file) => { | ||
| names = names.concat(` ${file.name} (${(file.size / 1024).toFixed(2)} KB)\n`); | ||
| }); | ||
| alert(`The following exceeded the maximum upload size: ${names}`); |
There was a problem hiding this comment.
Instead of alert, let's use a Material UI dialog box. This falls in line with the rest of the Admin Interface and is just a little less jarring than an alert message.
| }); | ||
| alert(`The following exceeded the maximum upload size: ${names}`); | ||
| } | ||
|
|
||
| // Reset to "No Files Chosen" in the input element instead of saying "10 files" or so | ||
| e.target.parentNode.parentNode.parentNode.parentNode.parentNode.reset(); |
There was a problem hiding this comment.
There must be a more elegant way to do this. What are all the .parentNodes referring to here??
There was a problem hiding this comment.
If I remember correctly, it was just really hard getting a ref to it or something like that, it is without a doubt ugly though
The image uploader only accepts images with a size less than 300 KB and alerts the user which files were too big.