From eade28079d33bec669e5c9c47ed7c4b6b0d0769f Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Fri, 27 Mar 2026 14:39:19 +0100 Subject: [PATCH] fix: address PR review comments - useCameraDevices: remove selectedDeviceId from useEffect deps (use ref instead) - useCameraDevices: fall back to first available device when selected device is unplugged - i18n: add missing keys (audio.defaultMicrophone, webcam.defaultCamera, webcam.searching) to en/es/zh-CN - LaunchWindow: replace hardcoded strings with t() i18n calls - tests: add afterEach(vi.resetAllMocks()), improve permission test assertions, add stale device fallback test --- src/components/launch/LaunchWindow.tsx | 6 +-- src/hooks/useCameraDevices.test.ts | 51 +++++++++++++++++++++++--- src/hooks/useCameraDevices.ts | 12 ++++-- src/i18n/locales/en/launch.json | 7 +++- src/i18n/locales/es/launch.json | 7 +++- src/i18n/locales/zh-CN/launch.json | 7 +++- 6 files changed, 71 insertions(+), 19 deletions(-) diff --git a/src/components/launch/LaunchWindow.tsx b/src/components/launch/LaunchWindow.tsx index 114cbd8..032ecb4 100644 --- a/src/components/launch/LaunchWindow.tsx +++ b/src/components/launch/LaunchWindow.tsx @@ -112,10 +112,10 @@ export function LaunchWindow() { const selectedMicLabel = micDevices.find((d) => d.deviceId === (microphoneDeviceId || selectedMicId))?.label || - "Default Microphone"; + t("audio.defaultMicrophone"); const selectedCameraLabel = cameraDevices.find((d) => d.deviceId === (webcamDeviceId || selectedCameraId))?.label || - "Default Camera"; + t("webcam.defaultCamera"); const { level } = useAudioLevelMeter({ enabled: showMicControls, @@ -340,7 +340,7 @@ export function LaunchWindow() { /> ) : ( - Searching... + {t("webcam.searching")} )} diff --git a/src/hooks/useCameraDevices.test.ts b/src/hooks/useCameraDevices.test.ts index 349d175..863946a 100644 --- a/src/hooks/useCameraDevices.test.ts +++ b/src/hooks/useCameraDevices.test.ts @@ -1,4 +1,4 @@ -import { renderHook, waitFor } from "@testing-library/react"; +import { act, renderHook, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { useCameraDevices } from "./useCameraDevices"; @@ -28,6 +28,11 @@ Object.defineProperty(global.navigator, "mediaDevices", { describe("useCameraDevices", () => { beforeEach(() => { vi.clearAllMocks(); + mockEnumerateDevices.mockResolvedValue(mockDevices); + }); + + afterEach(() => { + vi.resetAllMocks(); }); it("should list video input devices", async () => { @@ -49,15 +54,49 @@ describe("useCameraDevices", () => { }); }); - it("should request permission if labels are empty", async () => { - mockEnumerateDevices.mockResolvedValueOnce([ - { kind: "videoinput", deviceId: "cam1", label: "", groupId: "group1" }, - ]); + it("should request permission if labels are empty and populate devices after", async () => { + mockEnumerateDevices + .mockResolvedValueOnce([ + { kind: "videoinput", deviceId: "cam1", label: "", groupId: "group1" }, + ]) + .mockResolvedValueOnce(mockDevices); - renderHook(() => useCameraDevices(true)); + const { result } = renderHook(() => useCameraDevices(true)); await waitFor(() => { expect(mockGetUserMedia).toHaveBeenCalledWith({ video: true }); }); + + await waitFor(() => { + expect(result.current.devices[0]?.label).toBe("Camera 1"); + }); + }); + + it("should fall back to first available device when selected device is unplugged", async () => { + const { result } = renderHook(() => useCameraDevices(true)); + + await waitFor(() => { + expect(result.current.selectedDeviceId).toBe("cam1"); + }); + + // 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); + + // Trigger devicechange event via the registered handler + const devicechangeHandler = ( + navigator.mediaDevices.addEventListener as ReturnType + ).mock.calls[0]?.[1] as (() => void) | undefined; + + await act(async () => { + devicechangeHandler?.(); + }); + + await waitFor(() => { + expect(result.current.selectedDeviceId).toBe("cam2"); + }); }); }); diff --git a/src/hooks/useCameraDevices.ts b/src/hooks/useCameraDevices.ts index a456c9c..de12d9d 100644 --- a/src/hooks/useCameraDevices.ts +++ b/src/hooks/useCameraDevices.ts @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; export interface CameraDevice { deviceId: string; @@ -11,6 +11,8 @@ export function useCameraDevices(enabled: boolean = false) { const [selectedDeviceId, setSelectedDeviceId] = useState(""); const [isLoading, setIsLoading] = useState(false); const [error, setError] = useState(null); + const selectedDeviceIdRef = useRef(selectedDeviceId); + selectedDeviceIdRef.current = selectedDeviceId; useEffect(() => { let mounted = true; @@ -44,8 +46,10 @@ export function useCameraDevices(enabled: boolean = false) { if (mounted) { setDevices(videoInputs); - if (!selectedDeviceId && videoInputs.length > 0) { - setSelectedDeviceId(videoInputs[0].deviceId); + const currentId = selectedDeviceIdRef.current; + const stillAvailable = videoInputs.some((d) => d.deviceId === currentId); + if (!currentId || !stillAvailable) { + setSelectedDeviceId(videoInputs[0]?.deviceId ?? ""); } setIsLoading(false); } @@ -64,7 +68,7 @@ export function useCameraDevices(enabled: boolean = false) { mounted = false; navigator.mediaDevices.removeEventListener("devicechange", loadDevices); }; - }, [enabled, selectedDeviceId]); + }, [enabled]); return { devices, selectedDeviceId, setSelectedDeviceId, isLoading, error }; } diff --git a/src/i18n/locales/en/launch.json b/src/i18n/locales/en/launch.json index 6a8011f..f01e295 100644 --- a/src/i18n/locales/en/launch.json +++ b/src/i18n/locales/en/launch.json @@ -10,11 +10,14 @@ "enableSystemAudio": "Enable system audio", "disableSystemAudio": "Disable system audio", "enableMicrophone": "Enable microphone", - "disableMicrophone": "Disable microphone" + "disableMicrophone": "Disable microphone", + "defaultMicrophone": "Default Microphone" }, "webcam": { "enableWebcam": "Enable webcam", - "disableWebcam": "Disable webcam" + "disableWebcam": "Disable webcam", + "defaultCamera": "Default Camera", + "searching": "Searching..." }, "sourceSelector": { "loading": "Loading sources...", diff --git a/src/i18n/locales/es/launch.json b/src/i18n/locales/es/launch.json index 9b7761d..4902404 100644 --- a/src/i18n/locales/es/launch.json +++ b/src/i18n/locales/es/launch.json @@ -10,11 +10,14 @@ "enableSystemAudio": "Activar audio del sistema", "disableSystemAudio": "Desactivar audio del sistema", "enableMicrophone": "Activar micrófono", - "disableMicrophone": "Desactivar micrófono" + "disableMicrophone": "Desactivar micrófono", + "defaultMicrophone": "Micrófono predeterminado" }, "webcam": { "enableWebcam": "Activar cámara web", - "disableWebcam": "Desactivar cámara web" + "disableWebcam": "Desactivar cámara web", + "defaultCamera": "Cámara predeterminada", + "searching": "Buscando..." }, "sourceSelector": { "loading": "Cargando fuentes...", diff --git a/src/i18n/locales/zh-CN/launch.json b/src/i18n/locales/zh-CN/launch.json index 90b8180..48846a0 100644 --- a/src/i18n/locales/zh-CN/launch.json +++ b/src/i18n/locales/zh-CN/launch.json @@ -10,11 +10,14 @@ "enableSystemAudio": "启用系统音频", "disableSystemAudio": "禁用系统音频", "enableMicrophone": "启用麦克风", - "disableMicrophone": "禁用麦克风" + "disableMicrophone": "禁用麦克风", + "defaultMicrophone": "默认麦克风" }, "webcam": { "enableWebcam": "启用摄像头", - "disableWebcam": "禁用摄像头" + "disableWebcam": "禁用摄像头", + "defaultCamera": "默认摄像头", + "searching": "正在搜索..." }, "sourceSelector": { "loading": "正在加载源...",