Modify navigation and completion keyboard shortcuts to not use scroll. (#12502)

jacob314 · web-flow · commit ad33c22374fd · 2025-11-04T00:22:04.000Z
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<T> {
   key: string;
@@ -318,7 +319,7 @@ export function useSelectionList<T>({
   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<T>({
         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
@@ -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,
