-
Notifications
You must be signed in to change notification settings - Fork 67
feat: configure mime/type option #398
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.
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
@isaac-martin is attempting to deploy a commit to the Sanity Sandbox Team on Vercel. A member of the Team first needs to authorize it. |
name: 'mainVideo', | ||
type: 'mux.video', | ||
options: { | ||
encoding_tier: 'baseline', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encoding_tier
has been renamed to video_quality
(and smart
-> plus
and baseline
-> basic
)
I don't think the Sanity plugin supports video_quality
yet, so this isn't an actionable comment. Just making a note that, at some point, we'll need to update the Sanity plugin to reflect this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got some errors when trying to build the plugin locally but by making these changes it worked as in the recording
Co-authored-by: Renzo Delfino <[email protected]> Signed-off-by: Isaac Martin <[email protected]>
Co-authored-by: Renzo Delfino <[email protected]> Signed-off-by: Isaac Martin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After running the linter, I noticed that the config
prop is not being used in the UploadPlaceholder
component. It seems unnecessary, so we should remove it.
Let me know if there's any reason to keep it that I might have missed
@@ -364,6 +411,8 @@ export default function Uploader(props: Props) { | |||
/> | |||
) : ( | |||
<UploadPlaceholder | |||
accept={acceptMimeString} | |||
config={props.config} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config={props.config} |
@@ -12,9 +12,11 @@ interface UploadPlaceholderProps { | |||
hovering: boolean | |||
needsSetup: boolean | |||
onSelect: FileInputButtonProps['onSelect'] | |||
accept: string | |||
config: PluginConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config: PluginConfig |
Improve Audio File Handling and MIME Type Validation
Currently, the plugin's handling of audio files isn't optimal despite Mux supporting audio uploads. This was also reported in issue #382.
You could always upload audio types in this plugin (mainly due to a bug that was allowing it via drag and drop) but this makes it more configurable. It appears that this maybe used to be supported due to some history i found, but appears to be removed at some point.
There is still work to be done to make audio a first class citizen, but this is a good start.
Heres a screen recording of the states working.
Attached is the code for the schema including the various options, and a screenshot of how it would look
Key Changes
acceptedMimeTypes
to be configurable at a field and a config levelWas unsure if we should set the default to audio / video or just video. Technically right now you can do both, due to the broken drag and drop input, so i set both as the default for backwards compatibility.
Note: some browsers when using the
audio/*
will allow anmp4
to be uploaded as mimeType is just a hint, so we block in the upload and show a toast.