From ad33c22374fd88656f0785d1f9ad728bdac9075d Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Mon, 3 Nov 2025 16:22:04 -0800 Subject: [PATCH] Modify navigation and completion keyboard shortcuts to not use scroll. (#12502) --- docs/cli/keyboard-shortcuts.md | 10 +++--- packages/cli/src/config/keyBindings.test.ts | 22 ++++++++++++ packages/cli/src/config/keyBindings.ts | 36 +++++++++++++++---- .../cli/src/ui/components/SettingsDialog.tsx | 24 +++++++------ .../src/ui/hooks/useSelectionList.test.tsx | 35 ++++++++++++++++-- packages/cli/src/ui/hooks/useSelectionList.ts | 9 ++--- packages/cli/src/ui/keyMatchers.test.ts | 24 +++++++++++++ 7 files changed, 131 insertions(+), 29 deletions(-) diff --git a/docs/cli/keyboard-shortcuts.md b/docs/cli/keyboard-shortcuts.md index c712518504..05a5683ba2 100644 --- a/docs/cli/keyboard-shortcuts.md +++ b/docs/cli/keyboard-shortcuts.md @@ -52,11 +52,11 @@ This document lists the available keyboard shortcuts within Gemini CLI. ## Suggestions -| Shortcut | Description | -| --------------- | -------------------------------------- | -| `Down Arrow` | Navigate down through the suggestions. | -| `Tab` / `Enter` | Accept the selected suggestion. | -| `Up Arrow` | Navigate up through the suggestions. | +| Shortcut | Description | +| ----------------------- | -------------------------------------- | +| `Down Arrow` / `Ctrl+N` | Navigate down through the suggestions. | +| `Tab` / `Enter` | Accept the selected suggestion. | +| `Up Arrow` / `Ctrl+P` | Navigate up through the suggestions. | ## Radio Button Select diff --git a/packages/cli/src/config/keyBindings.test.ts b/packages/cli/src/config/keyBindings.test.ts index 1003290b8c..f80dd8aecb 100644 --- a/packages/cli/src/config/keyBindings.test.ts +++ b/packages/cli/src/config/keyBindings.test.ts @@ -55,5 +55,27 @@ describe('keyBindings config', () => { const config: KeyBindingConfig = defaultKeyBindings; expect(config[Command.HOME]).toBeDefined(); }); + + it('should have correct specific bindings', () => { + // Verify navigation ignores shift + const navUp = defaultKeyBindings[Command.NAVIGATION_UP]; + expect(navUp).toContainEqual({ key: 'up', shift: false }); + + const navDown = defaultKeyBindings[Command.NAVIGATION_DOWN]; + expect(navDown).toContainEqual({ key: 'down', shift: false }); + + // Verify dialog navigation + const dialogNavUp = defaultKeyBindings[Command.DIALOG_NAVIGATION_UP]; + expect(dialogNavUp).toContainEqual({ key: 'up', shift: false }); + expect(dialogNavUp).toContainEqual({ key: 'k', shift: false }); + + const dialogNavDown = defaultKeyBindings[Command.DIALOG_NAVIGATION_DOWN]; + expect(dialogNavDown).toContainEqual({ key: 'down', shift: false }); + expect(dialogNavDown).toContainEqual({ key: 'j', shift: false }); + + // Verify physical home/end keys + expect(defaultKeyBindings[Command.HOME]).toContainEqual({ key: 'home' }); + expect(defaultKeyBindings[Command.END]).toContainEqual({ key: 'end' }); + }); }); }); diff --git a/packages/cli/src/config/keyBindings.ts b/packages/cli/src/config/keyBindings.ts index 17a9c45d65..62d672a520 100644 --- a/packages/cli/src/config/keyBindings.ts +++ b/packages/cli/src/config/keyBindings.ts @@ -31,6 +31,10 @@ export enum Command { NAVIGATION_UP = 'navigationUp', NAVIGATION_DOWN = 'navigationDown', + // Dialog navigation + DIALOG_NAVIGATION_UP = 'dialogNavigationUp', + DIALOG_NAVIGATION_DOWN = 'dialogNavigationDown', + // Auto-completion ACCEPT_SUGGESTION = 'acceptSuggestion', COMPLETION_UP = 'completionUp', @@ -100,8 +104,8 @@ export const defaultKeyBindings: KeyBindingConfig = { [Command.ESCAPE]: [{ key: 'escape' }], // Cursor movement - [Command.HOME]: [{ key: 'a', ctrl: true }], - [Command.END]: [{ key: 'e', ctrl: true }], + [Command.HOME]: [{ key: 'a', ctrl: true }, { key: 'home' }], + [Command.END]: [{ key: 'e', ctrl: true }, { key: 'end' }], // Text deletion [Command.KILL_LINE_RIGHT]: [{ key: 'k', ctrl: true }], @@ -118,15 +122,33 @@ export const defaultKeyBindings: KeyBindingConfig = { // History navigation [Command.HISTORY_UP]: [{ key: 'p', ctrl: true, shift: false }], - [Command.HISTORY_DOWN]: [{ key: 'n', ctrl: true }], - [Command.NAVIGATION_UP]: [{ key: 'up' }], - [Command.NAVIGATION_DOWN]: [{ key: 'down' }], + [Command.HISTORY_DOWN]: [{ key: 'n', ctrl: true, shift: false }], + [Command.NAVIGATION_UP]: [{ key: 'up', shift: false }], + [Command.NAVIGATION_DOWN]: [{ key: 'down', shift: false }], + + // Dialog navigation + // Navigation shortcuts appropriate for dialogs where we do not need to accept + // text input. + [Command.DIALOG_NAVIGATION_UP]: [ + { key: 'up', shift: false }, + { key: 'k', shift: false }, + ], + [Command.DIALOG_NAVIGATION_DOWN]: [ + { key: 'down', shift: false }, + { key: 'j', shift: false }, + ], // Auto-completion [Command.ACCEPT_SUGGESTION]: [{ key: 'tab' }, { key: 'return', ctrl: false }], // Completion navigation (arrow or Ctrl+P/N) - [Command.COMPLETION_UP]: [{ key: 'up' }, { key: 'p', ctrl: true }], - [Command.COMPLETION_DOWN]: [{ key: 'down' }, { key: 'n', ctrl: true }], + [Command.COMPLETION_UP]: [ + { key: 'up', shift: false }, + { key: 'p', ctrl: true, shift: false }, + ], + [Command.COMPLETION_DOWN]: [ + { key: 'down', shift: false }, + { key: 'n', ctrl: true, shift: false }, + ], // Text input // Must also exclude shift to allow shift+enter for newline diff --git a/packages/cli/src/ui/components/SettingsDialog.tsx b/packages/cli/src/ui/components/SettingsDialog.tsx index e4f782cda0..b0f107c7a5 100644 --- a/packages/cli/src/ui/components/SettingsDialog.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.tsx @@ -38,6 +38,7 @@ import { TOGGLE_TYPES, } from '../../config/settingsSchema.js'; import { debugLogger } from '@google/gemini-cli-core'; +import { keyMatchers, Command } from '../keyMatchers.js'; interface SettingsDialogProps { settings: LoadedSettings; @@ -480,7 +481,7 @@ export function SettingsDialog({ useKeypress( (key) => { - const { name, ctrl } = key; + const { name } = key; if (name === 'tab' && showScopeSelection) { setFocusSection((prev) => (prev === 'settings' ? 'scope' : 'settings')); } @@ -523,11 +524,11 @@ export function SettingsDialog({ } return; } - if (name === 'escape') { + if (keyMatchers[Command.ESCAPE](key)) { commitEdit(editingKey); return; } - if (name === 'return') { + if (keyMatchers[Command.RETURN](key)) { commitEdit(editingKey); return; } @@ -564,18 +565,18 @@ export function SettingsDialog({ return; } // Home and End keys - if (name === 'home') { + if (keyMatchers[Command.HOME](key)) { setEditCursorPos(0); return; } - if (name === 'end') { + if (keyMatchers[Command.END](key)) { setEditCursorPos(cpLen(editBuffer)); return; } // Block other keys while editing return; } - if (name === 'up' || name === 'k') { + if (keyMatchers[Command.DIALOG_NAVIGATION_UP](key)) { // If editing, commit first if (editingKey) { commitEdit(editingKey); @@ -591,7 +592,7 @@ export function SettingsDialog({ } else if (newIndex < scrollOffset) { setScrollOffset(newIndex); } - } else if (name === 'down' || name === 'j') { + } else if (keyMatchers[Command.DIALOG_NAVIGATION_DOWN](key)) { // If editing, commit first if (editingKey) { commitEdit(editingKey); @@ -605,7 +606,7 @@ export function SettingsDialog({ } else if (newIndex >= scrollOffset + effectiveMaxItemsToShow) { setScrollOffset(newIndex - effectiveMaxItemsToShow + 1); } - } else if (name === 'return' || name === 'space') { + } else if (keyMatchers[Command.RETURN](key) || name === 'space') { const currentItem = items[activeSettingIndex]; if ( currentItem?.type === 'number' || @@ -620,7 +621,10 @@ export function SettingsDialog({ if (currentItem?.type === 'number') { startEditing(currentItem.value, key.sequence); } - } else if (ctrl && (name === 'c' || name === 'l')) { + } else if ( + keyMatchers[Command.CLEAR_INPUT](key) || + keyMatchers[Command.CLEAR_SCREEN](key) + ) { // Ctrl+C or Ctrl+L: Clear current setting and reset to default const currentSetting = items[activeSettingIndex]; if (currentSetting) { @@ -730,7 +734,7 @@ export function SettingsDialog({ setRestartRequiredSettings(new Set()); // Clear restart-required settings if (onRestartRequest) onRestartRequest(); } - if (name === 'escape') { + if (keyMatchers[Command.ESCAPE](key)) { if (editingKey) { commitEdit(editingKey); } else { diff --git a/packages/cli/src/ui/hooks/useSelectionList.test.tsx b/packages/cli/src/ui/hooks/useSelectionList.test.tsx index 61a181bcd0..7964bf591f 100644 --- a/packages/cli/src/ui/hooks/useSelectionList.test.tsx +++ b/packages/cli/src/ui/hooks/useSelectionList.test.tsx @@ -48,15 +48,19 @@ describe('useSelectionList', () => { mockOnHighlight.mockClear(); }); - const pressKey = (name: string, sequence: string = name) => { + const pressKey = ( + name: string, + sequence: string = name, + options: { shift?: boolean; ctrl?: boolean } = {}, + ) => { act(() => { if (activeKeypressHandler) { const key: Key = { name, sequence, - ctrl: false, + ctrl: options.ctrl ?? false, meta: false, - shift: false, + shift: options.shift ?? false, paste: false, }; activeKeypressHandler(key); @@ -202,6 +206,31 @@ describe('useSelectionList', () => { expect(result.current.activeIndex).toBe(0); }); + it('should ignore navigation keys when shift is pressed', async () => { + const { result } = await renderSelectionListHook({ + items, + initialIndex: 2, // Start at middle item 'C' + onSelect: mockOnSelect, + }); + expect(result.current.activeIndex).toBe(2); + + // Shift+Down / Shift+J should not move down + pressKey('down', undefined, { shift: true }); + expect(result.current.activeIndex).toBe(2); + pressKey('j', undefined, { shift: true }); + expect(result.current.activeIndex).toBe(2); + + // Shift+Up / Shift+K should not move up + pressKey('up', undefined, { shift: true }); + expect(result.current.activeIndex).toBe(2); + pressKey('k', undefined, { shift: true }); + expect(result.current.activeIndex).toBe(2); + + // Verify normal navigation still works + pressKey('down'); + expect(result.current.activeIndex).toBe(3); + }); + it('should wrap navigation correctly', async () => { const { result } = await renderSelectionListHook({ items, diff --git a/packages/cli/src/ui/hooks/useSelectionList.ts b/packages/cli/src/ui/hooks/useSelectionList.ts index d48466350e..eca760760d 100644 --- a/packages/cli/src/ui/hooks/useSelectionList.ts +++ b/packages/cli/src/ui/hooks/useSelectionList.ts @@ -6,6 +6,7 @@ import { useReducer, useRef, useEffect, useCallback } from 'react'; import { useKeypress, type Key } from './useKeypress.js'; +import { keyMatchers, Command } from '../keyMatchers.js'; export interface SelectionListItem { key: string; @@ -318,7 +319,7 @@ export function useSelectionList({ const itemsLength = items.length; const handleKeypress = useCallback( (key: Key) => { - const { sequence, name } = key; + const { sequence } = key; const isNumeric = showNumbers && /^[0-9]$/.test(sequence); // Clear number input buffer on non-numeric key press @@ -327,17 +328,17 @@ export function useSelectionList({ numberInputRef.current = ''; } - if (name === 'k' || name === 'up') { + if (keyMatchers[Command.DIALOG_NAVIGATION_UP](key)) { dispatch({ type: 'MOVE_UP' }); return; } - if (name === 'j' || name === 'down') { + if (keyMatchers[Command.DIALOG_NAVIGATION_DOWN](key)) { dispatch({ type: 'MOVE_DOWN' }); return; } - if (name === 'return') { + if (keyMatchers[Command.RETURN](key)) { dispatch({ type: 'SELECT_CURRENT' }); return; } diff --git a/packages/cli/src/ui/keyMatchers.test.ts b/packages/cli/src/ui/keyMatchers.test.ts index a2adeb090b..baaa88dbff 100644 --- a/packages/cli/src/ui/keyMatchers.test.ts +++ b/packages/cli/src/ui/keyMatchers.test.ts @@ -36,6 +36,10 @@ describe('keyMatchers', () => { [Command.HISTORY_DOWN]: (key: Key) => key.ctrl && key.name === 'n', [Command.NAVIGATION_UP]: (key: Key) => key.name === 'up', [Command.NAVIGATION_DOWN]: (key: Key) => key.name === 'down', + [Command.DIALOG_NAVIGATION_UP]: (key: Key) => + !key.shift && (key.name === 'up' || key.name === 'k'), + [Command.DIALOG_NAVIGATION_DOWN]: (key: Key) => + !key.shift && (key.name === 'down' || key.name === 'j'), [Command.ACCEPT_SUGGESTION]: (key: Key) => key.name === 'tab' || (key.name === 'return' && !key.ctrl), [Command.COMPLETION_UP]: (key: Key) => @@ -158,6 +162,26 @@ describe('keyMatchers', () => { negative: [createKey('n'), createKey('d')], }, + // Dialog navigation + { + command: Command.DIALOG_NAVIGATION_UP, + positive: [createKey('up'), createKey('k')], + negative: [ + createKey('up', { shift: true }), + createKey('k', { shift: true }), + createKey('p'), + ], + }, + { + command: Command.DIALOG_NAVIGATION_DOWN, + positive: [createKey('down'), createKey('j')], + negative: [ + createKey('down', { shift: true }), + createKey('j', { shift: true }), + createKey('n'), + ], + }, + // Auto-completion { command: Command.ACCEPT_SUGGESTION,