Skip to content
This repository has been archived by the owner on Jan 5, 2024. It is now read-only.

Feat: file validation #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MusabShakeel576
Copy link

@MusabShakeel576 MusabShakeel576 commented Nov 19, 2022

Summary:
Allows specific file formats to be previewed.

Issue:
#79

Checklist

  • Unit tests updated
  • Localization updated

@unframework
Copy link
Collaborator

Hi Musab, thanks a lot for contributing! Looks really useful, I'll just need to do a deeper check on this before merging. By the way, does this validation check for specific file extensions or just MIME type?

@MusabShakeel576
Copy link
Author

Hi Nick,

does this validation check for specific file extensions or just MIME type?

It checks for specific file types.

const supportedFileFormats = ["text/csv", "text/plain"]

if (!supportedFileFormats.includes(file.type)) return setUnsupportedFileFormat(true)

@unframework
Copy link
Collaborator

Hey, thanks again for making this change. There are a couple tweaks I would need before we can merge this:

  • the list of file types should be a configurable option (and maybe have file extensions too)
  • react-dropzone has options to help this, we should use them too: https://react-dropzone.js.org/#!/Accepting%20specific%20file%20types
  • indentation in FileSelector.tsx was affected - I have a PrettierJs config in the repo that should help auto-format it

I can make the necessary commits to augment your PR (no timing guarantee of course) or if you want to do it yourself then feel free as well. Thanks!

@devLeopar
Copy link

devLeopar commented May 18, 2023

I think only file format should be accepted is csv files. Because(#79 (comment)) That can be achieved that PR as I see or using useDropzone ability like below:

const acceptedFiles = {
"text/csv": [".csv"]
}

  const { getRootProps, getInputProps } = useDropzone({
...
    accept: acceptedFiles,
...
  })

@devLeopar
Copy link

@unframework could you please deploy it?

@dwene
Copy link

dwene commented Jun 26, 2023

Any chance we can get this deployed?

@dwene
Copy link

dwene commented Jun 26, 2023

@MusabShakeel576 it looks like there are conflicts in this PR. Could you resolve those please so we could maybe get this merged?

@unframework
Copy link
Collaborator

Hey everyone, looks like this feature is getting continued attention, thanks for all the suggestions. There are still some small issues outstanding with the PR (see my earlier comment) and as @dwene pointed out it has a conflict now. I will try to find some time this week to tweak the PR.

@dwene
Copy link

dwene commented Jun 27, 2023

I fixed the PR conflicts in a fork if you want to use any of that code.

https://github.com/adsupnow/react-csv-importer

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants