Skip to content

Add custom keyboard shortcuts configuration#8

Open
jasonkneen wants to merge 1 commit intotkattkat:mainfrom
jasonkneen:claude/keyboard-shortcuts-01WDijWuk2DooQ8GUnK8N4D9
Open

Add custom keyboard shortcuts configuration#8
jasonkneen wants to merge 1 commit intotkattkat:mainfrom
jasonkneen:claude/keyboard-shortcuts-01WDijWuk2DooQ8GUnK8N4D9

Conversation

@jasonkneen
Copy link
Contributor

  • Add configurable keyboard shortcuts for:
    • Open spotlight (default: Cmd/Ctrl+Shift+C)
    • New conversation (default: Cmd/Ctrl+N)
    • Toggle sidebar (default: Cmd/Ctrl+B)
  • Update settings UI to show all configurable shortcuts
  • Register all shortcuts globally via Electron's globalShortcut
  • Add IPC handlers for new-conversation and toggle-sidebar events
  • Maintain backwards compatibility with existing spotlightKeybind setting

- Add configurable keyboard shortcuts for:
  - Open spotlight (default: Cmd/Ctrl+Shift+C)
  - New conversation (default: Cmd/Ctrl+N)
  - Toggle sidebar (default: Cmd/Ctrl+B)
- Update settings UI to show all configurable shortcuts
- Register all shortcuts globally via Electron's globalShortcut
- Add IPC handlers for new-conversation and toggle-sidebar events
- Maintain backwards compatibility with existing spotlightKeybind setting
Copilot AI review requested due to automatic review settings December 8, 2025 09:42
@jasonkneen jasonkneen requested a review from tkattkat as a code owner December 8, 2025 09:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds configurable keyboard shortcuts for three core application actions: opening spotlight search, starting a new conversation, and toggling the sidebar. It extends the existing settings system to manage these shortcuts through a unified interface while maintaining backwards compatibility with the legacy spotlightKeybind setting.

Key Changes:

  • Added a KeyboardShortcuts interface to define the three configurable shortcuts with sensible defaults (Cmd/Ctrl+Shift+C for spotlight, Cmd/Ctrl+N for new conversation, Cmd/Ctrl+B for toggle sidebar)
  • Updated the settings UI to display all three keyboard shortcuts in a dedicated "Keyboard Shortcuts" section
  • Implemented global shortcut registration for the new conversation and toggle sidebar actions via Electron's globalShortcut API

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
static/settings.html Reorganized settings UI to create a dedicated "Keyboard Shortcuts" section with three configurable shortcuts, moving spotlight-specific settings to their own section
src/types/index.ts Added KeyboardShortcuts interface and integrated it into the SettingsSchema
src/renderer/settings.ts Refactored keybind handling to support multiple shortcuts, implemented migration logic for legacy settings, and added display updates for all three shortcuts
src/renderer/main.ts Added IPC event listeners for new-conversation and toggle-sidebar shortcuts that trigger the appropriate UI actions
src/preload.ts Exposed new IPC event handlers for the two new shortcuts and updated the saveSettings type signature to include keyboardShortcuts
src/main.ts Refactored shortcut registration to handle all three shortcuts, added fallback to legacy spotlightKeybind, and implemented IPC event sending for new conversation and toggle sidebar actions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +52 to 54
globalShortcut.register(DEFAULT_KEYBOARD_SHORTCUTS.spotlight, () => {
createSpotlightWindow();
});
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the default spotlight shortcut registration also fails (which could happen if another application is using it), the application will throw an unhandled error. Consider wrapping this fallback in a try-catch as well:

try {
  globalShortcut.register(spotlightKey, () => {
    createSpotlightWindow();
  });
} catch (e) {
  console.error('Failed to register spotlight keybind:', spotlightKey, e);
  try {
    globalShortcut.register(DEFAULT_KEYBOARD_SHORTCUTS.spotlight, () => {
      createSpotlightWindow();
    });
  } catch (fallbackError) {
    console.error('Failed to register default spotlight keybind:', fallbackError);
  }
}
Suggested change
globalShortcut.register(DEFAULT_KEYBOARD_SHORTCUTS.spotlight, () => {
createSpotlightWindow();
});
try {
globalShortcut.register(DEFAULT_KEYBOARD_SHORTCUTS.spotlight, () => {
createSpotlightWindow();
});
} catch (fallbackError) {
console.error('Failed to register default spotlight keybind:', fallbackError);
}

