-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
remotion.dev/convert: Add compression option to ConvertUi component #5853
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?
Changes from all commits
f7ae991
3a32329
4ccf650
451946e
a2cceca
7a0a020
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import { | ||
| QUALITY_HIGH, | ||
| QUALITY_LOW, | ||
| QUALITY_MEDIUM, | ||
| QUALITY_VERY_HIGH, | ||
| QUALITY_VERY_LOW, | ||
| } from 'mediabunny'; | ||
| import React from 'react'; | ||
| import {Label} from './ui/label'; | ||
| import { | ||
| Select, | ||
| SelectContent, | ||
| SelectItem, | ||
| SelectTrigger, | ||
| SelectValue, | ||
| } from './ui/select'; | ||
|
|
||
| type QualityLevel = | ||
| | typeof QUALITY_VERY_LOW | ||
| | typeof QUALITY_LOW | ||
| | typeof QUALITY_MEDIUM | ||
| | typeof QUALITY_HIGH | ||
| | typeof QUALITY_VERY_HIGH | ||
| | null; | ||
|
|
||
| export const CompressUi: React.FC<{ | ||
| readonly videoQuality: QualityLevel; | ||
| readonly setVideoQuality: (quality: QualityLevel) => void; | ||
| readonly audioQuality: QualityLevel; | ||
| readonly setAudioQuality: (quality: QualityLevel) => void; | ||
| readonly hasVideo: boolean; | ||
| readonly hasAudio: boolean; | ||
| }> = ({ | ||
| videoQuality, | ||
| setVideoQuality, | ||
| audioQuality, | ||
| setAudioQuality, | ||
| hasVideo, | ||
| hasAudio, | ||
| }) => { | ||
| const getQualityValue = (quality: QualityLevel): string => { | ||
| if (quality === null) return 'none'; | ||
| if (quality === QUALITY_VERY_LOW) return 'very-low'; | ||
| if (quality === QUALITY_LOW) return 'low'; | ||
| if (quality === QUALITY_MEDIUM) return 'medium'; | ||
| if (quality === QUALITY_HIGH) return 'high'; | ||
| if (quality === QUALITY_VERY_HIGH) return 'very-high'; | ||
| return 'none'; | ||
| }; | ||
|
|
||
| const setQualityFromValue = (value: string): QualityLevel => { | ||
| if (value === 'very-low') return QUALITY_VERY_LOW; | ||
| if (value === 'low') return QUALITY_LOW; | ||
| if (value === 'medium') return QUALITY_MEDIUM; | ||
| if (value === 'high') return QUALITY_HIGH; | ||
| if (value === 'very-high') return QUALITY_VERY_HIGH; | ||
| return null; | ||
| }; | ||
|
|
||
| return ( | ||
| <div> | ||
| <div className="h-4" /> | ||
| {hasVideo ? ( | ||
| <> | ||
| <Label htmlFor="video-quality">Video quality</Label> | ||
| <Select | ||
| value={getQualityValue(videoQuality)} | ||
| onValueChange={(val) => setVideoQuality(setQualityFromValue(val))} | ||
| > | ||
| <SelectTrigger> | ||
| <SelectValue placeholder="Select video quality" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| <SelectItem value="none">No compression</SelectItem> | ||
| <SelectItem value="very-low">Very Low</SelectItem> | ||
| <SelectItem value="low">Low</SelectItem> | ||
| <SelectItem value="medium">Medium</SelectItem> | ||
| <SelectItem value="high">High</SelectItem> | ||
| <SelectItem value="very-high">Very High</SelectItem> | ||
| </SelectContent> | ||
| </Select> | ||
| <div className="text-sm mt-2 text-muted-foreground"> | ||
| Lower quality reduces file size but may affect visual quality. | ||
| </div> | ||
| </> | ||
| ) : null} | ||
| {hasVideo && hasAudio ? <div className="h-4" /> : null} | ||
| {hasAudio ? ( | ||
| <> | ||
| <Label htmlFor="audio-quality">Audio quality</Label> | ||
| <Select | ||
| value={getQualityValue(audioQuality)} | ||
| onValueChange={(val) => setAudioQuality(setQualityFromValue(val))} | ||
| > | ||
| <SelectTrigger> | ||
| <SelectValue placeholder="Select audio quality" /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| <SelectItem value="none">No compression</SelectItem> | ||
| <SelectItem value="very-low">Very Low</SelectItem> | ||
| <SelectItem value="low">Low</SelectItem> | ||
| <SelectItem value="medium">Medium</SelectItem> | ||
| <SelectItem value="high">High</SelectItem> | ||
| <SelectItem value="very-high">Very High</SelectItem> | ||
| </SelectContent> | ||
| </Select> | ||
| <div className="text-sm mt-2 text-muted-foreground"> | ||
| Lower quality reduces file size but may affect audio quality. | ||
| </div> | ||
| </> | ||
| ) : null} | ||
| </div> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -89,12 +89,12 @@ | |||||||||||||||||||||||||||
| "outputLogs": "new-only" | ||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||
| "@remotion/convert#build-page": { | ||||||||||||||||||||||||||||
| "dependsOn": ["^make"], | ||||||||||||||||||||||||||||
| "dependsOn": ["make"], | ||||||||||||||||||||||||||||
| "outputs": ["build", ".vercel"], | ||||||||||||||||||||||||||||
| "outputLogs": "new-only" | ||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||
| "@remotion/convert#build-spa": { | ||||||||||||||||||||||||||||
| "dependsOn": ["^make"], | ||||||||||||||||||||||||||||
| "dependsOn": ["make"], | ||||||||||||||||||||||||||||
|
Comment on lines
+92
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The turbo.json build dependencies were changed from View DetailsAnalysisBroken turbo dependency configuration causes convert package build failuresWhat fails: @remotion/convert#build-page and @remotion/convert#build-spa tasks fail with module resolution errors when workspace dependencies aren't built first How to reproduce: # Remove built dependencies
rm -rf packages/{captions,paths,shapes,whisper-web}/dist
# Try building convert package
bunx turbo run @remotion/convert#build-pageResult: Build fails with Expected: Tasks should wait for upstream dependencies to build successfully, as specified in Turbo task dependencies documentation - Root cause: The convert package has no |
||||||||||||||||||||||||||||
| "outputs": ["spa-dist"], | ||||||||||||||||||||||||||||
| "outputLogs": "new-only" | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
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.
The SelectTrigger components are missing
idattributes that should match thehtmlForattributes on their corresponding labels, breaking the label-to-input association for accessibility.View Details
📝 Patch Details
Analysis
Missing id attributes break label-to-input accessibility association in CompressUi
What fails: SelectTrigger components in CompressUi.tsx lack
idattributes that match their LabelhtmlForattributes, breaking programmatic label-to-input associationHow to reproduce:
Result: Labels with
htmlFor="video-bitrate"andhtmlFor="audio-bitrate"have no matching elements, solabel.controlis null and accessibility features failExpected: Labels should programmatically associate with their inputs per HTML specification, enabling screen reader announcements and click-to-focus behavior
Pattern established: Other components (ConvertForm.tsx, downloadModel.tsx) correctly implement matching htmlFor/id pairs