-
-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: persist & restore screen/mic/webcam selection across sessions #532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -147,17 +147,30 @@ export function registerSettingsHandlers() { | |||||||||||||||||||||||||||||
| microphoneEnabled: parsed.microphoneEnabled === true, | ||||||||||||||||||||||||||||||
| microphoneDeviceId: typeof parsed.microphoneDeviceId === 'string' ? parsed.microphoneDeviceId : undefined, | ||||||||||||||||||||||||||||||
| systemAudioEnabled: parsed.systemAudioEnabled === true, | ||||||||||||||||||||||||||||||
| webcamEnabled: parsed.webcamEnabled === true, | ||||||||||||||||||||||||||||||
| webcamDeviceId: typeof parsed.webcamDeviceId === 'string' ? parsed.webcamDeviceId : undefined, | ||||||||||||||||||||||||||||||
| selectedSourceId: typeof parsed.selectedSourceId === 'string' ? parsed.selectedSourceId : undefined, | ||||||||||||||||||||||||||||||
| selectedSourceName: typeof parsed.selectedSourceName === 'string' ? parsed.selectedSourceName : undefined, | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||
| return { success: true, microphoneEnabled: false, microphoneDeviceId: undefined, systemAudioEnabled: false } | ||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||
| success: true, | ||||||||||||||||||||||||||||||
| microphoneEnabled: false, | ||||||||||||||||||||||||||||||
| microphoneDeviceId: undefined, | ||||||||||||||||||||||||||||||
| systemAudioEnabled: false, | ||||||||||||||||||||||||||||||
| webcamEnabled: false, | ||||||||||||||||||||||||||||||
| webcamDeviceId: undefined, | ||||||||||||||||||||||||||||||
| selectedSourceId: undefined, | ||||||||||||||||||||||||||||||
| selectedSourceName: undefined, | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ipcMain.handle('get-recording-audio-lab-config', () => { | ||||||||||||||||||||||||||||||
| return getBrowserMicrophoneProfileFromEnv() | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ipcMain.handle('set-recording-preferences', async (_, prefs: { microphoneEnabled?: boolean; microphoneDeviceId?: string; systemAudioEnabled?: boolean }) => { | ||||||||||||||||||||||||||||||
| ipcMain.handle('set-recording-preferences', async (_, prefs: { microphoneEnabled?: boolean; microphoneDeviceId?: string; systemAudioEnabled?: boolean; webcamEnabled?: boolean; webcamDeviceId?: string; selectedSourceId?: string; selectedSourceName?: string }) => { | ||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win Align The Proposed fix- ipcMain.handle('set-recording-preferences', async (_, prefs: { microphoneEnabled?: boolean; microphoneDeviceId?: string; systemAudioEnabled?: boolean; webcamEnabled?: boolean; webcamDeviceId?: string; selectedSourceId?: string; selectedSourceName?: string }) => {
+ ipcMain.handle('set-recording-preferences', async (_, prefs: {
+ microphoneEnabled?: boolean;
+ microphoneDeviceId?: string;
+ systemAudioEnabled?: boolean;
+ webcamEnabled?: boolean;
+ webcamDeviceId?: string;
+ selectedSourceId?: string;
+ selectedSourceName?: string;
+ selectedSourceDisplayId?: string;
+ selectedSourceThumbnail?: string | null;
+ selectedSourceAppIcon?: string | null;
+ selectedSourceType?: "screen" | "window";
+ }) => {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| let existing: Record<string, unknown> = {} | ||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,10 @@ export function useLaunchWindowActions() { | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const handleSourceSelect = useCallback(async (source: DesktopSource) => { | ||||||||||||||||||||||||||||||||||||||||||
| await window.electronAPI.selectSource(source); | ||||||||||||||||||||||||||||||||||||||||||
| void window.electronAPI.setRecordingPreferences({ | ||||||||||||||||||||||||||||||||||||||||||
| selectedSourceId: source.id, | ||||||||||||||||||||||||||||||||||||||||||
| selectedSourceName: source.name, | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle rejected preference writes in source selection flow. The persistence call is intentionally not awaited, but it should still be Proposed fix- void window.electronAPI.setRecordingPreferences({
+ void window.electronAPI
+ .setRecordingPreferences({
selectedSourceId: source.id,
selectedSourceName: source.name,
selectedSourceDisplayId: source.display_id,
selectedSourceThumbnail: source.thumbnail,
selectedSourceAppIcon: source.appIcon,
selectedSourceType: source.sourceType,
- });
+ })
+ .catch((error) => {
+ console.warn("Failed to persist selected source preferences:", error);
+ });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| setSelectedSource(source.name); | ||||||||||||||||||||||||||||||||||||||||||
| setHasSelectedSource(true); | ||||||||||||||||||||||||||||||||||||||||||
| window.electronAPI.showSourceHighlight?.({ | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1198,6 +1198,31 @@ export function useScreenRecorder(): UseScreenRecorderReturn { | |
| setMicrophoneDeviceId(result.microphoneDeviceId); | ||
| } | ||
| setSystemAudioEnabled(result.systemAudioEnabled); | ||
| if (result.webcamEnabled) { | ||
| setWebcamEnabled(true); | ||
| } | ||
| if (result.webcamDeviceId) { | ||
| setWebcamDeviceId(result.webcamDeviceId); | ||
| } | ||
| if (result.selectedSourceId && result.selectedSourceName) { | ||
| // Electron desktopCapturer ids are not stable across | ||
| // reboots / display changes. Only restore the source if it | ||
| // still exists in the current capture list, otherwise the | ||
| // app would silently capture a non-existent target. | ||
| try { | ||
| const available = await window.electronAPI.getSources({ | ||
| types: ["screen", "window"], | ||
| }); | ||
| const match = available.find( | ||
| (source) => source.id === result.selectedSourceId, | ||
| ); | ||
| if (match) { | ||
| void window.electronAPI.selectSource(match); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add rejection handling for new fire-and-forget IPC calls. These new non-awaited calls can reject without diagnostics. Please attach Proposed fix- void window.electronAPI.selectSource(match);
+ void window.electronAPI.selectSource(match).catch((error) => {
+ console.warn("Failed to restore persisted capture source:", {
+ error,
+ sourceId: match.id,
+ });
+ });
const persistWebcamEnabled = useCallback((enabled: boolean) => {
setWebcamEnabled(enabled);
- void window.electronAPI.setRecordingPreferences({ webcamEnabled: enabled });
+ void window.electronAPI
+ .setRecordingPreferences({ webcamEnabled: enabled })
+ .catch((error) => {
+ console.warn("Failed to persist webcamEnabled preference:", error);
+ });
}, []);
const persistWebcamDeviceId = useCallback((deviceId: string | undefined) => {
setWebcamDeviceId(deviceId);
- void window.electronAPI.setRecordingPreferences({ webcamDeviceId: deviceId });
+ void window.electronAPI
+ .setRecordingPreferences({ webcamDeviceId: deviceId })
+ .catch((error) => {
+ console.warn("Failed to persist webcamDeviceId preference:", error);
+ });
}, []);Also applies to: 1245-1253 🤖 Prompt for AI Agents |
||
| } | ||
| } catch { | ||
| // Source enumeration failed — skip restore silently. | ||
| } | ||
| } | ||
| } | ||
| })(); | ||
| }, []); | ||
|
|
@@ -1217,6 +1242,16 @@ export function useScreenRecorder(): UseScreenRecorderReturn { | |
| void window.electronAPI.setRecordingPreferences({ systemAudioEnabled: enabled }); | ||
| }, []); | ||
|
|
||
| const persistWebcamEnabled = useCallback((enabled: boolean) => { | ||
| setWebcamEnabled(enabled); | ||
| void window.electronAPI.setRecordingPreferences({ webcamEnabled: enabled }); | ||
| }, []); | ||
|
|
||
| const persistWebcamDeviceId = useCallback((deviceId: string | undefined) => { | ||
| setWebcamDeviceId(deviceId); | ||
| void window.electronAPI.setRecordingPreferences({ webcamDeviceId: deviceId }); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| let cleanup: (() => void) | undefined; | ||
|
|
||
|
|
@@ -2009,9 +2044,9 @@ export function useScreenRecorder(): UseScreenRecorderReturn { | |
| systemAudioEnabled, | ||
| setSystemAudioEnabled: persistSystemAudioEnabled, | ||
| webcamEnabled, | ||
| setWebcamEnabled, | ||
| setWebcamEnabled: persistWebcamEnabled, | ||
| webcamDeviceId, | ||
| setWebcamDeviceId, | ||
| setWebcamDeviceId: persistWebcamDeviceId, | ||
| countdownDelay, | ||
| setCountdownDelay, | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return full selected-source metadata from
get-recording-preferences.This handler currently restores only
selectedSourceId/selectedSourceName, but the renderer restore flow also consumesselectedSourceDisplayId,selectedSourceThumbnail,selectedSourceAppIcon, andselectedSourceType. Missing these forces fallback values during startup restore.Proposed fix
return { success: true, microphoneEnabled: parsed.microphoneEnabled === true, microphoneDeviceId: typeof parsed.microphoneDeviceId === 'string' ? parsed.microphoneDeviceId : undefined, systemAudioEnabled: parsed.systemAudioEnabled === true, webcamEnabled: parsed.webcamEnabled === true, webcamDeviceId: typeof parsed.webcamDeviceId === 'string' ? parsed.webcamDeviceId : undefined, selectedSourceId: typeof parsed.selectedSourceId === 'string' ? parsed.selectedSourceId : undefined, selectedSourceName: typeof parsed.selectedSourceName === 'string' ? parsed.selectedSourceName : undefined, + selectedSourceDisplayId: + typeof parsed.selectedSourceDisplayId === 'string' ? parsed.selectedSourceDisplayId : undefined, + selectedSourceThumbnail: + typeof parsed.selectedSourceThumbnail === 'string' || parsed.selectedSourceThumbnail === null + ? parsed.selectedSourceThumbnail + : undefined, + selectedSourceAppIcon: + typeof parsed.selectedSourceAppIcon === 'string' || parsed.selectedSourceAppIcon === null + ? parsed.selectedSourceAppIcon + : undefined, + selectedSourceType: + parsed.selectedSourceType === 'screen' || parsed.selectedSourceType === 'window' + ? parsed.selectedSourceType + : undefined, } } catch { return { success: true, microphoneEnabled: false, microphoneDeviceId: undefined, systemAudioEnabled: false, webcamEnabled: false, webcamDeviceId: undefined, selectedSourceId: undefined, selectedSourceName: undefined, + selectedSourceDisplayId: undefined, + selectedSourceThumbnail: undefined, + selectedSourceAppIcon: undefined, + selectedSourceType: undefined, } }Also applies to: 161-165
🤖 Prompt for AI Agents