add: shortcuts with global shortcuts editable, with a dialog.#457
add: shortcuts with global shortcuts editable, with a dialog.#457ExtraBinoss wants to merge 10 commits intowebadderallorg:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds launch (recording-control) keyboard shortcut support: types and preload API, main-process global-accelerator registration, split shortcuts library (editor vs launch), ShortcutsPopover UI and context persistence, runtime hooks wiring (DOM + Electron events), and localized strings in eight languages. ChangesLaunch Keyboard Shortcuts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
electron/ipc/register/settings.ts (1)
119-141: 💤 Low valueSilent failure on partial shortcut registration.
When
globalShortcut.register()returnsfalse(shortcut already taken by another app), the code continues without logging or reporting. Users won't know why a shortcut isn't working.Consider logging failed registrations or accumulating them in the response:
Optional: Track and report failed registrations
ipcMain.handle("register-launch-global-shortcuts", async (_, config: unknown) => { try { unregisterLaunchGlobalShortcuts(); if (!config || typeof config !== "object") { return { success: true }; } + const failed: string[] = []; const entries = Object.entries(config as Record<string, ShortcutBinding>); for (const [action, binding] of entries) { const accelerator = toElectronAccelerator(binding); if (!accelerator) continue; const registered = globalShortcut.register(accelerator, () => { notifyLaunchShortcutTriggered(action as LaunchShortcutAction); }); if (registered) { launchShortcutRegisteredAccelerators.push(accelerator); + } else { + console.warn(`Failed to register global shortcut: ${accelerator} for ${action}`); + failed.push(action); } } - return { success: true }; + return { success: true, failedActions: failed.length ? failed : undefined }; } catch (error) { return { success: false, error: String(error) }; } });🤖 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 119 - 141, In the ipcMain handler for "register-launch-global-shortcuts" update the loop so failed globalShortcut.register(accelerator) cases are captured: when register returns false, push the accelerator (and its action) into a new failedRegistrations list and log a descriptive message (use processLogger or similar) referencing the accelerator and action; after the loop return { success: true, failed: failedRegistrations } (or include an error message if all failed) so callers and users see which accelerators failed; keep existing calls to unregisterLaunchGlobalShortcuts(), toElectronAccelerator(), launchShortcutRegisteredAccelerators, and notifyLaunchShortcutTriggered unchanged except for adding the failure tracking and logging.
🤖 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 51-62: The toElectronAccelerator function currently capitalizes
keys generically, which mis-maps arrow keys (e.g. "arrowup" -> "Arrowup") and
causes silent registration failures; update toElectronAccelerator to first
normalize binding.key then consult a small explicit mapping (e.g.,
"arrowup"->"Up", "arrowdown"->"Down", "arrowleft"->"Left", "arrowright"->Right",
and any other special names you need like
"enter","tab","escape","backspace","space") and use the mapped value when
present, falling back to the existing single-char/Title-case logic; keep
existing modifier handling (binding.ctrl, shift, alt) and return parts.join("+")
as before.
In `@src/components/launch/hooks/useLaunchHudInteractionState.ts`:
- Around line 20-24: The effect exits early when hasModalOpen is true but never
schedules the delayed cleanup when hasModalOpen transitions back to false, so
modify the effect around hasModalOpen/openId (referencing hasModalOpen,
anyPopoverOpenRef.current, openId and
window.electronAPI?.hudOverlaySetIgnoreMouse) to ensure the same delayed cleanup
path runs when hasModalOpen becomes false and openId is null: specifically, when
hasModalOpen is true keep the early-return behavior but also set up or delegate
to the same setTimeout cleanup used when openId becomes null (clearing any
existing timeout, setting anyPopoverOpenRef.current = false, and calling
hudOverlaySetIgnoreMouse(true) after the delay), and ensure you clear the
timeout in the effect cleanup to avoid leaks.
In `@src/components/launch/hooks/useLaunchShortcuts.ts`:
- Around line 9-21: The shortcut "muteMicrophone" is being matched in
runLaunchShortcut but never invokes any handler; add a mute handler to the hook
params (e.g., muteMicrophone: () => void) in the UseLaunchShortcutsParams
interface and wire it into runLaunchShortcut so that when action ===
"muteMicrophone" the new muteMicrophone() is called (and awaited if it returns a
Promise); also update any call sites that construct UseLaunchShortcutsParams to
provide the handler and mirror the same wiring for the other missing actions
referenced (the same pattern around the runLaunchShortcut switch/if blocks and
parameter list).
- Around line 100-110: The effect currently unregisters all global shortcuts
when not recording, so launchShortcuts.startRecording never gets a global
binding; change the logic in useEffect to always register the startRecording
shortcut (use window.electronAPI.registerLaunchGlobalShortcuts and include
launchShortcuts.startRecording in the payload) even when recording is false, and
only register the other recording-related shortcuts (stopRecording,
pauseRecording, resumeRecording, muteMicrophone) when recording is true; keep
calling unregisterLaunchGlobalShortcuts on cleanup as before. Use the existing
symbols registerLaunchGlobalShortcuts, unregisterLaunchGlobalShortcuts, and
launchShortcuts.startRecording to locate and update the code.
In `@src/components/launch/popovers/ShortcutsPopover.tsx`:
- Around line 38-43: The effect that initializes the popover currently
early-returns when open is false, leaving capture state and global keydown
interception active; update the useEffect associated with open and
launchShortcuts so that when open is false it explicitly resets transient state
(call setDraft(null or default), setCaptureFor(null), setConflictAction(null))
and removes any global keydown interceptor created during capture mode; ensure
the cleanup path of the effect also unregisters the keydown handler so
preventDefault cannot block input after closing (refer to the useEffect, open,
setDraft, setCaptureFor, setConflictAction and the keydown handler registration
code).
---
Nitpick comments:
In `@electron/ipc/register/settings.ts`:
- Around line 119-141: In the ipcMain handler for
"register-launch-global-shortcuts" update the loop so failed
globalShortcut.register(accelerator) cases are captured: when register returns
false, push the accelerator (and its action) into a new failedRegistrations list
and log a descriptive message (use processLogger or similar) referencing the
accelerator and action; after the loop return { success: true, failed:
failedRegistrations } (or include an error message if all failed) so callers and
users see which accelerators failed; keep existing calls to
unregisterLaunchGlobalShortcuts(), toElectronAccelerator(),
launchShortcutRegisteredAccelerators, and notifyLaunchShortcutTriggered
unchanged except for adding the failure tracking and logging.
🪄 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: b4c0b105-349d-426f-8e5f-e130a1ea7952
📒 Files selected for processing (25)
electron/electron-env.d.tselectron/ipc/register/settings.tselectron/ipc/shortcutTypes.tselectron/preload.tssrc/App.tsxsrc/components/launch/LaunchWindow.module.csssrc/components/launch/LaunchWindow.tsxsrc/components/launch/hooks/useLaunchHudInteractionState.tssrc/components/launch/hooks/useLaunchShortcuts.tssrc/components/launch/popovers/MorePopover.tsxsrc/components/launch/popovers/PopoverScaffold.tsxsrc/components/launch/popovers/ShortcutsPopover.tsxsrc/components/video-editor/KeyboardShortcutsHelp.tsxsrc/components/video-editor/ShortcutsConfigDialog.tsxsrc/components/video-editor/TutorialHelp.tsxsrc/contexts/ShortcutsContext.tsxsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/fr/launch.jsonsrc/i18n/locales/ko/launch.jsonsrc/i18n/locales/nl/launch.jsonsrc/i18n/locales/pt-BR/launch.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/i18n/locales/zh-TW/launch.jsonsrc/lib/shortcuts.ts
There was a problem hiding this comment.
Pull request overview
Adds an editable “Launch shortcuts” UI for the HUD/Launch window and persists these bindings, including Electron main-process support for registering global shortcuts during recording.
Changes:
- Split shortcut infrastructure into editor shortcuts vs launch shortcuts, with defaults and backward-compatible persisted payload resolution.
- Add a Launch-window shortcuts popover (accessible from “More”) with i18n strings across supported locales.
- Add HUD-side launch shortcut handling plus Electron IPC to register/unregister global shortcuts and notify the HUD when triggered.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/shortcuts.ts | Introduces launch shortcut action types/defaults and persistence resolution helpers. |
| src/contexts/ShortcutsContext.tsx | Extends shortcuts context to manage/persist both editor + launch shortcut configs. |
| src/components/video-editor/TutorialHelp.tsx | Updates editor shortcut listing to use renamed editor action constant. |
| src/components/video-editor/ShortcutsConfigDialog.tsx | Updates types/constants for editor shortcut configuration dialog. |
| src/components/video-editor/KeyboardShortcutsHelp.tsx | Updates editor shortcut help listing to use renamed editor action constant. |
| src/components/launch/popovers/ShortcutsPopover.tsx | New popover UI to edit launch shortcuts with conflict detection and persistence. |
| src/components/launch/popovers/PopoverScaffold.tsx | Adds contentClassName support for popover content styling. |
| src/components/launch/popovers/MorePopover.tsx | Adds menu entry to open the new shortcuts popover. |
| src/components/launch/LaunchWindow.tsx | Wires in shortcuts context + useLaunchShortcuts and mounts the shortcuts popover. |
| src/components/launch/LaunchWindow.module.css | Adds styling for the shortcuts popover width. |
| src/components/launch/hooks/useLaunchShortcuts.ts | New hook to handle launch shortcuts + global shortcut registration lifecycle. |
| src/components/launch/hooks/useLaunchHudInteractionState.ts | Extends HUD interaction logic with hasModalOpen flag. |
| src/App.tsx | Wraps HUD overlay window with ShortcutsProvider so launch shortcuts are available. |
| src/i18n/locales/en/launch.json | Adds launch shortcuts strings. |
| src/i18n/locales/es/launch.json | Adds launch shortcuts strings. |
| src/i18n/locales/fr/launch.json | Adds launch shortcuts strings. |
| src/i18n/locales/ko/launch.json | Adds launch shortcuts strings. |
| src/i18n/locales/nl/launch.json | Adds launch shortcuts strings. |
| src/i18n/locales/pt-BR/launch.json | Adds launch shortcuts strings. |
| src/i18n/locales/zh-CN/launch.json | Adds launch shortcuts strings. |
| src/i18n/locales/zh-TW/launch.json | Adds launch shortcuts strings. |
| electron/preload.ts | Exposes IPC APIs for registering/unregistering launch global shortcuts and receiving triggers. |
| electron/ipc/shortcutTypes.ts | New shared Electron-side types for launch shortcut actions/bindings. |
| electron/ipc/register/settings.ts | Implements IPC handlers and globalShortcut registration + trigger forwarding to HUD. |
| electron/electron-env.d.ts | Adds renderer typing for the new Electron APIs and trigger callback payload. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/launch/hooks/useLaunchShortcuts.ts`:
- Around line 83-90: The onKeyDown handler fires runLaunchShortcut repeatedly
when a shortcut key is held; update the onKeyDown logic in useLaunchShortcuts
(the function containing onKeyDown and loop over LAUNCH_SHORTCUT_ACTIONS) to
ignore auto-repeat events by checking the KeyboardEvent.repeat flag (e.repeat)
before matching shortcuts—i.e., if e.repeat is true, do not call
runLaunchShortcut (or early return/continue) so actions like toggleRecording()
are dispatched only once per physical key press; keep matchesShortcut,
launchShortcuts, isMac checks intact and only add the repeat guard around the
existing matching/dispatch flow.
🪄 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: 3bb926a2-55e5-4d96-a759-b41d2f189157
📒 Files selected for processing (6)
electron/ipc/register/settings.tselectron/ipc/shortcutTypes.tssrc/components/launch/LaunchWindow.tsxsrc/components/launch/hooks/useLaunchHudInteractionState.tssrc/components/launch/hooks/useLaunchShortcuts.tssrc/components/launch/popovers/ShortcutsPopover.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/launch/hooks/useLaunchHudInteractionState.ts
- electron/ipc/register/settings.ts
- src/components/launch/LaunchWindow.tsx
- src/components/launch/popovers/ShortcutsPopover.tsx
meiiie
left a comment
There was a problem hiding this comment.
Found two focused follow-ups while triaging this PR. The TypeScript failure should be fixed before owner handoff; the Space shortcut case is smaller but still in scope for editable launch shortcuts.
| }; | ||
|
|
||
| const toggleMicrophoneMute = useCallback(() => { | ||
| setMicrophoneEnabled((enabled) => !enabled); |
There was a problem hiding this comment.
setMicrophoneEnabled is exposed by useScreenRecorder as (enabled: boolean) => void, not a React state setter, so this updater function breaks tsc --noEmit (Argument of type '(enabled) => boolean' is not assignable...). Use the current microphoneEnabled value here, or expose a dedicated toggle, so the PR stays type-clean.
| }; | ||
|
|
||
| function toElectronAccelerator(binding: ShortcutBinding): string | null { | ||
| const key = binding.key?.trim().toLowerCase(); |
There was a problem hiding this comment.
Space shortcuts are captured and stored as " ", but this trims before the Space mapping, so Ctrl+Shift+Space becomes an empty key and never registers as an Electron global shortcut. Preserve the raw key for the Space check, or map Space before trimming.
So sorry to have not made TSC compilation, this was clearly a mistake on my part. I have fixed your found issue. |
Summary
Users had no way of adding shortcuts to the HUD/Launch window.
Here is my proposition in order to add it.
This will make people be able to use/edit shortcuts to their likings for doing stuff like :
Default controls are here. Saved even when restarting the app
Motivation
I wanted to make the Teleprompter suggestion from the discord, I like the idea of the feature, but we needed also keyboard shortcuts in order to make it very useful. so thats what it came from. And the issue #428.
Type of Change
Related Issue(s)
Add toggle button for start, pause, and end to record #428
Screenshots / Video
Cannot screenshot recordly :/
Testing Guide
Checklist
Summary by CodeRabbit
New Features
Bug Fixes / UX
Localization