[shortcut] Add shortcut to focus textbox input #209
[shortcut] Add shortcut to focus textbox input #209kyan-yang wants to merge 1 commit intosohzm:masterfrom
Conversation
📝 WalkthroughWalkthroughA new keyboard shortcut feature is implemented across three components: the main process registers a global Ctrl+L/Cmd+L shortcut that shows and focuses the window, sends an IPC 'focus-input' event to the renderer, which the AssistantView listens for to focus the text input element. Configuration UI is added to CustomizeView. Changes
Sequence DiagramsequenceDiagram
actor User
participant Main as Main Process<br/>(window.js)
participant IPC as IPC Layer
participant Renderer as Renderer Process<br/>(AssistantView.js)
participant DOM as DOM<br/>(textInput)
User->>Main: Press Ctrl+L / Cmd+L
Main->>Main: Global shortcut handler triggered
Main->>Main: Show/Focus main window
Main->>IPC: Send 'focus-input' message
IPC->>Renderer: Receive 'focus-input' event
Renderer->>Renderer: handleFocusInput() called
Renderer->>DOM: Focus `#textInput` element
DOM->>User: Text input receives focus
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/window.js`:
- Around line 274-291: The focusInput shortcut callback uses mainWindow without
guarding against a destroyed window and can throw; update the
globalShortcut.register handler for keybinds.focusInput to first check that
mainWindow exists and is not destroyed (e.g., if (!mainWindow ||
mainWindow.isDestroyed()) return) before calling mainWindow.isVisible(),
.show(), .focus(), .webContents.focus(), or .webContents.send('focus-input');
optionally also ensure you unregister the shortcut or handle errors if the
window is gone.
| // Register focus input shortcut | ||
| if (keybinds.focusInput) { | ||
| try { | ||
| globalShortcut.register(keybinds.focusInput, () => { | ||
| console.log('Focus input shortcut triggered'); | ||
| if (!mainWindow.isVisible()) { | ||
| mainWindow.show(); | ||
| } else { | ||
| mainWindow.focus(); | ||
| } | ||
| mainWindow.webContents.focus(); | ||
| mainWindow.webContents.send('focus-input'); | ||
| }); | ||
| console.log(`Registered focusInput: ${keybinds.focusInput}`); | ||
| } catch (error) { | ||
| console.error(`Failed to register focusInput (${keybinds.focusInput}):`, error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing isDestroyed() guard before accessing mainWindow.
The emergencyErase handler (line 298) checks mainWindow && !mainWindow.isDestroyed() before interacting with the window. This handler does not, which can throw if the window has been destroyed while shortcuts remain registered.
🛡️ Proposed fix
if (keybinds.focusInput) {
try {
globalShortcut.register(keybinds.focusInput, () => {
console.log('Focus input shortcut triggered');
+ if (!mainWindow || mainWindow.isDestroyed()) return;
if (!mainWindow.isVisible()) {
mainWindow.show();
} else {
mainWindow.focus();
}
mainWindow.webContents.focus();
mainWindow.webContents.send('focus-input');
});🤖 Prompt for AI Agents
In `@src/utils/window.js` around lines 274 - 291, The focusInput shortcut callback
uses mainWindow without guarding against a destroyed window and can throw;
update the globalShortcut.register handler for keybinds.focusInput to first
check that mainWindow exists and is not destroyed (e.g., if (!mainWindow ||
mainWindow.isDestroyed()) return) before calling mainWindow.isVisible(),
.show(), .focus(), .webContents.focus(), or .webContents.send('focus-input');
optionally also ensure you unregister the shortcut or handle errors if the
window is gone.
There was a problem hiding this comment.
intended; if window is closed and you focus textbox, it is good that it auto opens and focuses. much faster
There was a problem hiding this comment.
@kyan-yang Got it! The auto-open behavior when the window is hidden works great with the current isVisible() check—that's already handling your use case.
My concern was specifically about the destroyed case: if mainWindow is destroyed (not just hidden), calling mainWindow.isVisible() would throw an error before the check executes. The isDestroyed() guard prevents that crash. It doesn't change your intended behavior—hidden windows would still auto-open—it just adds safety for the edge case where the window object is gone entirely.
That said, if you're confident this scenario won't occur in practice, feel free to keep it as-is!
✏️ Learnings added
Learnt from: kyan-yang
Repo: sohzm/cheating-daddy PR: 209
File: src/utils/window.js:274-291
Timestamp: 2026-02-09T14:57:59.175Z
Learning: In src/utils/window.js, the focusInput shortcut handler intentionally auto-opens the window when it's hidden (using isVisible() check) to provide faster access, as confirmed by kyan-yang.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
oh wait i confused the two states of visible and destroyed.
if window is destroyed though, app is like shut off and done so no need to check for that; just adds bloat.
There was a problem hiding this comment.
@kyan-yang Fair enough! You're right that in typical app lifecycle scenarios, if the window is destroyed, the app is shutting down and shortcuts won't be triggered anyway. I'll defer to your judgment on keeping it lean. 👍
🧠 Learnings used
Learnt from: kyan-yang
Repo: sohzm/cheating-daddy PR: 209
File: src/utils/window.js:274-291
Timestamp: 2026-02-09T14:57:59.175Z
Learning: In src/utils/window.js, the focusInput shortcut handler intentionally auto-opens the window when it's hidden (using isVisible() check) to provide faster access, as confirmed by kyan-yang.
1 participant
Add keyboard shortcut to focus textbox input
Default is Cmd+L
Summary by CodeRabbit