fix: address coderabbit review (concurrent stream, collapsed label, unified select, test quality)
- useCameraDevices: remove getUserMedia label probe to avoid conflict with useScreenRecorder acquiring the real stream; use enumerateDevices only and fall back to 'Camera <id>' for unlabeled devices; gate effect on enabled flag - LaunchWindow: fix selectedCameraLabel to reflect loading/error/empty states in the collapsed view (was always showing 'Default Camera') - LaunchWindow: unify webcam <select> to a single always-mounted element (sr-only when unavailable); mirrors the mic selector pattern - useCameraDevices.test.ts: re-seed mockGetUserMedia in beforeEach after vi.resetAllMocks(); update permission test to assert fallback label behavior
This commit is contained in:
@@ -120,9 +120,14 @@ export function LaunchWindow() {
|
||||
const selectedMicLabel =
|
||||
micDevices.find((d) => d.deviceId === (microphoneDeviceId || selectedMicId))?.label ||
|
||||
t("audio.defaultMicrophone");
|
||||
const selectedCameraLabel =
|
||||
cameraDevices.find((d) => d.deviceId === (webcamDeviceId || selectedCameraId))?.label ||
|
||||
t("webcam.defaultCamera");
|
||||
const selectedCameraLabel = isCameraDevicesLoading
|
||||
? t("webcam.searching")
|
||||
: cameraDevicesError
|
||||
? t("webcam.unavailable")
|
||||
: cameraDevices.length === 0
|
||||
? t("webcam.noneFound")
|
||||
: cameraDevices.find((d) => d.deviceId === (webcamDeviceId || selectedCameraId))?.label ||
|
||||
t("webcam.defaultCamera");
|
||||
|
||||
const { level } = useAudioLevelMeter({
|
||||
enabled: showMicControls,
|
||||
@@ -444,7 +449,7 @@ export function LaunchWindow() {
|
||||
className={`flex items-center gap-0.5 rounded-full p-2 transition-colors duration-150 ${styles.electronNoDrag} ${
|
||||
recording ? "animate-record-pulse bg-red-500/10" : "bg-white/5 hover:bg-white/[0.08]"
|
||||
}`}
|
||||
onClick={hasSelectedSource ? toggleRecording : openSourceSelector}
|
||||
onClick={toggleRecording}
|
||||
disabled={!hasSelectedSource && !recording}
|
||||
style={{ flex: "0 0 auto" }}
|
||||
>
|
||||
|
||||
@@ -29,6 +29,9 @@ describe("useCameraDevices", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockEnumerateDevices.mockResolvedValue(mockDevices);
|
||||
mockGetUserMedia.mockResolvedValue({
|
||||
getTracks: () => [{ stop: vi.fn() }],
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -54,22 +57,18 @@ describe("useCameraDevices", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("should request permission if labels are empty and populate devices after", async () => {
|
||||
mockEnumerateDevices
|
||||
.mockResolvedValueOnce([
|
||||
{ kind: "videoinput", deviceId: "cam1", label: "", groupId: "group1" },
|
||||
])
|
||||
.mockResolvedValueOnce(mockDevices);
|
||||
it("should use device ID as fallback label when label is missing", async () => {
|
||||
mockEnumerateDevices.mockResolvedValueOnce([
|
||||
{ kind: "videoinput", deviceId: "cam1abc123456", label: "", groupId: "group1" },
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useCameraDevices(true));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockGetUserMedia).toHaveBeenCalledWith({ video: true });
|
||||
expect(result.current.devices[0]?.label).toBe("Camera cam1abc1");
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.devices[0]?.label).toBe("Camera 1");
|
||||
});
|
||||
expect(mockGetUserMedia).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should fall back to first available device when selected device is unplugged", async () => {
|
||||
@@ -80,11 +79,10 @@ describe("useCameraDevices", () => {
|
||||
});
|
||||
|
||||
// Simulate cam1 being unplugged — only cam2 remains
|
||||
// loadDevices calls enumerateDevices twice, mock both to return only cam2
|
||||
const cam2Only = [
|
||||
{ kind: "videoinput", deviceId: "cam2", label: "Camera 2", groupId: "group1" },
|
||||
];
|
||||
mockEnumerateDevices.mockResolvedValueOnce(cam2Only).mockResolvedValueOnce(cam2Only);
|
||||
mockEnumerateDevices.mockResolvedValueOnce(cam2Only);
|
||||
|
||||
// Trigger devicechange event via the registered handler
|
||||
const devicechangeHandler = (
|
||||
|
||||
@@ -15,6 +15,7 @@ export function useCameraDevices(enabled: boolean = false) {
|
||||
selectedDeviceIdRef.current = selectedDeviceId;
|
||||
|
||||
useEffect(() => {
|
||||
if (!enabled) return;
|
||||
let mounted = true;
|
||||
|
||||
const loadDevices = async () => {
|
||||
@@ -22,19 +23,8 @@ export function useCameraDevices(enabled: boolean = false) {
|
||||
setIsLoading(true);
|
||||
setError(null);
|
||||
|
||||
// Re-request permission if labels are missing
|
||||
const allDevicesBefore = await navigator.mediaDevices.enumerateDevices();
|
||||
const needsPermission = allDevicesBefore.some((d) => d.kind === "videoinput" && !d.label);
|
||||
|
||||
if (needsPermission && enabled) {
|
||||
try {
|
||||
const stream = await navigator.mediaDevices.getUserMedia({ video: true });
|
||||
stream.getTracks().forEach((track) => track.stop());
|
||||
} catch (e) {
|
||||
console.warn("Failed to get camera permission for labels:", e);
|
||||
}
|
||||
}
|
||||
|
||||
// Enumerate without requesting a second stream — the recorder handles
|
||||
// the real acquisition; unlabeled devices fall back to their device ID.
|
||||
const allDevices = await navigator.mediaDevices.enumerateDevices();
|
||||
const videoInputs = allDevices
|
||||
.filter((device) => device.kind === "videoinput")
|
||||
|
||||
Reference in New Issue
Block a user