ICS-414-In5PIRE / in5PIRE-code

MIT License
0 stars 1 forks source link

Review Issue 4: UploadFile.jsx #127

Closed ehsuGit closed 1 month ago

ehsuGit commented 2 months ago

Please review the UploadFile.jsx, linked here

Use these guidelines:

DUE: 10/7

Ssunoo2 commented 1 month ago

L15: could add function comments Could add more descriptive error messages Could add loading message L26 specifies fourth sheet, not first

skulibas commented 1 month ago
daomcgill commented 1 month ago

Design-04: In the comment: UploadFile, not UploadsFile. There could be more comments here. Also, the comment says .csv files, but the code is reading .xlsx files. Either add support for .csv or change the comment at the top. Line 27: First sheet would be at index 0.

bobbyir commented 1 month ago

Show a loading indicator while the files is being processed File type validation should accepts csv files. (accepts only .xlsx, .xls, and .xlsm)

bfd2 commented 1 month ago
ehsuGit commented 1 month ago

Architecture OK Design 04 - Comment on line 7 should be changed to UploadFile.jsx JS and ESLint 08 - Could have more comments detailing the function of the file. Could add a swal section to confirm file upload React 01 - Could break up the code and have the file handling in a different file. Also doesn't look like it can handle files other than xlsx yet?

lukepag commented 1 month ago

Render statement seems like it could be simplified as a separate component Other than that, just comments could be updated