From 9817c85acfd000bb7229ce83e82864bb9ed63e7d Mon Sep 17 00:00:00 2001
From: Etienne Lescot <etiennelescot@gmail.com>
Date: Fri, 27 Mar 2026 15:15:43 +0100
Subject: [PATCH] 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
---
 src/components/launch/LaunchWindow.tsx | 13 +++++++++----
 src/hooks/useCameraDevices.test.ts     | 22 ++++++++++------------
 src/hooks/useCameraDevices.ts          | 16 +++-------------
 3 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/src/components/launch/LaunchWindow.tsx b/src/components/launch/LaunchWindow.tsx
index 2210276..0d7ae23 100644
--- a/src/components/launch/LaunchWindow.tsx
+++ b/src/components/launch/LaunchWindow.tsx
@@ -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" }}
 				>
diff --git a/src/hooks/useCameraDevices.test.ts b/src/hooks/useCameraDevices.test.ts
index 863946a..9026ce6 100644
--- a/src/hooks/useCameraDevices.test.ts
+++ b/src/hooks/useCameraDevices.test.ts
@@ -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 = (
diff --git a/src/hooks/useCameraDevices.ts b/src/hooks/useCameraDevices.ts
index de12d9d..a02e65a 100644
--- a/src/hooks/useCameraDevices.ts
+++ b/src/hooks/useCameraDevices.ts
@@ -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")