hdoro / sanity-plugin-external-files

Work w/ media hosted in external vendors from inside a Sanity.io studio

44 stars 14 forks source link

(#6) Allow users to upload additional file types #7

Closed nkgentile closed 2 years ago

nkgentile commented 2 years ago

Description

I've exposed defaultAccept and toolTitle as plugin configuration options. For example: config/s3-dam.json
```
{
"toolTitle": "S3 Media",
"defaultAccept": ["application/pdf", "image/*", "audio/*", "video/*"]
}
```
In order to accept non-video and non-audio files, I've added a new state to the uploadMachine as a fallback
I haven't added thumbnails or previews to any new file types, but I've adjusted the MediaPreview component's internal logic to accommodate more file types.

Type of change

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

hdoro commented 2 years ago

Thank you for the work, Noah! I'm yet to run it locally for manual testing, but it looks polished and ready to go - sorry for the messy code you had to go through 🙈

I won't have time for a thorough review until next week - in the meantime could you:

consider changing the default accept to be just a plain * (i.e. everything)?
explore what adding PDF thumbnails would entail. I think we'd need a heavy-duty HTML/iframe to image conversion process (using something like html-to-image), which wouldn't be worth it. But if there's an easy & reliable way to get PDF previews I'd go for it - it'd drastically improve UX (PDFs are the most requested file extension for these plugins).

Anywho, don't want to give you too much extra work, you're already rocking - thanks again!

nkgentile commented 2 years ago

No worries! I tried to be as surgical as I could be to prevent any confusion when reviewing. And yes, please test yourself to make sure I didn't accidentally create any bugs, especially in the other DAM plugins.

I can definitely look into these two changes and update this PR:

Rather than defaulting to a wildcard, I might look into if there is a sensible default value that other similar packages use (either inside or outside the Sanity ecosystem). Will it be considered a breaking change if the internal DEFAULT_ACCEPT value changes? Would changing the default toolTitle fall under this as well?
For sure, I was thinking this as well! Although, I didn't want to get too into the weeds on this part in case it involved a lot of changes elsewhere. I'll look into it 😎