Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions packages/convert/app/components/CompressUi.tsx
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
Copy link
Contributor

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:

  1. Navigate to CompressUi component with screen reader
  2. Focus the select inputs for video/audio bitrate
  3. Screen reader will not announce the label text
  4. 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

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>
);
};
72 changes: 71 additions & 1 deletion packages/convert/app/components/ConvertUi.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ import type {
InputFormat,
InputTrack,
InputVideoTrack,
QUALITY_HIGH,
QUALITY_LOW,
QUALITY_VERY_HIGH,
QUALITY_VERY_LOW,
Rotation,
} from 'mediabunny';
import {Conversion, Output, StreamTarget} from 'mediabunny';
import {Conversion, Output, QUALITY_MEDIUM, StreamTarget} from 'mediabunny';
import React, {useCallback, useMemo, useState} from 'react';
import {applyCrop} from '~/lib/apply-crop';
import type {Dimensions} from '~/lib/calculate-new-dimensions-from-dimensions';
Expand Down Expand Up @@ -37,6 +41,7 @@ import type {ConvertProgressType} from '~/lib/progress';
import {makeWaveformVisualizer} from '~/lib/waveform-visualizer';
import {makeWebFsTarget} from '~/lib/web-fs-target';
import type {OutputContainer, RouteAction} from '~/seo';
import {CompressUi} from './CompressUi';
import {ConversionDone} from './ConversionDone';
import {ConvertForm} from './ConvertForm';
import {ConvertProgress, convertProgressRef} from './ConvertProgress';
Expand All @@ -51,6 +56,14 @@ import {RotateComponents} from './RotateComponents';
import {useSupportedConfigs} from './use-supported-configs';
import type {VideoThumbnailRef} from './VideoThumbnail';

type QualityLevel =
| typeof QUALITY_VERY_LOW
| typeof QUALITY_LOW
| typeof QUALITY_MEDIUM
| typeof QUALITY_HIGH
| typeof QUALITY_VERY_HIGH
| null;

const ConvertUI = ({
currentAudioCodec,
currentVideoCodec,
Expand Down Expand Up @@ -139,6 +152,12 @@ const ConvertUI = ({
useState(false);
const [resampleRate, setResampleRate] = useState<number>(16000);

const [compressActive, setCompressActive] = useState(false);
const [videoQuality, setVideoQuality] =
useState<QualityLevel>(QUALITY_MEDIUM);
const [audioQuality, setAudioQuality] =
useState<QualityLevel>(QUALITY_MEDIUM);

const canResample = useMemo(() => {
return tracks?.find((t) => t.isAudioTrack());
}, [tracks]);
Expand All @@ -155,6 +174,30 @@ const ConvertUI = ({
return resampleRate;
}, [resampleRate, canResample, resampleUserPreferenceActive]);

const hasVideo = useMemo(() => {
return (tracks?.filter((t) => t.isVideoTrack()).length ?? 0) > 0;
}, [tracks]);

const hasAudio = useMemo(() => {
return (tracks?.filter((t) => t.isAudioTrack()).length ?? 0) > 0;
}, [tracks]);

const actualVideoQuality = useMemo(() => {
if (!hasVideo || !compressActive) {
return null;
}

return videoQuality;
}, [videoQuality, hasVideo, compressActive]);

const actualAudioQuality = useMemo(() => {
if (!hasAudio || !compressActive) {
return null;
}

return audioQuality;
}, [audioQuality, hasAudio, compressActive]);

const supportedConfigs = useSupportedConfigs({
outputContainer,
tracks,
Expand Down Expand Up @@ -316,6 +359,7 @@ const ConvertUI = ({
dimensionsAfterCrop ?? null,
),
codec: operation.videoCodec,
bitrate: actualVideoQuality ?? undefined,
};
},
audio: (audioTrack) => {
Expand Down Expand Up @@ -351,6 +395,7 @@ const ConvertUI = ({

return sample;
},
bitrate: actualAudioQuality ?? undefined,
};
},
});
Expand Down Expand Up @@ -429,6 +474,8 @@ const ConvertUI = ({
videoOperationSelection,
crop,
cropRect,
actualVideoQuality,
actualAudioQuality,
]);

const dimissError = useCallback(() => {
Expand Down Expand Up @@ -761,6 +808,29 @@ const ConvertUI = ({
);
}

if (section === 'compress') {
return (
<div key="compress">
<ConvertUiSection
active={compressActive}
setActive={setCompressActive}
>
Compress
</ConvertUiSection>
{compressActive ? (
<CompressUi
videoQuality={videoQuality}
setVideoQuality={setVideoQuality}
audioQuality={audioQuality}
setAudioQuality={setAudioQuality}
hasVideo={hasVideo}
hasAudio={hasAudio}
/>
) : null}
</div>
);
}

throw new Error('Unknown section ' + (section satisfies never));
})}
</div>
Expand Down
9 changes: 8 additions & 1 deletion packages/convert/app/lib/default-ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ export type ConvertSections =
| 'mirror'
| 'resize'
| 'crop'
| 'resample';
| 'resample'
| 'compress';

export const getOrderOfSections = (
action: RouteAction,
Expand All @@ -139,6 +140,7 @@ export const getOrderOfSections = (
mirror: 3,
convert: 4,
resample: 5,
compress: 6,
};
}

Expand All @@ -150,6 +152,7 @@ export const getOrderOfSections = (
mirror: 3,
convert: 4,
resample: 5,
compress: 6,
};
}

Expand All @@ -161,6 +164,7 @@ export const getOrderOfSections = (
rotate: 3,
mirror: 4,
resample: 5,
compress: 6,
};
}

Expand All @@ -172,6 +176,7 @@ export const getOrderOfSections = (
rotate: 3,
mirror: 4,
resample: 5,
compress: 6,
};
}

Expand All @@ -183,6 +188,7 @@ export const getOrderOfSections = (
rotate: 3,
convert: 4,
resample: 5,
compress: 6,
};
}

Expand All @@ -199,6 +205,7 @@ export const getOrderOfSections = (
mirror: 3,
convert: 4,
resample: 5,
compress: 6,
};
}

Expand Down
4 changes: 2 additions & 2 deletions turbo.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"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-page

Result: 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.

"outputs": ["spa-dist"],
"outputLogs": "new-only"
}
Expand Down
Loading