From d1532956fd32f47aa8fea700ca231cad8dad7fda Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 17 Jul 2026 18:57:26 -0700 Subject: [PATCH] fix(keybindings): AltGr-safe default for Add Review Note (#9257) * fix(keybindings): use AltGr-safe default for Add Review Note The editor.addReviewNote default was Mod+Alt+N, which resolves to Ctrl+Alt+N (AltGr) on Windows/Linux. On diacritic layouts AltGr+N types a real character (e.g. Polish n-acute), so the editor-scope chord hijacked normal typing. Switch the default to Mod+Shift+A, which is AltGr-safe and keeps a mnemonic (A for annotate). * test(keybindings): cover Add Review Note chord end to end --- .../editor/editor-shortcuts.test.ts | 28 +++++++------- ...rkdown-preview-annotation-shortcut.test.ts | 22 ++++++++--- .../rich-markdown-annotation-shortcut.ts | 2 +- .../editor/rich-markdown-key-handler.test.ts | 4 +- src/shared/keybindings.test.ts | 38 ++++++++++++++++--- src/shared/keybindings.ts | 4 +- .../markdown-add-review-note-shortcut.spec.ts | 8 ++-- 7 files changed, 74 insertions(+), 32 deletions(-) diff --git a/src/renderer/src/components/editor/editor-shortcuts.test.ts b/src/renderer/src/components/editor/editor-shortcuts.test.ts index b9a64b55c..6e7c0a774 100644 --- a/src/renderer/src/components/editor/editor-shortcuts.test.ts +++ b/src/renderer/src/components/editor/editor-shortcuts.test.ts @@ -212,29 +212,29 @@ describe('installEditorAddReviewNoteShortcut', () => { const dispose = installEditorAddReviewNoteShortcut(container, onAddReviewNote) const defaultEvent = dispatchKeyDown(input, { - key: 'n', - code: 'KeyN', + key: 'a', + code: 'KeyA', metaKey: true, - altKey: true + shiftKey: true }) const repeatEvent = dispatchKeyDown(input, { - key: 'n', - code: 'KeyN', + key: 'a', + code: 'KeyA', metaKey: true, - altKey: true, + shiftKey: true, repeat: true }) - const unrelatedEvent = dispatchKeyDown(input, { key: 'n', code: 'KeyN', metaKey: true }) + const unrelatedEvent = dispatchKeyDown(input, { key: 'a', code: 'KeyA', metaKey: true }) expect(defaultEvent.defaultPrevented).toBe(true) expect(repeatEvent.defaultPrevented).toBe(false) expect(unrelatedEvent.defaultPrevented).toBe(false) expect(onAddReviewNote).toHaveBeenCalledTimes(1) - shortcutState.keybindings = { 'editor.addReviewNote': ['Mod+Shift+A'] } + shortcutState.keybindings = { 'editor.addReviewNote': ['Mod+Shift+K'] } const overriddenEvent = dispatchKeyDown(input, { - key: 'a', - code: 'KeyA', + key: 'k', + code: 'KeyK', metaKey: true, shiftKey: true }) @@ -242,7 +242,7 @@ describe('installEditorAddReviewNoteShortcut', () => { expect(onAddReviewNote).toHaveBeenCalledTimes(2) dispose() - dispatchKeyDown(input, { key: 'a', code: 'KeyA', metaKey: true, shiftKey: true }) + dispatchKeyDown(input, { key: 'k', code: 'KeyK', metaKey: true, shiftKey: true }) expect(onAddReviewNote).toHaveBeenCalledTimes(2) }) @@ -257,10 +257,10 @@ describe('installEditorAddReviewNoteShortcut', () => { const dispose = installEditorAddReviewNoteShortcut(container, onAddReviewNote) const event = dispatchKeyDown(input, { - key: 'n', - code: 'KeyN', + key: 'a', + code: 'KeyA', metaKey: true, - altKey: true + shiftKey: true }) expect(onAddReviewNote).toHaveBeenCalledTimes(1) diff --git a/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.test.ts b/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.test.ts index ed16221df..296cb31b9 100644 --- a/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.test.ts +++ b/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.test.ts @@ -76,19 +76,31 @@ describe('getMarkdownAnnotationBlockKeyForSelection', () => { describe('isMarkdownPreviewAddReviewNoteShortcut', () => { it('matches the default binding and respects overrides', () => { const defaultEvent = { - key: 'n', - code: 'KeyN', + key: 'a', + code: 'KeyA', metaKey: true, ctrlKey: false, - altKey: true, - shiftKey: false + altKey: false, + shiftKey: true } expect(isMarkdownPreviewAddReviewNoteShortcut(defaultEvent, 'darwin')).toBe(true) expect(isMarkdownPreviewAddReviewNoteShortcut(defaultEvent, 'linux')).toBe(false) + expect( + isMarkdownPreviewAddReviewNoteShortcut( + { ...defaultEvent, metaKey: false, ctrlKey: true }, + 'linux' + ) + ).toBe(true) + expect( + isMarkdownPreviewAddReviewNoteShortcut( + { ...defaultEvent, metaKey: false, ctrlKey: true }, + 'win32' + ) + ).toBe(true) expect( isMarkdownPreviewAddReviewNoteShortcut(defaultEvent, 'darwin', { - 'editor.addReviewNote': ['Mod+Shift+A'] + 'editor.addReviewNote': ['Mod+Shift+K'] }) ).toBe(false) }) diff --git a/src/renderer/src/components/editor/rich-markdown-annotation-shortcut.ts b/src/renderer/src/components/editor/rich-markdown-annotation-shortcut.ts index 318154d41..5b9fccbf3 100644 --- a/src/renderer/src/components/editor/rich-markdown-annotation-shortcut.ts +++ b/src/renderer/src/components/editor/rich-markdown-annotation-shortcut.ts @@ -2,7 +2,7 @@ import type { KeyHandlerContext } from './rich-markdown-key-handler' import { editorShortcutMatches } from './editor-shortcuts' /** - * Mod+Alt+N: open the review-note composer for the current selection. + * Mod+Shift+A: open the review-note composer for the current selection. */ export function handleRichMarkdownAddReviewNoteShortcut( ctx: KeyHandlerContext, diff --git a/src/renderer/src/components/editor/rich-markdown-key-handler.test.ts b/src/renderer/src/components/editor/rich-markdown-key-handler.test.ts index ebd71f84f..f2eaba114 100644 --- a/src/renderer/src/components/editor/rich-markdown-key-handler.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-key-handler.test.ts @@ -117,7 +117,7 @@ describe('rich markdown key handler', () => { try { const ctx = createContext(editor, false) - const event = keyEvent('n', { metaKey: true, altKey: true, code: 'KeyN' }) + const event = keyEvent('a', { metaKey: true, shiftKey: true, code: 'KeyA' }) expect(createRichMarkdownKeyHandler(ctx)(null, event)).toBe(true) expect(event.preventDefault).toHaveBeenCalled() @@ -133,7 +133,7 @@ describe('rich markdown key handler', () => { try { const ctx = createContext(editor, false) ctx.openAnnotationPopoverRef.current = vi.fn(() => false) - const event = keyEvent('n', { metaKey: true, altKey: true, code: 'KeyN' }) + const event = keyEvent('a', { metaKey: true, shiftKey: true, code: 'KeyA' }) expect(createRichMarkdownKeyHandler(ctx)(null, event)).toBe(false) expect(event.preventDefault).not.toHaveBeenCalled() diff --git a/src/shared/keybindings.test.ts b/src/shared/keybindings.test.ts index 6c8608975..ea69dee11 100644 --- a/src/shared/keybindings.test.ts +++ b/src/shared/keybindings.test.ts @@ -216,12 +216,40 @@ describe('keybindings', () => { it('defines a default shortcut for adding an editor review note', () => { expect(getEffectiveKeybindingsForAction('editor.addReviewNote', 'darwin')).toEqual([ - 'Mod+Alt+N' + 'Mod+Shift+A' ]) - expect(getEffectiveKeybindingsForAction('editor.addReviewNote', 'linux')).toEqual(['Mod+Alt+N']) - expect(getEffectiveKeybindingsForAction('editor.addReviewNote', 'win32')).toEqual(['Mod+Alt+N']) - expect(formatKeybindingList(['Mod+Alt+N'], 'darwin')).toBe('⌘⌥N') - expect(formatKeybindingList(['Mod+Alt+N'], 'linux')).toBe('Ctrl+Alt+N') + expect(getEffectiveKeybindingsForAction('editor.addReviewNote', 'linux')).toEqual([ + 'Mod+Shift+A' + ]) + expect(getEffectiveKeybindingsForAction('editor.addReviewNote', 'win32')).toEqual([ + 'Mod+Shift+A' + ]) + expect(formatKeybindingList(['Mod+Shift+A'], 'darwin')).toBe('⌘⇧A') + expect(formatKeybindingList(['Mod+Shift+A'], 'linux')).toBe('Ctrl+Shift+A') + + const macChord = { + key: 'a', + code: 'KeyA', + meta: true, + control: false, + alt: false, + shift: true + } + const ctrlChord = { ...macChord, meta: false, control: true } + expect(keybindingMatchesAction('editor.addReviewNote', macChord, 'darwin')).toBe(true) + expect(keybindingMatchesAction('editor.addReviewNote', ctrlChord, 'linux')).toBe(true) + expect(keybindingMatchesAction('editor.addReviewNote', ctrlChord, 'win32')).toBe(true) + + const oldCtrlAltChord = { + key: 'n', + code: 'KeyN', + meta: false, + control: true, + alt: true, + shift: false + } + expect(keybindingMatchesAction('editor.addReviewNote', oldCtrlAltChord, 'linux')).toBe(false) + expect(keybindingMatchesAction('editor.addReviewNote', oldCtrlAltChord, 'win32')).toBe(false) }) it('defines platform-native replace-in-editor shortcuts', () => { diff --git a/src/shared/keybindings.ts b/src/shared/keybindings.ts index 857446a80..8dfba7e15 100644 --- a/src/shared/keybindings.ts +++ b/src/shared/keybindings.ts @@ -865,7 +865,9 @@ export const KEYBINDING_DEFINITIONS: readonly KeybindingDefinition[] = [ group: 'Editors', scope: 'editor', searchKeywords: ['shortcut', 'editor', 'markdown', 'note', 'comment', 'annotation', 'review'], - defaultBindings: platformBindings(['Mod+Alt+N']) + // Why: Ctrl+Alt+letter is AltGr text input on Windows/Linux; an editor-scope + // default must not reserve characters such as Polish `ń`. + defaultBindings: platformBindings(['Mod+Shift+A']) }, { id: 'fileExplorer.undo', diff --git a/tests/e2e/markdown-add-review-note-shortcut.spec.ts b/tests/e2e/markdown-add-review-note-shortcut.spec.ts index 3fb16f5c0..e9ba51e0a 100644 --- a/tests/e2e/markdown-add-review-note-shortcut.spec.ts +++ b/tests/e2e/markdown-add-review-note-shortcut.spec.ts @@ -32,7 +32,7 @@ test.describe('Markdown add-review-note shortcut', () => { await editor.click() await orcaPage.keyboard.press('ControlOrMeta+A') - await orcaPage.keyboard.press('ControlOrMeta+Alt+N') + await orcaPage.keyboard.press('ControlOrMeta+Shift+A') await expect(orcaPage.getByPlaceholder('Add note for the AI')).toBeVisible({ timeout: 5_000 @@ -76,7 +76,7 @@ test.describe('Markdown add-review-note shortcut', () => { await monaco.click() await orcaPage.keyboard.press('ControlOrMeta+A') - await orcaPage.keyboard.press('ControlOrMeta+Alt+N') + await orcaPage.keyboard.press('ControlOrMeta+Shift+A') await expect(orcaPage.getByPlaceholder('Add note for the AI')).toBeVisible({ timeout: 5_000 @@ -149,7 +149,7 @@ test.describe('Markdown add-review-note shortcut', () => { selection?.addRange(range) }) - await orcaPage.keyboard.press('ControlOrMeta+Alt+N') + await orcaPage.keyboard.press('ControlOrMeta+Shift+A') await expect(orcaPage.getByPlaceholder('Add note for the AI')).toBeVisible({ timeout: 5_000 @@ -174,7 +174,7 @@ test.describe('Markdown add-review-note shortcut', () => { const editor = await waitForRichMarkdownEditor(orcaPage) await editor.click() - await orcaPage.keyboard.press('ControlOrMeta+Alt+N') + await orcaPage.keyboard.press('ControlOrMeta+Shift+A') await expect(orcaPage.getByPlaceholder('Add note for the AI')).toHaveCount(0) } finally {