Add launch global shortcuts to tray and More menu#654
Open
awaismirza wants to merge 2 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds global OS-level keyboard shortcut support for recording control on macOS, expands recording state tracking to include a ChangesGlobal Launch Shortcuts and Recording Control
Sequence Diagram(s)sequenceDiagram
participant User
participant Keyboard as Window Keyboard
participant Electron as Electron Main
participant Recorder as useScreenRecorder
participant HUD as HUD Window
User->>Keyboard: Press Ctrl+Shift+R (Start)
Keyboard->>HUD: useLaunchShortcuts detects key
HUD->>Recorder: toggleRecording() if not recording
Recorder->>Electron: setRecordingState(true, false)
Electron->>HUD: onRecordingStateChanged({recording:true, paused:false})
Electron->>Electron: Update Tray Menu
User->>Electron: Click Pause on Tray
Electron->>HUD: sendTrayRecordingCommand('pause')
HUD->>Recorder: pauseRecording()
Recorder->>Electron: setRecordingState(true, true)
Electron->>HUD: onRecordingStateChanged({recording:true, paused:true})
Electron->>Electron: Update Tray Menu (Resume shown)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
awaismirza
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
electron/ipc/recording/mac.ts (1)
166-175:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUnexpected helper exits still bypass the tray state pipeline.
Line 169 broadcasts the renderer event, but this path never invokes the
onRecordingStateChangecallback thatelectron/main.tsuses at Lines 1014-1019 to refresh the tray/menu state. The renderer listener only updates local UI state, so an unexpected native stop leaves the tray stuck in Recording/Paused until some later explicitset-recording-statecall. The same gap exists inelectron/ipc/recording/windows.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/ipc/recording/mac.ts` around lines 166 - 175, The broadcast to renderer windows (BrowserWindow.getAllWindows().forEach(...) sending "recording-state-changed") updates only UI but skips the tray/menu pipeline; after sending the renderer event, also invoke the same internal callback used by main (onRecordingStateChange) with the identical payload ({ recording: false, paused: false, sourceName }) so the tray/menu state is refreshed; apply the same change to the Windows implementation in electron/ipc/recording/windows.ts.electron/ipc/register/recording.ts (1)
1813-1829:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't reinitialize cursor capture on pause/resume updates.
Once
pausedbecame part of this IPC contract, Line 1814 still treats everyrecording === truecall as a fresh start. A pause or resume transition will now clear buffered cursor samples, reset the capture clock, and restart sampling, even though Lines 1858-1867 already model pause boundaries separately. Gate this setup/teardown work to real start/stop transitions only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/ipc/register/recording.ts` around lines 1813 - 1829, Gate the heavy cursor init/teardown to real recording start/stop transitions instead of any call with recording === true; detect the previous recording state (e.g., read an existing getter like getIsRecording() or introduce a module-scoped prevRecording flag) and only run the startup sequence (stopCursorCapture, stopInteractionCapture, startWindowBoundsCapture, startNativeCursorMonitor, setIsCursorCaptureActive, setActiveCursorSamples, setPendingCursorSamples, setCursorCaptureStartTimeMs, resetCursorCaptureClock, setLinuxCursorScreenPoint, setLastLeftClick, sampleCursorPoint, startCursorSampling, startInteractionCapture) when prevRecording === false && recording === true, and only run the teardown when prevRecording === true && recording === false; leave pause/resume updates (the logic already around lines 1858-1867) to handle paused toggles without clearing buffers or resetting clocks.
🧹 Nitpick comments (2)
src/hooks/useScreenRecorder.ts (1)
1371-1375: Tray stop only handled via generic channel; legacy listener appears unused
Insrc/hooks/useScreenRecorder.ts,stopRecording.current()is triggered by the genericonTrayRecordingCommandwhencommand === "stop"(~2227-2240). The legacyonStopRecordingFromTraysubscription (~1369-1375) listens for"stop-recording-from-tray", but the repo contains no corresponding sender/emit path for that channel (only the preload listener).
Optional cleanup: remove or gate the legacy listener to prevent a future double-stop regression.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useScreenRecorder.ts` around lines 1371 - 1375, The legacy tray handler onStopRecordingFromTray in useScreenRecorder registers a listener that calls stopRecording.current(), but the code already handles tray "stop" via onTrayRecordingCommand, so keep them from double-invoking stop. Remove the onStopRecordingFromTray subscription or guard it so it only registers when onTrayRecordingCommand is not available (check window.electronAPI?.onTrayRecordingCommand first); update the cleanup assignment to match the chosen approach and ensure stopRecording.current is only called once. Reference the functions/fields: useScreenRecorder, window.electronAPI.onStopRecordingFromTray, window.electronAPI.onTrayRecordingCommand, and stopRecording.current when making the change.src/components/launch/popovers/MorePopover.test.tsx (1)
168-174: ⚡ Quick winAdd coverage for the countdown-disabled start state.
countdownActivenow gates the primary launch action, but this suite only exercises the enabled idle path. A small assertion here would lock down the new branch and catch regressions in the menu state logic.Suggested test
it("shows the start recording shortcut in idle state", () => { const buttons = renderButtons(); const startButton = findButton(buttons, "Start Recording"); expect(startButton).toBeDefined(); expect(extractText(startButton)).toContain("Ctrl + Shift + R"); }); + +it("disables start while the countdown is active", () => { + const buttons = renderButtons({ countdownActive: true }); + const startButton = findButton(buttons, "Start Recording"); + + expect(startButton).toBeDefined(); + expect(startButton?.props.disabled).toBe(true); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/popovers/MorePopover.test.tsx` around lines 168 - 174, Update the test to also cover the countdown-disabled path: call renderButtons with countdownActive: true (or otherwise simulate the countdownActive state) then locate the "Start Recording" action with findButton and assert that this primary launch action is disabled (e.g., expect(startButton).toHaveAttribute('aria-disabled', 'true') or expect(startButton).toBeDisabled()) to lock down the branch introduced by countdownActive; use the existing renderButtons, findButton and extractText helpers to implement this assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/ipc/register/settings.ts`:
- Around line 223-245: The loop over config entries calls
toElectronAccelerator(binding) without validating that binding is an object with
the expected shape, so a malformed persisted entry can throw and abort
registering all shortcuts; before calling toElectronAccelerator (inside the
for...of that uses isLaunchShortcutAction), check that binding is an object and
binding.key is a non-empty string (and any other expected fields),
log/console.warn and continue for invalid entries, and/or wrap the per-binding
conversion/registration (the call to toElectronAccelerator and
globalShortcut.register) in its own try/catch so one bad entry doesn't prevent
others from registering.
In `@electron/main.ts`:
- Around line 246-271: The tray commands must always go to the HUD overlay
window rather than falling back to mainWindow; update sendTrayRecordingCommand
to stop using mainWindow as a fallback and instead ensure a HUD overlay is
created (call createWindow() or the HUD-specific creator) and re-query
getHudOverlayWindow()—only send "tray-recording-command" when the resolved
target is the HUD overlay (and is not destroyed); if the HUD still doesn't exist
after creation, return without sending. Also remove reliance on mainWindow (and
be mindful of createEditorWindowWrapper re-binding mainWindow) so the command is
never routed to the editor.
In `@src/App.tsx`:
- Around line 62-65: The app currently mounts ShortcutsProvider around
LaunchWindow (wrapping <LaunchWindow /> and <Toaster />) which duplicates the
provider already mounted by the editor path and causes racey app-wide shortcut
registration; remove the ShortcutsProvider wrapper here so only the
editor-mounted provider owns global shortcuts (i.e., render <LaunchWindow /> and
<Toaster /> directly), or alternatively hoist a single ShortcutsProvider to the
true application root so ShortcutsProvider is instantiated exactly once; refer
to the ShortcutsProvider and LaunchWindow components to locate and change the
wrapper.
In `@src/components/launch/hooks/useLaunchShortcuts.ts`:
- Around line 84-111: On macOS the DOM keydown handler and the Electron
global-shortcut subscription can both fire for the same keypress causing double
execution; fix this by tracking recent Electron-triggered launches and ignoring
the DOM handler when an Electron trigger was just received: add a ref like
lastElectronTriggerRef used in the effect that subscribes to
window.electronAPI.onLaunchShortcutTriggered to set
lastElectronTriggerRef.current = Date.now() when it calls
runLaunchShortcut(action), and in the onKeyDown handler (inside the useEffect
that iterates LAUNCH_SHORTCUT_ACTIONS and calls matchesShortcut with
launchShortcuts and isMac) early-return when isMac && Date.now() -
(lastElectronTriggerRef.current || 0) < 100 (or similar small debounce window)
before calling runLaunchShortcut; ensure the ref is declared in the hook scope
so both effects can access it.
In `@src/components/video-editor/ShortcutsConfigDialog.tsx`:
- Around line 177-183: In handleSave, call persistShortcuts(draft, launchDraft)
and await its resolution before calling setShortcuts and setLaunchShortcuts so
live state isn't mutated on a failed save; wrap the await in try/catch, on
success call setShortcuts(draft), setLaunchShortcuts(launchDraft),
toast.success(t("shortcutsConfig.saved")) and closeConfig(), and on error show
toast.error with the error message and do not update live bindings or close the
dialog; reference handleSave, persistShortcuts, setShortcuts,
setLaunchShortcuts, draft, launchDraft, toast, and closeConfig when making the
change.
In `@src/contexts/ShortcutsContext.tsx`:
- Around line 73-85: The current expression calls .then on a possibly undefined
return from
window.electronAPI?.registerLaunchGlobalShortcuts?.(launchShortcuts), which can
throw; change it to first capture the call into a variable and only attach .then
if it's defined (e.g., const promise =
window.electronAPI?.registerLaunchGlobalShortcuts?.(launchShortcuts); if
(promise) promise.then(...)), or alternatively await the call inside an async
function after checking window.electronAPI and registerLaunchGlobalShortcuts
exist; update the logic around launchShortcuts and the .then result handling
(result?.success / result.failedRegistrations) accordingly so .then is never
invoked on undefined.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 2227-2231: The tray "start" case currently calls toggleRecording()
which can stop an active recording; change the onTrayRecordingCommand handler in
useScreenRecorder.ts so that the "start" command calls the explicit
startRecording() method (and only starts when not already recording) and ensure
"stop" calls stopRecording() (or the explicit stop method) rather than
toggleRecording(); update the switch in the callback to use startRecording() for
"start" and stopRecording() for "stop" to enforce start-only/stop-only semantics
and avoid accidental toggles.
In `@src/i18n/locales/es/dialogs.json`:
- Around line 48-50: Update the Spanish translations for the shortcut dialog
keys localShortcuts, globalShortcuts, and globalDescription so they are in
Spanish instead of English; locate the JSON entries "localShortcuts",
"globalShortcuts", and "globalDescription" in the Spanish locales file and
replace their English strings with the correct Spanish translations (e.g.,
"Atajos locales", "Atajos globales" and an equivalent Spanish sentence for
"Available on macOS even when Recordly is in the background.").
In `@src/i18n/locales/es/launch.json`:
- Line 13: The Spanish label for the launch menu key "startRecording" is missing
an accent; update its value from "Iniciar grabacion" to "Iniciar grabación" and
also fix the other newly added Spanish label in this file (the added entry at
the file's line 29) to include the appropriate accents (e.g., change "grabacion"
to "grabación" in that string) so both entries match proper orthography.
In `@src/i18n/locales/fr/dialogs.json`:
- Around line 48-50: Translate the three English strings for the French locale
by replacing the values for the keys "localShortcuts", "globalShortcuts", and
"globalDescription" in the dialogs.json fragment with proper French translations
(e.g., "Raccourcis locaux", "Raccourcis globaux d'enregistrement", and
"Disponible sur macOS même lorsque Recordly est en arrière-plan.") so the
shortcuts dialog is fully localized.
In `@src/i18n/locales/fr/launch.json`:
- Line 13: Update the French labels to use correct accents: change the
"startRecording" value to "Démarrer l'enregistrement" and the corresponding
reactivation label (e.g., "reactivateRecording") to "Réactiver l'enregistrement"
so both strings use proper French orthography and capitalization.
In `@src/i18n/locales/it/dialogs.json`:
- Around line 48-50: The three i18n strings localShortcuts, globalShortcuts, and
globalDescription in the Italian locale are still in English; update their
values to Italian (e.g., localShortcuts -> "Scorciatoie locali", globalShortcuts
-> "Scorciatoie globali di registrazione", globalDescription -> "Disponibili su
macOS anche quando Recordly è in background.") so the shortcuts dialog is fully
translated for Italian users.
In `@src/i18n/locales/ko/dialogs.json`:
- Around line 48-50: Translate the three new keys "localShortcuts",
"globalShortcuts", and "globalDescription" in the Korean locale (dialogs.json)
into proper Korean strings; update the values for those keys so they are fully
localized (keep the keys unchanged), ensure valid JSON string syntax (escape
quotes if needed) and match the tone of existing translations in this file so
the shortcuts UI is consistently Korean.
In `@src/i18n/locales/nl/dialogs.json`:
- Around line 48-50: The three English-only keys "localShortcuts",
"globalShortcuts", and "globalDescription" in src/i18n/locales/nl/dialogs.json
must be translated to Dutch; update the values for these keys with proper Dutch
strings (e.g., "Lokale snelkoppelingen" for localShortcuts, "Globale
opname-snelkoppelingen" or similar for globalShortcuts, and a Dutch equivalent
for globalDescription such as "Beschikbaar op macOS, zelfs wanneer Recordly op
de achtergrond draait.") so the Dutch locale is consistent.
In `@src/i18n/locales/pt-BR/dialogs.json`:
- Around line 48-50: Translate the three English shortcut label entries in the
pt-BR locale by replacing the values for "localShortcuts", "globalShortcuts",
and "globalDescription" in dialogs.json with Brazilian Portuguese equivalents
(e.g., "Atalhos locais", "Atalhos globais de gravação", and a Portuguese version
of the description such as "Disponível no macOS mesmo quando o Recordly está em
segundo plano."). Ensure the keys remain unchanged and only the string values
are updated so the UI shows consistent Portuguese text.
In `@src/i18n/locales/pt-BR/launch.json`:
- Line 13: Update the pt-BR translation value for the key "startRecording" in
the launch.json locale file: replace the current string "Iniciar gravacao" with
the correctly accented Portuguese "Iniciar gravação" so the startRecording entry
uses the proper spelling.
In `@src/i18n/locales/ru/dialogs.json`:
- Around line 48-50: Translate the three English strings in the Russian locale
by replacing the values for the JSON keys "localShortcuts", "globalShortcuts",
and "globalDescription" with their Russian equivalents so the shortcuts dialog
is fully localized; update "localShortcuts" to a Russian phrase for "Local
shortcuts", "globalShortcuts" to Russian for "Global recording shortcuts", and
"globalDescription" to Russian describing that these are available on macOS even
when the app is in the background.
In `@src/i18n/locales/zh-CN/dialogs.json`:
- Around line 48-50: The three English strings for the shortcut dialog (keys
localShortcuts, globalShortcuts, globalDescription) are untranslated; replace
their values with Simplified Chinese equivalents: set localShortcuts to "本地快捷键",
globalShortcuts to "全局录制快捷键", and globalDescription to "在 macOS 上可用，即使 Recordly
处于后台运行。".
In `@src/i18n/locales/zh-TW/dialogs.json`:
- Around line 48-50: Replace the English values for the keys localShortcuts,
globalShortcuts, and globalDescription in the zh-TW dialogs JSON with proper
Traditional Chinese translations: update "localShortcuts" to a Traditional
Chinese string for "Local shortcuts", "globalShortcuts" to one for "Global
recording shortcuts", and "globalDescription" to a Chinese sentence conveying
"Available on macOS even when Recordly is in the background."; keep the keys
unchanged (localShortcuts, globalShortcuts, globalDescription) and ensure the
JSON string quoting and punctuation remain valid.
---
Outside diff comments:
In `@electron/ipc/recording/mac.ts`:
- Around line 166-175: The broadcast to renderer windows
(BrowserWindow.getAllWindows().forEach(...) sending "recording-state-changed")
updates only UI but skips the tray/menu pipeline; after sending the renderer
event, also invoke the same internal callback used by main
(onRecordingStateChange) with the identical payload ({ recording: false, paused:
false, sourceName }) so the tray/menu state is refreshed; apply the same change
to the Windows implementation in electron/ipc/recording/windows.ts.
In `@electron/ipc/register/recording.ts`:
- Around line 1813-1829: Gate the heavy cursor init/teardown to real recording
start/stop transitions instead of any call with recording === true; detect the
previous recording state (e.g., read an existing getter like getIsRecording() or
introduce a module-scoped prevRecording flag) and only run the startup sequence
(stopCursorCapture, stopInteractionCapture, startWindowBoundsCapture,
startNativeCursorMonitor, setIsCursorCaptureActive, setActiveCursorSamples,
setPendingCursorSamples, setCursorCaptureStartTimeMs, resetCursorCaptureClock,
setLinuxCursorScreenPoint, setLastLeftClick, sampleCursorPoint,
startCursorSampling, startInteractionCapture) when prevRecording === false &&
recording === true, and only run the teardown when prevRecording === true &&
recording === false; leave pause/resume updates (the logic already around lines
1858-1867) to handle paused toggles without clearing buffers or resetting
clocks.
---
Nitpick comments:
In `@src/components/launch/popovers/MorePopover.test.tsx`:
- Around line 168-174: Update the test to also cover the countdown-disabled
path: call renderButtons with countdownActive: true (or otherwise simulate the
countdownActive state) then locate the "Start Recording" action with findButton
and assert that this primary launch action is disabled (e.g.,
expect(startButton).toHaveAttribute('aria-disabled', 'true') or
expect(startButton).toBeDisabled()) to lock down the branch introduced by
countdownActive; use the existing renderButtons, findButton and extractText
helpers to implement this assertion.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 1371-1375: The legacy tray handler onStopRecordingFromTray in
useScreenRecorder registers a listener that calls stopRecording.current(), but
the code already handles tray "stop" via onTrayRecordingCommand, so keep them
from double-invoking stop. Remove the onStopRecordingFromTray subscription or
guard it so it only registers when onTrayRecordingCommand is not available
(check window.electronAPI?.onTrayRecordingCommand first); update the cleanup
assignment to match the chosen approach and ensure stopRecording.current is only
called once. Reference the functions/fields: useScreenRecorder,
window.electronAPI.onStopRecordingFromTray,
window.electronAPI.onTrayRecordingCommand, and stopRecording.current when making
the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 65f14aff-1a86-4df7-bd16-9d2f412436dc
📒 Files selected for processing (49)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/ipc/recording/mac.tselectron/ipc/recording/windows.tselectron/ipc/register/recording.tselectron/ipc/register/settings.tselectron/ipc/shortcutTypes.tselectron/main.tselectron/native/bin/darwin-arm64/recordly-native-cursor-monitorelectron/native/bin/darwin-arm64/recordly-system-cursorselectron/native/bin/darwin-arm64/recordly-window-listelectron/native/bin/darwin-x64/recordly-native-cursor-monitorelectron/native/bin/darwin-x64/recordly-system-cursorselectron/native/bin/darwin-x64/recordly-window-listelectron/preload.tssrc/App.tsxsrc/components/launch/LaunchWindow.module.csssrc/components/launch/LaunchWindow.tsxsrc/components/launch/hooks/useLaunchShortcuts.tssrc/components/launch/hooks/useLaunchWindowActions.test.tssrc/components/launch/hooks/useLaunchWindowActions.tssrc/components/launch/hooks/useLaunchWindowSystemState.tssrc/components/launch/popovers/MorePopover.test.tsxsrc/components/launch/popovers/MorePopover.tsxsrc/components/launch/popovers/PopoverScaffold.tsxsrc/components/video-editor/ShortcutsConfigDialog.tsxsrc/contexts/ShortcutsContext.tsxsrc/hooks/useScreenRecorder.tssrc/i18n/locales/en/dialogs.jsonsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/dialogs.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/fr/dialogs.jsonsrc/i18n/locales/fr/launch.jsonsrc/i18n/locales/it/dialogs.jsonsrc/i18n/locales/it/launch.jsonsrc/i18n/locales/ko/dialogs.jsonsrc/i18n/locales/ko/launch.jsonsrc/i18n/locales/nl/dialogs.jsonsrc/i18n/locales/nl/launch.jsonsrc/i18n/locales/pt-BR/dialogs.jsonsrc/i18n/locales/pt-BR/launch.jsonsrc/i18n/locales/ru/dialogs.jsonsrc/i18n/locales/zh-CN/dialogs.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/i18n/locales/zh-TW/dialogs.jsonsrc/i18n/locales/zh-TW/launch.jsonsrc/lib/shortcuts.test.tssrc/lib/shortcuts.ts
Comment on lines
+223
to
+245
| if (!config || typeof config !== "object") { | ||
| return { success: true }; | ||
| } | ||
|
|
||
| const failedRegistrations: Array<{ | ||
| action: LaunchShortcutAction; | ||
| accelerator: string; | ||
| }> = []; | ||
|
|
||
| for (const [action, binding] of Object.entries( | ||
| config as Record<string, ShortcutBinding>, | ||
| )) { | ||
| if (!isLaunchShortcutAction(action)) { | ||
| console.warn("Ignoring unknown launch shortcut action in config:", action); | ||
| continue; | ||
| } | ||
|
|
||
| const accelerator = toElectronAccelerator(binding); | ||
| if (!accelerator) { | ||
| continue; | ||
| } | ||
|
|
||
| const registered = globalShortcut.register(accelerator, () => { |
Contributor
There was a problem hiding this comment.
Validate each shortcut binding before converting it.
Line 240 calls toElectronAccelerator(binding) without first proving that binding is an object with a string key. Because Lines 205-213 persist arbitrary JSON, one corrupt entry can throw on binding.key, hit the catch at Line 259, and prevent every remaining valid launch shortcut from registering.
Suggested hardening
for (const [action, binding] of Object.entries(
config as Record<string, ShortcutBinding>,
)) {
if (!isLaunchShortcutAction(action)) {
console.warn("Ignoring unknown launch shortcut action in config:", action);
continue;
}
+
+ if (
+ !binding ||
+ typeof binding !== "object" ||
+ typeof (binding as { key?: unknown }).key !== "string"
+ ) {
+ console.warn("Ignoring malformed launch shortcut binding:", { action, binding });
+ continue;
+ }
const accelerator = toElectronAccelerator(binding);
if (!accelerator) {
continue;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/register/settings.ts` around lines 223 - 245, The loop over
config entries calls toElectronAccelerator(binding) without validating that
binding is an object with the expected shape, so a malformed persisted entry can
throw and abort registering all shortcuts; before calling toElectronAccelerator
(inside the for...of that uses isLaunchShortcutAction), check that binding is an
object and binding.key is a non-empty string (and any other expected fields),
log/console.warn and continue for invalid entries, and/or wrap the per-binding
conversion/registration (the call to toElectronAccelerator and
globalShortcut.register) in its own try/catch so one bad entry doesn't prevent
others from registering.
Comment on lines
+246
to
+271
| function sendTrayRecordingCommand(command: "start" | "pause" | "resume" | "stop") { | ||
| let targetWindow = getHudOverlayWindow() ?? mainWindow; | ||
| if (!targetWindow || targetWindow.isDestroyed()) { | ||
| createWindow(); | ||
| targetWindow = getHudOverlayWindow() ?? mainWindow; | ||
| } | ||
|
|
||
| if (!targetWindow || targetWindow.isDestroyed()) { | ||
| return; | ||
| } | ||
|
|
||
| const sendCommand = () => { | ||
| if (!targetWindow || targetWindow.isDestroyed()) { | ||
| return; | ||
| } | ||
|
|
||
| targetWindow.webContents.send("tray-recording-command", command); | ||
| }; | ||
|
|
||
| if (targetWindow.webContents.isLoadingMainFrame()) { | ||
| targetWindow.webContents.once("did-finish-load", sendCommand); | ||
| return; | ||
| } | ||
|
|
||
| sendCommand(); | ||
| } |
Contributor
There was a problem hiding this comment.
Route tray recording commands to the HUD only.
This helper falls back to mainWindow, but createEditorWindowWrapper() rebinds mainWindow to the editor at Lines 850-852. If the HUD overlay is absent, tray Start/Pause/Resume/Stop can be sent to the editor window instead of the launch window that owns these handlers, so the action is dropped.
Suggested fix
function sendTrayRecordingCommand(command: "start" | "pause" | "resume" | "stop") {
- let targetWindow = getHudOverlayWindow() ?? mainWindow;
+ let targetWindow = getHudOverlayWindow();
if (!targetWindow || targetWindow.isDestroyed()) {
createWindow();
- targetWindow = getHudOverlayWindow() ?? mainWindow;
+ targetWindow = getHudOverlayWindow();
}
if (!targetWindow || targetWindow.isDestroyed()) {
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function sendTrayRecordingCommand(command: "start" | "pause" | "resume" | "stop") { | |
| let targetWindow = getHudOverlayWindow() ?? mainWindow; | |
| if (!targetWindow || targetWindow.isDestroyed()) { | |
| createWindow(); | |
| targetWindow = getHudOverlayWindow() ?? mainWindow; | |
| } | |
| if (!targetWindow || targetWindow.isDestroyed()) { | |
| return; | |
| } | |
| const sendCommand = () => { | |
| if (!targetWindow || targetWindow.isDestroyed()) { | |
| return; | |
| } | |
| targetWindow.webContents.send("tray-recording-command", command); | |
| }; | |
| if (targetWindow.webContents.isLoadingMainFrame()) { | |
| targetWindow.webContents.once("did-finish-load", sendCommand); | |
| return; | |
| } | |
| sendCommand(); | |
| } | |
| function sendTrayRecordingCommand(command: "start" | "pause" | "resume" | "stop") { | |
| let targetWindow = getHudOverlayWindow(); | |
| if (!targetWindow || targetWindow.isDestroyed()) { | |
| createWindow(); | |
| targetWindow = getHudOverlayWindow(); | |
| } | |
| if (!targetWindow || targetWindow.isDestroyed()) { | |
| return; | |
| } | |
| const sendCommand = () => { | |
| if (!targetWindow || targetWindow.isDestroyed()) { | |
| return; | |
| } | |
| targetWindow.webContents.send("tray-recording-command", command); | |
| }; | |
| if (targetWindow.webContents.isLoadingMainFrame()) { | |
| targetWindow.webContents.once("did-finish-load", sendCommand); | |
| return; | |
| } | |
| sendCommand(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/main.ts` around lines 246 - 271, The tray commands must always go to
the HUD overlay window rather than falling back to mainWindow; update
sendTrayRecordingCommand to stop using mainWindow as a fallback and instead
ensure a HUD overlay is created (call createWindow() or the HUD-specific
creator) and re-query getHudOverlayWindow()—only send "tray-recording-command"
when the resolved target is the HUD overlay (and is not destroyed); if the HUD
still doesn't exist after creation, return without sending. Also remove reliance
on mainWindow (and be mindful of createEditorWindowWrapper re-binding
mainWindow) so the command is never routed to the editor.
Comment on lines
+62
to
+65
| <ShortcutsProvider> | ||
| <LaunchWindow /> | ||
| <Toaster className="pointer-events-auto" /> | ||
| </> | ||
| </ShortcutsProvider> |
Contributor
There was a problem hiding this comment.
Avoid a second renderer owning app-wide shortcut registration.
ShortcutsProvider now registers/unregisters launch global shortcuts, and the editor path already mounts one provider. Adding another provider here means two windows can race on the same app-wide registration, so closing either window can unregister shortcuts for the other.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.tsx` around lines 62 - 65, The app currently mounts ShortcutsProvider
around LaunchWindow (wrapping <LaunchWindow /> and <Toaster />) which duplicates
the provider already mounted by the editor path and causes racey app-wide
shortcut registration; remove the ShortcutsProvider wrapper here so only the
editor-mounted provider owns global shortcuts (i.e., render <LaunchWindow /> and
<Toaster /> directly), or alternatively hoist a single ShortcutsProvider to the
true application root so ShortcutsProvider is instantiated exactly once; refer
to the ShortcutsProvider and LaunchWindow components to locate and change the
wrapper.
Comment on lines
+84
to
+111
| useEffect(() => { | ||
| const onKeyDown = (event: KeyboardEvent) => { | ||
| if (event.repeat) { | ||
| return; | ||
| } | ||
|
|
||
| for (const action of LAUNCH_SHORTCUT_ACTIONS) { | ||
| if (!matchesShortcut(event, launchShortcuts[action], isMac)) { | ||
| continue; | ||
| } | ||
|
|
||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| runLaunchShortcut(action); | ||
| break; | ||
| } | ||
| }; | ||
|
|
||
| window.addEventListener("keydown", onKeyDown); | ||
| return () => window.removeEventListener("keydown", onKeyDown); | ||
| }, [isMac, launchShortcuts, runLaunchShortcut]); | ||
|
|
||
| useEffect(() => { | ||
| const unsubscribe = window.electronAPI?.onLaunchShortcutTriggered?.((action) => { | ||
| runLaunchShortcut(action); | ||
| }); | ||
| return () => unsubscribe?.(); | ||
| }, [runLaunchShortcut]); |
Contributor
There was a problem hiding this comment.
Avoid dispatching the same launch shortcut from both handlers on macOS.
These two effects can fire for the same key press when the launch window is focused: the DOM keydown path and the Electron global-shortcut callback. On macOS that turns toggle-style actions into double execution, so startRecording/stopRecording and muteMicrophone can cancel themselves out.
Suggested fix
useEffect(() => {
+ if (isMac) {
+ return;
+ }
+
const onKeyDown = (event: KeyboardEvent) => {
if (event.repeat) {
return;
}
@@
window.addEventListener("keydown", onKeyDown);
return () => window.removeEventListener("keydown", onKeyDown);
}, [isMac, launchShortcuts, runLaunchShortcut]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| const onKeyDown = (event: KeyboardEvent) => { | |
| if (event.repeat) { | |
| return; | |
| } | |
| for (const action of LAUNCH_SHORTCUT_ACTIONS) { | |
| if (!matchesShortcut(event, launchShortcuts[action], isMac)) { | |
| continue; | |
| } | |
| event.preventDefault(); | |
| event.stopPropagation(); | |
| runLaunchShortcut(action); | |
| break; | |
| } | |
| }; | |
| window.addEventListener("keydown", onKeyDown); | |
| return () => window.removeEventListener("keydown", onKeyDown); | |
| }, [isMac, launchShortcuts, runLaunchShortcut]); | |
| useEffect(() => { | |
| const unsubscribe = window.electronAPI?.onLaunchShortcutTriggered?.((action) => { | |
| runLaunchShortcut(action); | |
| }); | |
| return () => unsubscribe?.(); | |
| }, [runLaunchShortcut]); | |
| useEffect(() => { | |
| if (isMac) { | |
| return; | |
| } | |
| const onKeyDown = (event: KeyboardEvent) => { | |
| if (event.repeat) { | |
| return; | |
| } | |
| for (const action of LAUNCH_SHORTCUT_ACTIONS) { | |
| if (!matchesShortcut(event, launchShortcuts[action], isMac)) { | |
| continue; | |
| } | |
| event.preventDefault(); | |
| event.stopPropagation(); | |
| runLaunchShortcut(action); | |
| break; | |
| } | |
| }; | |
| window.addEventListener("keydown", onKeyDown); | |
| return () => window.removeEventListener("keydown", onKeyDown); | |
| }, [isMac, launchShortcuts, runLaunchShortcut]); | |
| useEffect(() => { | |
| const unsubscribe = window.electronAPI?.onLaunchShortcutTriggered?.((action) => { | |
| runLaunchShortcut(action); | |
| }); | |
| return () => unsubscribe?.(); | |
| }, [runLaunchShortcut]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/launch/hooks/useLaunchShortcuts.ts` around lines 84 - 111, On
macOS the DOM keydown handler and the Electron global-shortcut subscription can
both fire for the same keypress causing double execution; fix this by tracking
recent Electron-triggered launches and ignoring the DOM handler when an Electron
trigger was just received: add a ref like lastElectronTriggerRef used in the
effect that subscribes to window.electronAPI.onLaunchShortcutTriggered to set
lastElectronTriggerRef.current = Date.now() when it calls
runLaunchShortcut(action), and in the onKeyDown handler (inside the useEffect
that iterates LAUNCH_SHORTCUT_ACTIONS and calls matchesShortcut with
launchShortcuts and isMac) early-return when isMac && Date.now() -
(lastElectronTriggerRef.current || 0) < 100 (or similar small debounce window)
before calling runLaunchShortcut; ensure the ref is declared in the hook scope
so both effects can access it.
Comment on lines
177
to
+183
| const handleSave = useCallback(async () => { | ||
| setShortcuts(draft); | ||
| await persistShortcuts(draft); | ||
| setLaunchShortcuts(launchDraft); | ||
| await persistShortcuts(draft, launchDraft); | ||
| toast.success(t("shortcutsConfig.saved")); | ||
| closeConfig(); | ||
| }, [draft, setShortcuts, persistShortcuts, closeConfig, t]); | ||
| }, [draft, launchDraft, setShortcuts, setLaunchShortcuts, persistShortcuts, closeConfig, t]); |
Contributor
There was a problem hiding this comment.
Persist before mutating live shortcut state.
These setters run before persistShortcuts resolves. If the save fails, the dialog still updates live editor/global bindings, and ShortcutsContext can re-register unsaved launch shortcuts until restart.
Suggested fix
const handleSave = useCallback(async () => {
- setShortcuts(draft);
- setLaunchShortcuts(launchDraft);
- await persistShortcuts(draft, launchDraft);
- toast.success(t("shortcutsConfig.saved"));
- closeConfig();
+ try {
+ await persistShortcuts(draft, launchDraft);
+ setShortcuts(draft);
+ setLaunchShortcuts(launchDraft);
+ toast.success(t("shortcutsConfig.saved"));
+ closeConfig();
+ } catch {
+ toast.error(t("shortcutsConfig.saveFailed", "Failed to save shortcuts"));
+ }
}, [draft, launchDraft, setShortcuts, setLaunchShortcuts, persistShortcuts, closeConfig, t]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleSave = useCallback(async () => { | |
| setShortcuts(draft); | |
| await persistShortcuts(draft); | |
| setLaunchShortcuts(launchDraft); | |
| await persistShortcuts(draft, launchDraft); | |
| toast.success(t("shortcutsConfig.saved")); | |
| closeConfig(); | |
| }, [draft, setShortcuts, persistShortcuts, closeConfig, t]); | |
| }, [draft, launchDraft, setShortcuts, setLaunchShortcuts, persistShortcuts, closeConfig, t]); | |
| const handleSave = useCallback(async () => { | |
| try { | |
| await persistShortcuts(draft, launchDraft); | |
| setShortcuts(draft); | |
| setLaunchShortcuts(launchDraft); | |
| toast.success(t("shortcutsConfig.saved")); | |
| closeConfig(); | |
| } catch { | |
| toast.error(t("shortcutsConfig.saveFailed", "Failed to save shortcuts")); | |
| } | |
| }, [draft, launchDraft, setShortcuts, setLaunchShortcuts, persistShortcuts, closeConfig, t]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/video-editor/ShortcutsConfigDialog.tsx` around lines 177 -
183, In handleSave, call persistShortcuts(draft, launchDraft) and await its
resolution before calling setShortcuts and setLaunchShortcuts so live state
isn't mutated on a failed save; wrap the await in try/catch, on success call
setShortcuts(draft), setLaunchShortcuts(launchDraft),
toast.success(t("shortcutsConfig.saved")) and closeConfig(), and on error show
toast.error with the error message and do not update live bindings or close the
dialog; reference handleSave, persistShortcuts, setShortcuts,
setLaunchShortcuts, draft, launchDraft, toast, and closeConfig when making the
change.
Comment on lines
+48
to
+50
| "localShortcuts": "Local shortcuts", | ||
| "globalShortcuts": "Global recording shortcuts", | ||
| "globalDescription": "Available on macOS even when Recordly is in the background.", |
Contributor
There was a problem hiding this comment.
Translate the new pt-BR shortcut labels instead of shipping English fallback text.
These three entries are user-visible in the Brazilian Portuguese locale, but the new values are still English. That will leak mixed-language UI in the shortcuts dialog.
Suggested fix
- "localShortcuts": "Local shortcuts",
- "globalShortcuts": "Global recording shortcuts",
- "globalDescription": "Available on macOS even when Recordly is in the background.",
+ "localShortcuts": "Atalhos locais",
+ "globalShortcuts": "Atalhos globais de gravação",
+ "globalDescription": "Disponíveis no macOS mesmo quando o Recordly estiver em segundo plano.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "localShortcuts": "Local shortcuts", | |
| "globalShortcuts": "Global recording shortcuts", | |
| "globalDescription": "Available on macOS even when Recordly is in the background.", | |
| "localShortcuts": "Atalhos locais", | |
| "globalShortcuts": "Atalhos globais de gravação", | |
| "globalDescription": "Disponíveis no macOS mesmo quando o Recordly estiver em segundo plano.", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/pt-BR/dialogs.json` around lines 48 - 50, Translate the
three English shortcut label entries in the pt-BR locale by replacing the values
for "localShortcuts", "globalShortcuts", and "globalDescription" in dialogs.json
with Brazilian Portuguese equivalents (e.g., "Atalhos locais", "Atalhos globais
de gravação", and a Portuguese version of the description such as "Disponível no
macOS mesmo quando o Recordly está em segundo plano."). Ensure the keys remain
unchanged and only the string values are updated so the UI shows consistent
Portuguese text.
| "countdownDelay": "Atraso da contagem regressiva", | ||
| "noDelay": "Sem atraso", | ||
| "record": "Gravar", | ||
| "startRecording": "Iniciar gravacao", |
Contributor
There was a problem hiding this comment.
Fix the pt-BR spelling for startRecording.
"Iniciar gravacao" is missing the cedilla/tilde. In pt-BR this should be "Iniciar gravação".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/pt-BR/launch.json` at line 13, Update the pt-BR translation
value for the key "startRecording" in the launch.json locale file: replace the
current string "Iniciar gravacao" with the correctly accented Portuguese
"Iniciar gravação" so the startRecording entry uses the proper spelling.
Comment on lines
+48
to
+50
| "localShortcuts": "Local shortcuts", | ||
| "globalShortcuts": "Global recording shortcuts", | ||
| "globalDescription": "Available on macOS even when Recordly is in the background.", |
Contributor
There was a problem hiding this comment.
Translate the new shortcut strings in this locale.
These entries are still English, so the shortcuts dialog will render mixed-language copy for Russian users.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/ru/dialogs.json` around lines 48 - 50, Translate the three
English strings in the Russian locale by replacing the values for the JSON keys
"localShortcuts", "globalShortcuts", and "globalDescription" with their Russian
equivalents so the shortcuts dialog is fully localized; update "localShortcuts"
to a Russian phrase for "Local shortcuts", "globalShortcuts" to Russian for
"Global recording shortcuts", and "globalDescription" to Russian describing that
these are available on macOS even when the app is in the background.
Comment on lines
+48
to
+50
| "localShortcuts": "Local shortcuts", | ||
| "globalShortcuts": "Global recording shortcuts", | ||
| "globalDescription": "Available on macOS even when Recordly is in the background.", |
Contributor
There was a problem hiding this comment.
Translate the new shortcut strings in this locale.
These values are still English, so Simplified Chinese users will see a mixed-language shortcuts dialog.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/zh-CN/dialogs.json` around lines 48 - 50, The three English
strings for the shortcut dialog (keys localShortcuts, globalShortcuts,
globalDescription) are untranslated; replace their values with Simplified
Chinese equivalents: set localShortcuts to "本地快捷键", globalShortcuts to
"全局录制快捷键", and globalDescription to "在 macOS 上可用，即使 Recordly 处于后台运行。".
Comment on lines
+48
to
+50
| "localShortcuts": "Local shortcuts", | ||
| "globalShortcuts": "Global recording shortcuts", | ||
| "globalDescription": "Available on macOS even when Recordly is in the background.", |
Contributor
There was a problem hiding this comment.
Translate the new shortcut strings in this locale.
These values are still English, so Traditional Chinese users will see a mixed-language shortcuts dialog.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/i18n/locales/zh-TW/dialogs.json` around lines 48 - 50, Replace the
English values for the keys localShortcuts, globalShortcuts, and
globalDescription in the zh-TW dialogs JSON with proper Traditional Chinese
translations: update "localShortcuts" to a Traditional Chinese string for "Local
shortcuts", "globalShortcuts" to one for "Global recording shortcuts", and
"globalDescription" to a Chinese sentence conveying "Available on macOS even
when Recordly is in the background."; keep the keys unchanged (localShortcuts,
globalShortcuts, globalDescription) and ensure the JSON string quoting and
punctuation remain valid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch builds on the launch/global shortcut work already started in
feature/shortcutand finishes the user-facing integration.It adds configurable launch shortcuts for recording controls, wires those controls through the Electron tray and recording state updates, and surfaces the active shortcut bindings directly in the launch window's
Moremenu.What changed
Launch/global shortcut foundation
LaunchShortcutsConfigalongside editor shortcutsShortcutsContexton macOSTray and recording-state integration
Launch More menu improvements
MoremenuTesting
npx vitest run src/components/launch/popovers/MorePopover.test.tsx src/lib/shortcuts.test.ts src/components/launch/hooks/useLaunchWindowActions.test.tsnpx tsc --noEmitEOF
Summary by CodeRabbit
Release Notes
New Features
Improvements