diff --git a/packages/cli/src/ui/hooks/useSelectionList.test.ts b/packages/cli/src/ui/hooks/useSelectionList.test.ts index a5c4ea1882..b0d8cdc981 100644 --- a/packages/cli/src/ui/hooks/useSelectionList.test.ts +++ b/packages/cli/src/ui/hooks/useSelectionList.test.ts @@ -744,6 +744,109 @@ describe('useSelectionList', () => { rerender({ items: [] }); expect(result.current.activeIndex).toBe(0); }); + + it('should not reset activeIndex when items are deeply equal', () => { + const initialItems = [ + { value: 'A' }, + { value: 'B', disabled: true }, + { value: 'C' }, + { value: 'D' }, + ]; + + const { result, rerender } = renderHook( + ({ items: testItems }: { items: Array> }) => + useSelectionList({ + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + initialIndex: 2, + items: testItems, + }), + { initialProps: { items: initialItems } }, + ); + + expect(result.current.activeIndex).toBe(2); + + act(() => { + result.current.setActiveIndex(3); + }); + expect(result.current.activeIndex).toBe(3); + + mockOnHighlight.mockClear(); + + // Create new array with same content (deeply equal but not identical) + const newItems = [ + { value: 'A' }, + { value: 'B', disabled: true }, + { value: 'C' }, + { value: 'D' }, + ]; + + rerender({ items: newItems }); + + // Active index should remain the same since items are deeply equal + expect(result.current.activeIndex).toBe(3); + // onHighlight should NOT be called since the index didn't change + expect(mockOnHighlight).not.toHaveBeenCalled(); + }); + + it('should update activeIndex when items change structurally', () => { + const initialItems = [ + { value: 'A' }, + { value: 'B', disabled: true }, + { value: 'C' }, + { value: 'D' }, + ]; + + const { result, rerender } = renderHook( + ({ items: testItems }: { items: Array> }) => + useSelectionList({ + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + initialIndex: 3, + items: testItems, + }), + { initialProps: { items: initialItems } }, + ); + + expect(result.current.activeIndex).toBe(3); + mockOnHighlight.mockClear(); + + // Change item values (not deeply equal) + const newItems = [{ value: 'X' }, { value: 'Y' }, { value: 'Z' }]; + + rerender({ items: newItems }); + + // Active index should update based on initialIndex and new items + expect(result.current.activeIndex).toBe(0); + }); + + it('should handle partial changes in items array', () => { + const initialItems = [{ value: 'A' }, { value: 'B' }, { value: 'C' }]; + + const { result, rerender } = renderHook( + ({ items: testItems }: { items: Array> }) => + useSelectionList({ + onSelect: mockOnSelect, + initialIndex: 1, + items: testItems, + }), + { initialProps: { items: initialItems } }, + ); + + expect(result.current.activeIndex).toBe(1); + + // Change only one item's disabled status + const newItems = [ + { value: 'A' }, + { value: 'B', disabled: true }, + { value: 'C' }, + ]; + + rerender({ items: newItems }); + + // Should find next valid index since current became disabled + expect(result.current.activeIndex).toBe(2); + }); }); describe('Manual Control', () => { diff --git a/packages/cli/src/ui/hooks/useSelectionList.ts b/packages/cli/src/ui/hooks/useSelectionList.ts index 51d625dd4c..2ac45a73f8 100644 --- a/packages/cli/src/ui/hooks/useSelectionList.ts +++ b/packages/cli/src/ui/hooks/useSelectionList.ts @@ -26,10 +26,12 @@ export interface UseSelectionListResult { setActiveIndex: (index: number) => void; } -interface SelectionListState { +interface SelectionListState { activeIndex: number; + initialIndex: number; pendingHighlight: boolean; pendingSelect: boolean; + items: Array>; } type SelectionListAction = @@ -68,6 +70,27 @@ type SelectionListAction = const NUMBER_INPUT_TIMEOUT_MS = 1000; +/** + * Performs an equality check on two arrays of SelectionListItem. + * + * It compares the length of the arrays and then the 'value' and 'disabled' + * properties of each item. + */ +const areItemsEqual = ( + a: Array>, + b: Array>, +): boolean => { + if (a.length !== b.length) { + return false; + } + for (let i = 0; i < a.length; i++) { + if (a[i]!.value !== b[i]!.value || a[i]!.disabled !== b[i]!.disabled) { + return false; + } + } + return true; +}; + /** * Helper function to find the next enabled index in a given direction, supporting wrapping. */ @@ -96,10 +119,32 @@ const findNextValidIndex = ( return currentIndex; }; +const computeInitialIndex = ( + initialIndex: number, + items: Array>, +): number => { + if (items.length === 0) { + return 0; + } + + let targetIndex = initialIndex; + + if (targetIndex < 0 || targetIndex >= items.length) { + targetIndex = 0; + } + + if (items[targetIndex]?.disabled) { + const nextValid = findNextValidIndex(targetIndex, 'down', items); + targetIndex = nextValid; + } + + return targetIndex; +}; + function selectionListReducer( - state: SelectionListState, + state: SelectionListState, action: SelectionListAction, -): SelectionListState { +): SelectionListState { switch (action.type) { case 'SET_ACTIVE_INDEX': { const { index, items } = action.payload; @@ -139,30 +184,20 @@ function selectionListReducer( case 'INITIALIZE': { const { initialIndex, items } = action.payload; - - if (items.length === 0) { - const newIndex = 0; - return newIndex === state.activeIndex - ? state - : { ...state, activeIndex: newIndex }; + if ( + state.initialIndex === initialIndex && + areItemsEqual(state.items, items) + ) { + return state; } + const targetIndex = computeInitialIndex(initialIndex, items); - let targetIndex = initialIndex; - - if (targetIndex < 0 || targetIndex >= items.length) { - targetIndex = 0; - } - - if (items[targetIndex]?.disabled) { - const nextValid = findNextValidIndex(targetIndex, 'down', items); - targetIndex = nextValid; - } - - // Only return new state if activeIndex actually changed - // Don't set pendingHighlight on initialization - return targetIndex === state.activeIndex - ? state - : { ...state, activeIndex: targetIndex, pendingHighlight: false }; + return { + ...state, + items, + activeIndex: targetIndex, + pendingHighlight: false, + }; } case 'CLEAR_PENDING_FLAGS': { @@ -201,9 +236,11 @@ export function useSelectionList({ showNumbers = false, }: UseSelectionListOptions): UseSelectionListResult { const [state, dispatch] = useReducer(selectionListReducer, { - activeIndex: initialIndex, + activeIndex: computeInitialIndex(initialIndex, items), + initialIndex, pendingHighlight: false, pendingSelect: false, + items, }); const numberInputRef = useRef(''); const numberInputTimer = useRef(null);