diff --git a/packages/cli/src/ui/components/shared/BaseSettingsDialog.test.tsx b/packages/cli/src/ui/components/shared/BaseSettingsDialog.test.tsx index 2cdc314e39..42fc6ec845 100644 --- a/packages/cli/src/ui/components/shared/BaseSettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/shared/BaseSettingsDialog.test.tsx @@ -35,40 +35,56 @@ enum TerminalKeys { CTRL_L = '\u000C', } -const createMockItems = (): SettingsDialogItem[] => [ - { - key: 'boolean-setting', - label: 'Boolean Setting', - description: 'A boolean setting for testing', - displayValue: 'true', - rawValue: true, - type: 'boolean', - }, - { - key: 'string-setting', - label: 'String Setting', - description: 'A string setting for testing', - displayValue: 'test-value', - rawValue: 'test-value', - type: 'string', - }, - { - key: 'number-setting', - label: 'Number Setting', - description: 'A number setting for testing', - displayValue: '42', - rawValue: 42, - type: 'number', - }, - { - key: 'enum-setting', - label: 'Enum Setting', - description: 'An enum setting for testing', - displayValue: 'option-a', - rawValue: 'option-a', - type: 'enum', - }, -]; +const createMockItems = (count = 4): SettingsDialogItem[] => { + const items: SettingsDialogItem[] = [ + { + key: 'boolean-setting', + label: 'Boolean Setting', + description: 'A boolean setting for testing', + displayValue: 'true', + rawValue: true, + type: 'boolean', + }, + { + key: 'string-setting', + label: 'String Setting', + description: 'A string setting for testing', + displayValue: 'test-value', + rawValue: 'test-value', + type: 'string', + }, + { + key: 'number-setting', + label: 'Number Setting', + description: 'A number setting for testing', + displayValue: '42', + rawValue: 42, + type: 'number', + }, + { + key: 'enum-setting', + label: 'Enum Setting', + description: 'An enum setting for testing', + displayValue: 'option-a', + rawValue: 'option-a', + type: 'enum', + }, + ]; + + // If count is larger than our base mock items, generate dynamic ones + if (count > items.length) { + for (let i = items.length; i < count; i++) { + items.push({ + key: `extra-setting-${i}`, + label: `Extra Setting ${i}`, + displayValue: `value-${i}`, + type: 'string', + }); + } + } + + return items.slice(0, count); +}; describe('BaseSettingsDialog', () => { let mockOnItemToggle: ReturnType; @@ -211,7 +227,7 @@ describe('BaseSettingsDialog', () => { }); it('should wrap around when navigating past last item', async () => { - const items = createMockItems().slice(0, 2); // Only 2 items + const items = createMockItems(2); // Only 2 items const { stdin } = renderDialog({ items }); // Press down twice to go past the last item @@ -260,6 +276,112 @@ describe('BaseSettingsDialog', () => { }); }); + describe('scrolling and resizing list (search filtering)', () => { + it('should preserve focus on the active item if it remains in the filtered list', async () => { + const items = createMockItems(5); // items 0 to 4 + const { rerender, stdin, lastFrame } = renderDialog({ + items, + maxItemsToShow: 5, + }); + + // Move focus down to item 2 ("Number Setting") + // Separate acts needed so React state updates between keypresses + await act(async () => { + stdin.write(TerminalKeys.DOWN_ARROW); + }); + await act(async () => { + stdin.write(TerminalKeys.DOWN_ARROW); + }); + + // Rerender with a filtered list where "Number Setting" is now at index 1 + const filteredItems = [items[0], items[2], items[4]]; + rerender( + + + , + ); + + // Verify the dialog hasn't crashed and the items are displayed + await waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Boolean Setting'); + expect(frame).toContain('Number Setting'); + expect(frame).toContain('Extra Setting 4'); + expect(frame).not.toContain('No matches found.'); + }); + + // Press Enter. If focus was preserved, it should be on "Number Setting" (index 1). + // Since it's a number, it enters edit mode (mockOnItemToggle is NOT called). + await act(async () => { + stdin.write(TerminalKeys.ENTER); + }); + + await waitFor(() => { + expect(mockOnItemToggle).not.toHaveBeenCalled(); + }); + }); + + it('should reset focus to the top if the active item is filtered out', async () => { + const items = createMockItems(5); + const { rerender, stdin, lastFrame } = renderDialog({ + items, + maxItemsToShow: 5, + }); + + // Move focus down to item 2 ("Number Setting") + await act(async () => { + stdin.write(TerminalKeys.DOWN_ARROW); + }); + await act(async () => { + stdin.write(TerminalKeys.DOWN_ARROW); + }); + + // Rerender with a filtered list that EXCLUDES "Number Setting" + const filteredItems = [items[0], items[1]]; + rerender( + + + , + ); + + await waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Boolean Setting'); + expect(frame).toContain('String Setting'); + }); + + // Press Enter. Since focus reset to index 0 ("Boolean Setting"), it should toggle. + await act(async () => { + stdin.write(TerminalKeys.ENTER); + }); + + await waitFor(() => { + expect(mockOnItemToggle).toHaveBeenCalledWith( + 'boolean-setting', + expect.anything(), + ); + }); + }); + }); + describe('item interactions', () => { it('should call onItemToggle for boolean items on Enter', async () => { const { stdin } = renderDialog(); @@ -278,7 +400,7 @@ describe('BaseSettingsDialog', () => { }); it('should call onItemToggle for enum items on Enter', async () => { - const items = createMockItems(); + const items = createMockItems(4); // Move enum to first position const enumItem = items.find((i) => i.type === 'enum')!; const { stdin } = renderDialog({ items: [enumItem] }); @@ -297,7 +419,7 @@ describe('BaseSettingsDialog', () => { }); it('should enter edit mode for string items on Enter', async () => { - const items = createMockItems(); + const items = createMockItems(4); const stringItem = items.find((i) => i.type === 'string')!; const { lastFrame, stdin } = renderDialog({ items: [stringItem] }); @@ -315,7 +437,7 @@ describe('BaseSettingsDialog', () => { }); it('should enter edit mode for number items on Enter', async () => { - const items = createMockItems(); + const items = createMockItems(4); const numberItem = items.find((i) => i.type === 'number')!; const { lastFrame, stdin } = renderDialog({ items: [numberItem] }); @@ -350,7 +472,7 @@ describe('BaseSettingsDialog', () => { describe('edit mode', () => { it('should commit edit on Enter', async () => { - const items = createMockItems(); + const items = createMockItems(4); const stringItem = items.find((i) => i.type === 'string')!; const { stdin } = renderDialog({ items: [stringItem] }); @@ -379,7 +501,7 @@ describe('BaseSettingsDialog', () => { }); it('should commit edit on Escape', async () => { - const items = createMockItems(); + const items = createMockItems(4); const stringItem = items.find((i) => i.type === 'string')!; const { stdin } = renderDialog({ items: [stringItem] }); @@ -399,7 +521,7 @@ describe('BaseSettingsDialog', () => { }); it('should commit edit and navigate on Down arrow', async () => { - const items = createMockItems(); + const items = createMockItems(4); const stringItem = items.find((i) => i.type === 'string')!; const numberItem = items.find((i) => i.type === 'number')!; const { stdin } = renderDialog({ items: [stringItem, numberItem] }); @@ -420,7 +542,7 @@ describe('BaseSettingsDialog', () => { }); it('should commit edit and navigate on Up arrow', async () => { - const items = createMockItems(); + const items = createMockItems(4); const stringItem = items.find((i) => i.type === 'string')!; const numberItem = items.find((i) => i.type === 'number')!; const { stdin } = renderDialog({ items: [stringItem, numberItem] }); @@ -446,7 +568,7 @@ describe('BaseSettingsDialog', () => { }); it('should allow number input for number fields', async () => { - const items = createMockItems(); + const items = createMockItems(4); const numberItem = items.find((i) => i.type === 'number')!; const { stdin } = renderDialog({ items: [numberItem] }); @@ -481,7 +603,7 @@ describe('BaseSettingsDialog', () => { }); it('should support quick number entry for number fields', async () => { - const items = createMockItems(); + const items = createMockItems(4); const numberItem = items.find((i) => i.type === 'number')!; const { stdin } = renderDialog({ items: [numberItem] }); diff --git a/packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx b/packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx index b65febaa04..ab18a71aff 100644 --- a/packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx +++ b/packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useState, useEffect, useCallback } from 'react'; +import React, { useState, useEffect, useCallback, useRef } from 'react'; import { Box, Text } from 'ink'; import chalk from 'chalk'; import { theme } from '../../semantic-colors.js'; @@ -137,12 +137,38 @@ export function BaseSettingsDialog({ const [editCursorPos, setEditCursorPos] = useState(0); const [cursorVisible, setCursorVisible] = useState(true); - // Reset active index when items change (e.g., search filter) + const prevItemsRef = useRef(items); + + // Preserve focus when items change (e.g., search filter) useEffect(() => { - if (activeIndex >= items.length) { - setActiveIndex(Math.max(0, items.length - 1)); + const prevItems = prevItemsRef.current; + if (prevItems !== items) { + const prevActiveItem = prevItems[activeIndex]; + if (prevActiveItem) { + const newIndex = items.findIndex((i) => i.key === prevActiveItem.key); + if (newIndex !== -1) { + // Item still exists in the filtered list, keep focus on it + setActiveIndex(newIndex); + // Adjust scroll offset to ensure the item is visible + let newScroll = scrollOffset; + if (newIndex < scrollOffset) newScroll = newIndex; + else if (newIndex >= scrollOffset + maxItemsToShow) + newScroll = newIndex - maxItemsToShow + 1; + + const maxScroll = Math.max(0, items.length - maxItemsToShow); + setScrollOffset(Math.min(newScroll, maxScroll)); + } else { + // Item was filtered out, reset to the top + setActiveIndex(0); + setScrollOffset(0); + } + } else { + setActiveIndex(0); + setScrollOffset(0); + } + prevItemsRef.current = items; } - }, [items.length, activeIndex]); + }, [items, activeIndex, scrollOffset, maxItemsToShow]); // Cursor blink effect useEffect(() => {