Copilot uses AI. Check for mistakes.
isRecordingKeybind = true;
// Set up keybind input handlers
function setupKeybindInput(inputEl: HTMLElement) {
const shortcutKey = inputEl.dataset.shortcut as keyof KeyboardShortcuts;
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dataset.shortcut value is cast to keyof KeyboardShortcuts without validation. If the HTML contains an invalid data-shortcut value, this could cause runtime errors when trying to access properties on the shortcuts object.

Consider adding validation:

const shortcutKey = inputEl.dataset.shortcut as keyof KeyboardShortcuts;
if (!shortcutKey || !(shortcutKey in DEFAULT_SHORTCUTS)) {
  console.error('Invalid shortcut key:', shortcutKey);
  return;
}
Suggested change
const shortcutKey = inputEl.dataset.shortcut as keyof KeyboardShortcuts;
const shortcutKey = inputEl.dataset.shortcut as keyof KeyboardShortcuts;
if (!shortcutKey || !(shortcutKey in DEFAULT_SHORTCUTS)) {
console.error('Invalid shortcut key:', shortcutKey);
return;
}

Copilot uses AI. Check for mistakes.
if (!isRecordingKeybind) return;
if (!activeRecordingInput) return;

const shortcutKey = activeRecordingInput.dataset.shortcut as keyof KeyboardShortcuts;
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dataset.shortcut value is cast to keyof KeyboardShortcuts without validation. If the HTML contains an invalid data-shortcut value, this could result in undefined behavior when calling getShortcutValue(shortcutKey).

Consider adding validation similar to the suggestion for line 196, or refactor to share the validation logic.

Copilot uses AI. Check for mistakes.
Comment on lines 3 to +10
const claude = (window as any).claude;

interface KeyboardShortcuts {
spotlight: string;
newConversation: string;
toggleSidebar: string;
}

Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The KeyboardShortcuts interface is duplicated in this file (lines 5-9) and also exported from src/types/index.ts (lines 2-6). Consider importing the type from src/types/index.ts instead to maintain a single source of truth and avoid potential inconsistencies:

import type { KeyboardShortcuts, SettingsSchema } from '../types';

Then remove the local interface definition.

Suggested change
const claude = (window as any).claude;
interface KeyboardShortcuts {
spotlight: string;
newConversation: string;
toggleSidebar: string;
}
import type { KeyboardShortcuts } from '../types';

Copilot uses AI. Check for mistakes.
Comment on lines 11 to 15
interface Settings {
spotlightKeybind: string;
spotlightPersistHistory: boolean;
keyboardShortcuts: KeyboardShortcuts;
}
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Settings interface is defined here but it duplicates SettingsSchema from src/types/index.ts. Consider importing and reusing the exported SettingsSchema type to avoid duplication:

import type { KeyboardShortcuts, SettingsSchema } from '../types';

Then use SettingsSchema directly instead of defining a local Settings interface.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +22
const DEFAULT_SHORTCUTS: KeyboardShortcuts = {
spotlight: 'CommandOrControl+Shift+C',
newConversation: 'CommandOrControl+N',
toggleSidebar: 'CommandOrControl+B',
};
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default keyboard shortcuts are duplicated in two places: here in src/renderer/settings.ts (lines 18-22) and in src/main.ts (lines 13-17). If these values need to change, they would need to be updated in both places. Consider:

  1. Exporting DEFAULT_KEYBOARD_SHORTCUTS from src/types/index.ts or a shared constants file
  2. Having the renderer fetch these defaults from the main process via IPC

This ensures a single source of truth for default values.

Suggested change
const DEFAULT_SHORTCUTS: KeyboardShortcuts = {
spotlight: 'CommandOrControl+Shift+C',
newConversation: 'CommandOrControl+N',
toggleSidebar: 'CommandOrControl+B',
};
import { DEFAULT_SHORTCUTS } from '../types/index';

Copilot uses AI. Check for mistakes.
Comment on lines +125 to 126
saveSettings: (settings: { spotlightKeybind?: string; spotlightPersistHistory?: boolean; keyboardShortcuts?: { spotlight?: string; newConversation?: string; toggleSidebar?: string } }) =>
ipcRenderer.invoke('save-settings', settings),
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline type definition for keyboardShortcuts in the saveSettings parameter makes all nested properties optional, which differs from the KeyboardShortcuts interface in src/types/index.ts where all properties are required. This inconsistency could lead to confusion.

Consider using a proper Partial type or importing the actual type:

saveSettings: (settings: { 
  spotlightKeybind?: string; 
  spotlightPersistHistory?: boolean; 
  keyboardShortcuts?: Partial<KeyboardShortcuts>
}) => ipcRenderer.invoke('save-settings', settings),

Or better yet, import and use Partial<SettingsSchema> to match what the main process expects.

Copilot uses AI. Check for mistakes.
isRecordingKeybind = true;
// Set up keybind input handlers
function setupKeybindInput(inputEl: HTMLElement) {
const shortcutKey = inputEl.dataset.shortcut as keyof KeyboardShortcuts;
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable shortcutKey.

Suggested change
const shortcutKey = inputEl.dataset.shortcut as keyof KeyboardShortcuts;

Copilot uses AI. Check for mistakes.
@tkattkat
Copy link
Owner

tkattkat commented Dec 8, 2025

did this work for you? The sidebar shortcut hoes not seem to be working on my end after setting it to a key

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants