feat: configurable keyboard shortcuts#2321
Open
Basit-Balogun10 wants to merge 7 commits into
Open
Conversation
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/code/src/renderer/stores/keybindingsStore.ts:118-121
`updateKeybinding` does not deduplicate the result, so editing one binding to a value that already exists as another binding for the same shortcut silently produces `["ctrl+q", "ctrl+q"]`. The conflict-detection in the recording modal excludes the shortcut being edited (the `excludeId` skip), so the duplicate slips through undetected. The UI then renders two chips with identical `key` props, producing a React duplicate-key warning and potentially broken reconciliation.
```suggestion
const updated = base.map((k) => (k === oldKey ? newKey : k));
// Deduplicate in case newKey already exists elsewhere in the array.
const deduped = [...new Set(updated)];
set({
customKeybindings: { ...get().customKeybindings, [id]: deduped },
});
```
### Issue 2 of 2
apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts:326-365
`prompt-history-prev/next` shortcuts silently break when remapped to non-arrow-key combos. The outer `event.key === "ArrowUp"` / `event.key === "ArrowDown"` guards are still hardcoded, so `forcePrev`/`forceNext` can be `true` from the store but the handler never fires unless the physical key is an arrow. A user who remaps `prompt-history-prev` to e.g. `ctrl+k` gets a chip in the UI that saves correctly but does nothing when pressed.
Reviews (1): Last reviewed commit: "feat: make prompt-history shortcuts conf..." | Re-trigger Greptile |
Users can now remap any of the 17 configurable shortcuts via Settings > Shortcuts (or the ⌘/ sheet). Custom bindings fully replace all defaults (including alternates) and multiple custom combos per action are supported. Bindings persist across sessions via electronStorage. - Add `configurable` flag + `DEFAULT_KEYBINDINGS` map to keyboard-shortcuts.ts - New `keybindingsStore` (persist + electronStorage) with array-based custom combos, conflict detection helper, and individual/bulk reset - New `useShortcut(id)` hook — reactive Zustand selector, feeds useHotkeys - New `Keycap` component extracted to avoid circular imports - New `ShortcutRecorder` component: click + to enter recording mode, captures keydown, shows conflict toast, per-binding × remove, per-shortcut ↩ reset - Update all useHotkeys call sites (GlobalEventHandlers, SpaceSwitcher, usePanelKeyboardShortcuts, ExternalAppsOpener) to use useShortcut() - KeyboardShortcutsSheet: configurable rows render ShortcutRecorder instead of static keycaps; "Reset all shortcuts" button shown when customisations exist Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218
Bare letter keys (e.g. just "k") would fire every time that character is typed anywhere in the app. Require at least mod/ctrl/alt to be held. Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218
24 tests covering resolveKey, addKeybinding, removeKeybinding, resetShortcut, resetAll, getKey, and findConflict — including conflict detection against comma-separated default alternates. Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218
- KeyboardShortcutsSheet header now reads the "shortcuts" key via useShortcut() so the trigger keycap updates when remapped - ExternalAppsOpener dropdown labels for open-in-editor and copy-path now derive from useShortcut() + formatHotkeyParts() instead of hardcoded Mac-only symbols test(e2e): add Playwright shortcut sheet tests Covers sheet open/close, category sections, hover controls, recording mode entry/cancellation, bare-key rejection, saving bindings, conflict detection, removing bindings, per-shortcut reset, and reset-all. Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218
Hardcoded Cmd glyphs were leaking onto Windows in the send-messages dropdown and the tiptap paste hint, and two handlers were gated on metaKey only so the corresponding shortcut never fired on Windows (mod+1..9 task switching, Cmd/Ctrl-click multi-select in the inbox). Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218
- Add prompt-history-prev/next to CONFIGURABLE_SHORTCUT_IDS and DEFAULT_KEYBINDINGS so they appear in the shortcuts sheet and can be rebound like any other shortcut - Add tiptapEventToCombo() — accepts shift-only combos (no Ctrl/Meta required) so shift+up/down can be matched against live bindings - Fix eventToCombo() to normalise Arrow-prefixed key names (ArrowUp to up) - Wire useTiptapEditor to resolve prompt-history keys from the store instead of hardcoding event.shiftKey - Fix paste hint toast to show the live paste-as-file binding instead of the hardcoded mod+shift+v string - Fix noStaticElementInteractions lint on recording modal backdrop - Rewrite E2E shortcut tests to match the current recording modal UI (chips + right-click context menu) rather than the old hover-button and inline-input design
- Deduplicate in updateKeybinding — conflict detection excludes the shortcut being edited so editing one binding to match another on the same shortcut could produce ["ctrl+q","ctrl+q"], duplicate React keys and broken chip reconciliation - Remove ArrowUp/Down gate around prompt-history navigation so custom non-arrow bindings (e.g. Ctrl+K) actually fire when pressed, not just when the physical key is an arrow - Remove obvious section-divider comments and redundant JSX labels (Header, Scrollable list, Sticky footer); keep non-obvious rationale comments (window-level capture, backdrop dismiss, canAddMore budget, dedup note, ArrowKey gate explanation)
5a62f10 to
aa353a1
Compare
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
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.
Problem
Users cannot remap keyboard shortcuts to suit their workflow or resolve conflicts with OS/app bindings on their machine. Closes #300
Changes
Core store (
keybindingsStore.ts)Persisted Zustand store backed by
electronStorage— bindings survive app restarts. Stores custom overrides asPartial<Record<ConfigurableShortcutId, string[]>>.addKeybinding/updateKeybinding/removeKeybinding— per-binding CRUD, max 2 custom bindings per shortcutresetShortcut/resetAll— revert one or all shortcuts to defaultsresolveKey— falls back to the static default when no custom entry existssplitBindings— splits comma-separated binding strings while correctly handlingmod+,(the Settings shortcut key contains a literal comma — uses negative lookbehind regex(?<!\+),to distinguish separator commas from key commas)findConflict— checks the candidate key against all other configurable shortcuts (with live overrides applied) and all non-configurable static shortcuts; returns the conflicting description and whether it is a fixed shortcutDefinitions (
keyboard-shortcuts.ts)configurable: truewith an explicitCONFIGURABLE_SHORTCUT_IDSarray andDEFAULT_KEYBINDINGSrecordeventToCombo— converts aKeyboardEventto a normalised combo string (mod+shift+v); requires at least one non-shift modifier; normalisesArrowUp → upetc.tiptapEventToCombo— likeeventToCombobut also accepts shift-only combos, needed forprompt-history-prev/nextwhose defaults areshift+up/shift+down⌘⇧⌥↩↑↓, Windows showsCtrl+Shift+Alt+Enter+↑+↓. This also fixed a bug where hardcoded shortcut labels in the editor (e.g. the paste-as-file hint toast) were showing the Mac⌘symbol on Windows instead ofCtrlShortcut recorder UI (
ShortcutRecorder.tsx)ShortcutRecordingModal— a fullscreen capture overlay:keydownlistener (no element focus required)BindingChip— individual keycap with left-click to edit, right-click context menu:ShortcutRecorder— row-level component:mod+n,mod+t) into individual chipscanAddMorebased on custom count only — defaults don't consume the 2-binding budgetupdateKeybindingcopies them into the custom array before replacing only the targetShortcuts sheet (
KeyboardShortcutsSheet.tsx)ShortcutRecorder; non-configurable rows render staticShortcutKeyswith a "This shortcut cannot be customized" tooltip on hoverEditor integration (
useTiptapEditor.ts)paste-as-filereads from the store dynamically; supports multiple bindingsprompt-history-prev/nextresolved from store viatiptapEventToCombo; replaces the old hardcodedevent.shiftKeycheckpaste-as-filebinding (was hardcodedmod+shift+v)Non-configurable shortcuts — decisions
mod+1-9)ctrl+1-9)mod+f)mod+b/i/u/e)How did you test this?
Unit tests — 30 tests in
keybindingsStore.test.tscovering:resolveKey,addKeybinding(dedup, max-limit enforcement),removeKeybinding,resetShortcut,resetAll,getKey,updateKeybinding(editing a default preserves siblings, editing custom replaces only target), andfindConflict(configurable defaults, custom overrides on other shortcuts, non-configurable fixed shortcuts,mod+,edge case, excluded shortcut own key not flagged as conflict). All 30 pass.Typecheck — clean (
tsc --noEmiton bothtsconfig.node.jsonandtsconfig.web.json).E2E tests (
shortcuts.spec.ts) — wrote a full E2E suite covering:Manual testing:
1. Open the Keyboard Shortcuts Sheet
Default shortcut:
Cmd+/(Mac) /Ctrl+/(Windows)2. The Sheet Layout — Configurable vs Fixed
Scroll through all four categories: General, Navigation, Panels & Tabs, Editor.
Configurable rows (20 shortcuts) show interactive keycap chips you can click.
Fixed rows (8 shortcuts) show static chips. Hover one — tooltip reads "This shortcut cannot be customized."
Fixed shortcuts and why:
Mod+1-9Ctrl+1-9Mod+FMod+B/I/U/Econfigurable-vs-fixed-shortcuts-posthog-code-configurable-shortcuts.mp4
3. Edit a Shortcut — Click to Edit
Cmd+K/Ctrl+K).Cmd+Shift+P. The modal shows the combo formatted with platform symbols (Mac:⌘⇧P, Windows:Ctrl+Shift+P).editing-a-shortcut-posthog-code-configurable-shortcuts.mp4.mp4
4. Conflict Detection — Amber Warning
Cmd+N(which is "New task").Edge case —
mod+,(Settings): The comma is also the separator character used internally. Try to bindCmd+,to something — it correctly detects it conflicts with "Open settings" despite the comma in the key name.Non-configurable conflict: Try to bind
Mod+B(Bold, which is a fixed Tiptap shortcut). The conflict detection catches this too — you will see it warns "Conflicts with 'Bold'."conflict-detection-posthog-code-configurable-shortcuts.mp4.mp4.mp4
5. Escape / Backdrop to Cancel
escape-to-cancel-posthog-code-configurable-shortcuts.mp4.mp4.mp4
6. Add a Second Binding (Up to 2 Custom Bindings)
Cmd+Shift+N. Press Enter.Cmd+N or Cmd+Shift+N(separated by "or").adding-a-second-binding-posthog-code-configurable-shortcuts.mp4.mp4
7. Default Shortcuts with Multiple Bindings
Some shortcuts ship with 2 default bindings (e.g. New task:
Cmd+NorCmd+T).Cmd+Nstores["mod+t"]in the custom array. The other default is preserved.Cmd+NandCmd+Ttogether.default-shortcuts-with-multiple-binding-posthog-code-configurable-shortcuts.mp4
8. Edit One Default Binding — Preserves the Other
Cmd+NandCmd+Tchips visible).Cmd+Nto edit it. RecordCmd+Shift+X. Press Enter.Cmd+Shift+X or Cmd+T— only the edited key changed, the sibling default (Cmd+T) is preserved.Uploading editing-one-default-binding-posthog-code-configurable-shortcuts.mp4…
9. Reset ALL Shortcuts
resetting-all-shortcuts-posthog-code-configurable-shortcuts.mp4
10. Paste-as-File Shortcut (Configurable, Live Hint)
In the Editor category, find "Paste as file attachment" (
Cmd+Shift+V/Ctrl+Shift+V).Ctrl+Shift+Vto paste as a file attachment instead." (shows the live current binding).Ctrl+Shift+G.Ctrl+Shift+Gto paste as a file attachment instead." ✅paste-as-file-shortcut-posthog-code-configurable-shortcuts.mp4
11. Persistence Across App Restarts
electronStorage(Electron user data directory).Publish to changelog?
Probably? Users can now remap any of the 20 configurable shortcuts directly inside the app via the Keyboard Shortcuts sheet (
⌘//Ctrl+/) - and potentially any new additions to the app shortcuts can now be configurable