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
This commit is contained in:
parent
3e276d78ba
commit
d1532956fd
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue