-
-
Couldn't load subscription status.
- Fork 1.3k
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?
remotion.dev/convert: Add compression option to ConvertUi component #5853
Conversation
- Created CompressUi component with video and audio bitrate controls - Added compress section to ConvertSections type and ordering - Integrated compression state management in ConvertUi - Pass bitrate values to mediabunny video and audio conversion callbacks - Support separate bitrate control for video (500 Kbps - 10 Mbps) and audio (64 - 320 Kbps) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
| {hasVideo ? ( | ||
| <> | ||
| <Label htmlFor="video-bitrate">Video bitrate</Label> | ||
| <Select |
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 id attributes that should match the htmlFor attributes on their corresponding labels, breaking the label-to-input association for accessibility.
View Details
📝 Patch Details
diff --git a/packages/convert/app/components/CompressUi.tsx b/packages/convert/app/components/CompressUi.tsx
index 904a73611e..a6d3088349 100644
--- a/packages/convert/app/components/CompressUi.tsx
+++ b/packages/convert/app/components/CompressUi.tsx
@@ -42,7 +42,7 @@ export const CompressUi: React.FC<{
setVideoBitrate(val === 'none' ? null : Number(val))
}
>
- <SelectTrigger>
+ <SelectTrigger id="video-bitrate">
<SelectValue placeholder="Select video bitrate" />
</SelectTrigger>
<SelectContent>
@@ -71,7 +71,7 @@ export const CompressUi: React.FC<{
setAudioBitrate(val === 'none' ? null : Number(val))
}
>
- <SelectTrigger>
+ <SelectTrigger id="audio-bitrate">
<SelectValue placeholder="Select audio bitrate" />
</SelectTrigger>
<SelectContent>
Analysis
Missing id attributes break label-to-input accessibility association in CompressUi
What fails: SelectTrigger components in CompressUi.tsx lack id attributes that match their Label htmlFor attributes, breaking programmatic label-to-input association
How to reproduce:
- Navigate to CompressUi component with screen reader
- Focus the select inputs for video/audio bitrate
- Screen reader will not announce the label text
- Click on the label text - select will not receive focus
Result: Labels with htmlFor="video-bitrate" and htmlFor="audio-bitrate" have no matching elements, so label.control is null and accessibility features fail
Expected: 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
- Replace exact bitrate pickers with quality level selectors - Import QUALITY_* constants from mediabunny - Updated CompressUi to use quality levels (Very Low, Low, Medium, High, Very High) - Updated ConvertUi state to use quality instead of bitrate - Pass quality parameter to mediabunny video and audio conversion callbacks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
| "dependsOn": ["make"], | ||
| "outputs": ["build", ".vercel"], | ||
| "outputLogs": "new-only" | ||
| }, | ||
| "@remotion/convert#build-spa": { | ||
| "dependsOn": ["^make"], | ||
| "dependsOn": ["make"], |
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.
| "dependsOn": ["make"], | |
| "outputs": ["build", ".vercel"], | |
| "outputLogs": "new-only" | |
| }, | |
| "@remotion/convert#build-spa": { | |
| "dependsOn": ["^make"], | |
| "dependsOn": ["make"], | |
| "dependsOn": ["^make"], | |
| "outputs": ["build", ".vercel"], | |
| "outputLogs": "new-only" | |
| }, | |
| "@remotion/convert#build-spa": { | |
| "dependsOn": ["^make"], |
The turbo.json build dependencies were changed from ["^make"] to ["make"], which removes the requirement for dependency packages' make tasks to run first. This could cause build failures if these tasks depend on code from other packages.
View Details
Analysis
Broken turbo dependency configuration causes convert package build failures
What 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 Failed to resolve entry for package "@remotion/whisper-web". The package depends on workspace packages (@remotion/captions, @remotion/paths, @remotion/shapes, @remotion/whisper-web) that must be built first.
Expected: Tasks should wait for upstream dependencies to build successfully, as specified in Turbo task dependencies documentation - ^make runs dependencies' make tasks first, while make only looks within the same package.
Root cause: The convert package has no make script, so "dependsOn": ["make"] resolves to nothing, bypassing dependency builds entirely.
Uh oh!
There was an error while loading. Please reload this page.