fix(cli): override j/k navigation in settings dialog to fix search input conflict (#22800)

This commit is contained in:
Sehoon Shon
2026-03-17 15:08:45 -04:00
committed by GitHub
parent fc51e50bc6
commit b211f30d95
3 changed files with 57 additions and 5 deletions

View File

@@ -52,6 +52,8 @@ enum TerminalKeys {
RIGHT_ARROW = '\u001B[C',
ESCAPE = '\u001B',
BACKSPACE = '\u0008',
CTRL_P = '\u0010',
CTRL_N = '\u000E',
}
vi.mock('../../config/settingsSchema.js', async (importOriginal) => {
@@ -357,9 +359,9 @@ describe('SettingsDialog', () => {
up: TerminalKeys.UP_ARROW,
},
{
name: 'vim keys (j/k)',
down: 'j',
up: 'k',
name: 'emacs keys (Ctrl+P/N)',
down: TerminalKeys.CTRL_N,
up: TerminalKeys.CTRL_P,
},
])('should navigate with $name', async ({ down, up }) => {
const settings = createMockSettings();
@@ -397,6 +399,31 @@ describe('SettingsDialog', () => {
unmount();
});
it('should allow j and k characters to be typed in search without triggering navigation', async () => {
const settings = createMockSettings();
const onSelect = vi.fn();
const { lastFrame, stdin, waitUntilReady, unmount } = renderDialog(
settings,
onSelect,
);
await waitUntilReady();
// Enter 'j' and 'k' in search
await act(async () => stdin.write('j'));
await waitUntilReady();
await act(async () => stdin.write('k'));
await waitUntilReady();
await waitFor(() => {
const frame = lastFrame();
// The search box should contain 'jk'
expect(frame).toContain('jk');
// Since 'jk' doesn't match any setting labels, it should say "No matches found."
expect(frame).toContain('No matches found.');
});
unmount();
});
it('wraps around when at the top of the list', async () => {
const settings = createMockSettings();
const onSelect = vi.fn();

View File

@@ -43,6 +43,8 @@ import {
BaseSettingsDialog,
type SettingsDialogItem,
} from './shared/BaseSettingsDialog.js';
import { useKeyMatchers } from '../hooks/useKeyMatchers.js';
import { Command, KeyBinding } from '../key/keyBindings.js';
interface FzfResult {
item: string;
@@ -60,6 +62,11 @@ interface SettingsDialogProps {
const MAX_ITEMS_TO_SHOW = 8;
const KEY_UP = new KeyBinding('up');
const KEY_CTRL_P = new KeyBinding('ctrl+p');
const KEY_DOWN = new KeyBinding('down');
const KEY_CTRL_N = new KeyBinding('ctrl+n');
// Create a snapshot of the initial per-scope state of Restart Required Settings
// This creates a nested map of the form
// restartRequiredSetting -> Map { scopeName -> value }
@@ -336,6 +343,18 @@ export function SettingsDialog({
onSelect(undefined, selectedScope as SettingScope);
}, [onSelect, selectedScope]);
const globalKeyMatchers = useKeyMatchers();
const settingsKeyMatchers = useMemo(
() => ({
...globalKeyMatchers,
[Command.DIALOG_NAVIGATION_UP]: (key: Key) =>
KEY_UP.matches(key) || KEY_CTRL_P.matches(key),
[Command.DIALOG_NAVIGATION_DOWN]: (key: Key) =>
KEY_DOWN.matches(key) || KEY_CTRL_N.matches(key),
}),
[globalKeyMatchers],
);
// Custom key handler for restart key
const handleKeyPress = useCallback(
(key: Key, _currentItem: SettingsDialogItem | undefined): boolean => {
@@ -371,6 +390,7 @@ export function SettingsDialog({
onItemClear={handleItemClear}
onClose={handleClose}
onKeyPress={handleKeyPress}
keyMatchers={settingsKeyMatchers}
footer={
showRestartPrompt
? {

View File

@@ -19,7 +19,7 @@ import { TextInput } from './TextInput.js';
import type { TextBuffer } from './text-buffer.js';
import { cpSlice, cpLen, cpIndexToOffset } from '../../utils/textUtils.js';
import { useKeypress, type Key } from '../../hooks/useKeypress.js';
import { Command } from '../../key/keyMatchers.js';
import { Command, type KeyMatchers } from '../../key/keyMatchers.js';
import { useSettingsNavigation } from '../../hooks/useSettingsNavigation.js';
import { useInlineEditBuffer } from '../../hooks/useInlineEditBuffer.js';
import { formatCommand } from '../../key/keybindingUtils.js';
@@ -103,6 +103,9 @@ export interface BaseSettingsDialogProps {
currentItem: SettingsDialogItem | undefined,
) => boolean;
/** Optional override for key matchers used for navigation. */
keyMatchers?: KeyMatchers;
/** Available terminal height for dynamic windowing */
availableHeight?: number;
@@ -134,10 +137,12 @@ export function BaseSettingsDialog({
onItemClear,
onClose,
onKeyPress,
keyMatchers: customKeyMatchers,
availableHeight,
footer,
}: BaseSettingsDialogProps): React.JSX.Element {
const keyMatchers = useKeyMatchers();
const globalKeyMatchers = useKeyMatchers();
const keyMatchers = customKeyMatchers ?? globalKeyMatchers;
// Calculate effective max items and scope visibility based on terminal height
const { effectiveMaxItemsToShow, finalShowScopeSelector } = useMemo(() => {
const initialShowScope = showScopeSelector;