Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
94 changes: 94 additions & 0 deletions packages/convert/app/components/CompressUi.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import React from 'react';
import {Label} from './ui/label';
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from './ui/select';

const formatBitrate = (bitrate: number): string => {
if (bitrate >= 1000000) {
return `${(bitrate / 1000000).toFixed(1)} Mbps`;
}
return `${(bitrate / 1000).toFixed(0)} Kbps`;

Check failure on line 15 in packages/convert/app/components/CompressUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Expected blank line before this statement
};

export const CompressUi: React.FC<{
videoBitrate: number | null;

Check failure on line 19 in packages/convert/app/components/CompressUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Prop 'videoBitrate' should be read-only
setVideoBitrate: (bitrate: number | null) => void;

Check failure on line 20 in packages/convert/app/components/CompressUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Prop 'setVideoBitrate' should be read-only
audioBitrate: number | null;

Check failure on line 21 in packages/convert/app/components/CompressUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Prop 'audioBitrate' should be read-only
setAudioBitrate: (bitrate: number | null) => void;

Check failure on line 22 in packages/convert/app/components/CompressUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Prop 'setAudioBitrate' should be read-only
hasVideo: boolean;

Check failure on line 23 in packages/convert/app/components/CompressUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Prop 'hasVideo' should be read-only
hasAudio: boolean;

Check failure on line 24 in packages/convert/app/components/CompressUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Prop 'hasAudio' should be read-only
}> = ({
videoBitrate,
setVideoBitrate,
audioBitrate,
setAudioBitrate,
hasVideo,
hasAudio,
}) => {
return (
<div>
<div className="h-4" />
{hasVideo ? (
<>
<Label htmlFor="video-bitrate">Video bitrate</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={String(videoBitrate ?? 'none')}
onValueChange={(val) =>
setVideoBitrate(val === 'none' ? null : Number(val))
}
>
<SelectTrigger>
<SelectValue placeholder="Select video bitrate" />
</SelectTrigger>
<SelectContent>
<SelectItem value="none">No compression</SelectItem>
<SelectItem value="500000">{formatBitrate(500000)}</SelectItem>
<SelectItem value="1000000">{formatBitrate(1000000)}</SelectItem>
<SelectItem value="2000000">{formatBitrate(2000000)}</SelectItem>
<SelectItem value="3000000">{formatBitrate(3000000)}</SelectItem>
<SelectItem value="5000000">{formatBitrate(5000000)}</SelectItem>
<SelectItem value="8000000">{formatBitrate(8000000)}</SelectItem>
<SelectItem value="10000000">{formatBitrate(10000000)}</SelectItem>
</SelectContent>
</Select>
<div className="text-sm mt-2 text-muted-foreground">
Lower bitrate reduces file size but may affect quality.
</div>
</>
) : null}
{hasVideo && hasAudio ? <div className="h-4" /> : null}
{hasAudio ? (
<>
<Label htmlFor="audio-bitrate">Audio bitrate</Label>
<Select
value={String(audioBitrate ?? 'none')}
onValueChange={(val) =>
setAudioBitrate(val === 'none' ? null : Number(val))
}
>
<SelectTrigger>
<SelectValue placeholder="Select audio bitrate" />
</SelectTrigger>
<SelectContent>
<SelectItem value="none">No compression</SelectItem>
<SelectItem value="64000">{formatBitrate(64000)}</SelectItem>
<SelectItem value="96000">{formatBitrate(96000)}</SelectItem>
<SelectItem value="128000">{formatBitrate(128000)}</SelectItem>
<SelectItem value="192000">{formatBitrate(192000)}</SelectItem>
<SelectItem value="256000">{formatBitrate(256000)}</SelectItem>
<SelectItem value="320000">{formatBitrate(320000)}</SelectItem>
</SelectContent>
</Select>
<div className="text-sm mt-2 text-muted-foreground">
Lower bitrate reduces file size but may affect audio quality.
</div>
</>
) : null}
</div>
);
};
54 changes: 54 additions & 0 deletions packages/convert/app/components/ConvertUi.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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 Down Expand Up @@ -139,6 +140,10 @@
useState(false);
const [resampleRate, setResampleRate] = useState<number>(16000);

const [compressActive, setCompressActive] = useState(false);
const [videoBitrate, setVideoBitrate] = useState<number | null>(2000000);
const [audioBitrate, setAudioBitrate] = useState<number | null>(128000);

const canResample = useMemo(() => {
return tracks?.find((t) => t.isAudioTrack());
}, [tracks]);
Expand All @@ -155,6 +160,28 @@
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 actualVideoBitrate = useMemo(() => {
if (!hasVideo || !compressActive) {
return null;
}
return videoBitrate;

Check failure on line 175 in packages/convert/app/components/ConvertUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Expected blank line before this statement
}, [videoBitrate, hasVideo, compressActive]);

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

Check failure on line 182 in packages/convert/app/components/ConvertUi.tsx

View workflow job for this annotation

GitHub Actions / Linting + Formatting

Expected blank line before this statement
}, [audioBitrate, hasAudio, compressActive]);

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

return sample;
},
bitrate: actualAudioBitrate ?? undefined,
};
},
});
Expand Down Expand Up @@ -429,6 +458,8 @@
videoOperationSelection,
crop,
cropRect,
actualVideoBitrate,
actualAudioBitrate,
]);

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

if (section === 'compress') {
return (
<div key="compress">
<ConvertUiSection
active={compressActive}
setActive={setCompressActive}
>
Compress
</ConvertUiSection>
{compressActive ? (
<CompressUi
videoBitrate={videoBitrate}
setVideoBitrate={setVideoBitrate}
audioBitrate={audioBitrate}
setAudioBitrate={setAudioBitrate}
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
Loading