diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx index f4d2547319..919c025f4e 100644 --- a/packages/cli/src/ui/components/SettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx @@ -1052,7 +1052,7 @@ describe('SettingsDialog', () => { expect(lastFrame()).toContain('Settings'); // Title expect(lastFrame()).toContain('● Vim Mode'); // Active setting expect(lastFrame()).toContain('Apply To'); // Scope section - expect(lastFrame()).toContain('1. User Settings'); // Scope options + expect(lastFrame()).toContain('User Settings'); // Scope options (no numbers when settings focused) expect(lastFrame()).toContain( '(Use Enter to select, Tab to change focus)', ); // Help text diff --git a/packages/cli/src/ui/components/SettingsDialog.tsx b/packages/cli/src/ui/components/SettingsDialog.tsx index 0d625d8554..d58fe3ea13 100644 --- a/packages/cli/src/ui/components/SettingsDialog.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.tsx @@ -880,7 +880,9 @@ export function SettingsDialog({ item.value === selectedScope, + )} onSelect={handleScopeSelect} onHighlight={handleScopeHighlight} isFocused={focusSection === 'scope'} diff --git a/packages/cli/src/ui/components/__snapshots__/SettingsDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/SettingsDialog.test.tsx.snap index e2a51c1f00..bf1515b9c8 100644 --- a/packages/cli/src/ui/components/__snapshots__/SettingsDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/SettingsDialog.test.tsx.snap @@ -26,9 +26,9 @@ exports[`SettingsDialog > Snapshot Tests > should render default state correctly │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -61,9 +61,9 @@ exports[`SettingsDialog > Snapshot Tests > should render focused on scope select │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -96,9 +96,9 @@ exports[`SettingsDialog > Snapshot Tests > should render with accessibility sett │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -131,9 +131,9 @@ exports[`SettingsDialog > Snapshot Tests > should render with all boolean settin │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -166,9 +166,9 @@ exports[`SettingsDialog > Snapshot Tests > should render with different scope se │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -201,9 +201,9 @@ exports[`SettingsDialog > Snapshot Tests > should render with different scope se │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -236,9 +236,9 @@ exports[`SettingsDialog > Snapshot Tests > should render with file filtering set │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -271,9 +271,9 @@ exports[`SettingsDialog > Snapshot Tests > should render with mixed boolean and │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -306,9 +306,9 @@ exports[`SettingsDialog > Snapshot Tests > should render with tools and security │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ @@ -341,9 +341,9 @@ exports[`SettingsDialog > Snapshot Tests > should render with various boolean se │ │ │ │ │ Apply To │ -│ ● 1. User Settings │ -│ 2. Workspace Settings │ -│ 3. System Settings │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ │ │ │ (Use Enter to select, Tab to change focus) │ │ │ diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx new file mode 100644 index 0000000000..ebe52e0d27 --- /dev/null +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx @@ -0,0 +1,521 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { waitFor } from '@testing-library/react'; +import { renderWithProviders } from '../../../test-utils/render.js'; +import { + BaseSelectionList, + type BaseSelectionListProps, + type RenderItemContext, +} from './BaseSelectionList.js'; +import { useSelectionList } from '../../hooks/useSelectionList.js'; +import { Text } from 'ink'; +import type { theme } from '../../semantic-colors.js'; + +vi.mock('../../hooks/useSelectionList.js'); + +const mockTheme = { + text: { primary: 'COLOR_PRIMARY', secondary: 'COLOR_SECONDARY' }, + status: { success: 'COLOR_SUCCESS' }, +} as typeof theme; + +vi.mock('../../semantic-colors.js', () => ({ + theme: { + text: { primary: 'COLOR_PRIMARY', secondary: 'COLOR_SECONDARY' }, + status: { success: 'COLOR_SUCCESS' }, + }, +})); + +describe('BaseSelectionList', () => { + const mockOnSelect = vi.fn(); + const mockOnHighlight = vi.fn(); + const mockRenderItem = vi.fn(); + + // Define standard test items + const items = [ + { value: 'A', label: 'Item A' }, + { value: 'B', label: 'Item B', disabled: true }, + { value: 'C', label: 'Item C' }, + ]; + + // Helper to render the component with default props + const renderComponent = ( + props: Partial> = {}, + activeIndex: number = 0, + ) => { + vi.mocked(useSelectionList).mockReturnValue({ + activeIndex, + setActiveIndex: vi.fn(), + }); + + mockRenderItem.mockImplementation( + (item: (typeof items)[0], context: RenderItemContext) => ( + {item.label} + ), + ); + + const defaultProps: BaseSelectionListProps = { + items, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + renderItem: mockRenderItem, + ...props, + }; + + return renderWithProviders(); + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('Rendering and Structure', () => { + it('should render all items using the renderItem prop', () => { + const { lastFrame } = renderComponent(); + + expect(lastFrame()).toContain('Item A'); + expect(lastFrame()).toContain('Item B'); + expect(lastFrame()).toContain('Item C'); + + expect(mockRenderItem).toHaveBeenCalledTimes(3); + expect(mockRenderItem).toHaveBeenCalledWith(items[0], expect.any(Object)); + }); + + it('should render the selection indicator (● or space) and layout', () => { + const { lastFrame } = renderComponent({}, 0); + const output = lastFrame(); + + // Use regex to assert the structure: Indicator + Whitespace + Number + Label + expect(output).toMatch(/●\s+1\.\s+Item A/); + expect(output).toMatch(/\s+2\.\s+Item B/); + expect(output).toMatch(/\s+3\.\s+Item C/); + }); + + it('should handle an empty list gracefully', () => { + const { lastFrame } = renderComponent({ items: [] }); + expect(mockRenderItem).not.toHaveBeenCalled(); + expect(lastFrame()).toBe(''); + }); + }); + + describe('useSelectionList Integration', () => { + it('should pass props correctly to useSelectionList', () => { + const initialIndex = 1; + const isFocused = false; + const showNumbers = false; + + renderComponent({ initialIndex, isFocused, showNumbers }); + + expect(useSelectionList).toHaveBeenCalledWith({ + items, + initialIndex, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + isFocused, + showNumbers, + }); + }); + + it('should use the activeIndex returned by the hook', () => { + renderComponent({}, 2); // Active index is C + + expect(mockRenderItem).toHaveBeenCalledWith( + items[0], + expect.objectContaining({ isSelected: false }), + ); + expect(mockRenderItem).toHaveBeenCalledWith( + items[2], + expect.objectContaining({ isSelected: true }), + ); + }); + }); + + describe('Styling and Colors', () => { + it('should apply success color to the selected item', () => { + renderComponent({}, 0); // Item A selected + + // Check renderItem context colors against the mocked theme + expect(mockRenderItem).toHaveBeenCalledWith( + items[0], + expect.objectContaining({ + titleColor: mockTheme.status.success, + numberColor: mockTheme.status.success, + isSelected: true, + }), + ); + }); + + it('should apply primary color to unselected, enabled items', () => { + renderComponent({}, 0); // Item A selected, Item C unselected/enabled + + // Check renderItem context colors for Item C + expect(mockRenderItem).toHaveBeenCalledWith( + items[2], + expect.objectContaining({ + titleColor: mockTheme.text.primary, + numberColor: mockTheme.text.primary, + isSelected: false, + }), + ); + }); + + it('should apply secondary color to disabled items (when not selected)', () => { + renderComponent({}, 0); // Item A selected, Item B disabled + + // Check renderItem context colors for Item B + expect(mockRenderItem).toHaveBeenCalledWith( + items[1], + expect.objectContaining({ + titleColor: mockTheme.text.secondary, + numberColor: mockTheme.text.secondary, + isSelected: false, + }), + ); + }); + + it('should apply success color to disabled items if they are selected', () => { + // The component should visually reflect the selection even if the item is disabled. + renderComponent({}, 1); // Item B (disabled) selected + + // Check renderItem context colors for Item B + expect(mockRenderItem).toHaveBeenCalledWith( + items[1], + expect.objectContaining({ + titleColor: mockTheme.status.success, + numberColor: mockTheme.status.success, + isSelected: true, + }), + ); + }); + }); + + describe('Numbering (showNumbers)', () => { + it('should show numbers by default with correct formatting', () => { + const { lastFrame } = renderComponent(); + const output = lastFrame(); + + expect(output).toContain('1.'); + expect(output).toContain('2.'); + expect(output).toContain('3.'); + }); + + it('should hide numbers when showNumbers is false', () => { + const { lastFrame } = renderComponent({ showNumbers: false }); + const output = lastFrame(); + + expect(output).not.toContain('1.'); + expect(output).not.toContain('2.'); + expect(output).not.toContain('3.'); + }); + + it('should apply correct padding for alignment in long lists', () => { + const longList = Array.from({ length: 15 }, (_, i) => ({ + value: `Item ${i + 1}`, + label: `Item ${i + 1}`, + })); + + // We must increase maxItemsToShow (default 10) to see the 10th item and beyond + const { lastFrame } = renderComponent({ + items: longList, + maxItemsToShow: 15, + }); + const output = lastFrame(); + + // Check formatting for single and double digits. + // The implementation uses padStart, resulting in " 1." and "10.". + expect(output).toContain(' 1.'); + expect(output).toContain('10.'); + }); + + it('should apply secondary color to numbers if showNumbers is false (internal logic check)', () => { + renderComponent({ showNumbers: false }, 0); + + expect(mockRenderItem).toHaveBeenCalledWith( + items[0], + expect.objectContaining({ + isSelected: true, + titleColor: mockTheme.status.success, + numberColor: mockTheme.text.secondary, + }), + ); + }); + }); + + describe('Scrolling and Pagination (maxItemsToShow)', () => { + const longList = Array.from({ length: 10 }, (_, i) => ({ + value: `Item ${i + 1}`, + label: `Item ${i + 1}`, + })); + const MAX_ITEMS = 3; + + const renderScrollableList = (initialActiveIndex: number = 0) => { + // Define the props used for the initial render and subsequent rerenders + const componentProps: BaseSelectionListProps = + { + items: longList, + maxItemsToShow: MAX_ITEMS, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + renderItem: mockRenderItem, + }; + + vi.mocked(useSelectionList).mockReturnValue({ + activeIndex: initialActiveIndex, + setActiveIndex: vi.fn(), + }); + + mockRenderItem.mockImplementation( + (item: (typeof longList)[0], context: RenderItemContext) => ( + {item.label} + ), + ); + + const { rerender, lastFrame } = renderWithProviders( + , + ); + + // Function to simulate the activeIndex changing over time + const updateActiveIndex = async (newIndex: number) => { + vi.mocked(useSelectionList).mockReturnValue({ + activeIndex: newIndex, + setActiveIndex: vi.fn(), + }); + + rerender(); + + await waitFor(() => { + expect(lastFrame()).toBeTruthy(); + }); + }; + + return { updateActiveIndex, lastFrame }; + }; + + it('should only show maxItemsToShow items initially', () => { + const { lastFrame } = renderScrollableList(0); + const output = lastFrame(); + + expect(output).toContain('Item 1'); + expect(output).toContain('Item 3'); + expect(output).not.toContain('Item 4'); + }); + + it('should scroll down when activeIndex moves beyond the visible window', async () => { + const { updateActiveIndex, lastFrame } = renderScrollableList(0); + + // Move to index 3 (Item 4). Should trigger scroll. + // New visible window should be Items 2, 3, 4 (scroll offset 1). + await updateActiveIndex(3); + + const output = lastFrame(); + expect(output).not.toContain('Item 1'); + expect(output).toContain('Item 2'); + expect(output).toContain('Item 4'); + expect(output).not.toContain('Item 5'); + }); + + it('should scroll up when activeIndex moves before the visible window', async () => { + const { updateActiveIndex, lastFrame } = renderScrollableList(0); + + await updateActiveIndex(4); + + let output = lastFrame(); + expect(output).toContain('Item 3'); // Should see items 3, 4, 5 + expect(output).toContain('Item 5'); + expect(output).not.toContain('Item 2'); + + // Now test scrolling up: move to index 1 (Item 2) + // This should trigger scroll up to show items 2, 3, 4 + await updateActiveIndex(1); + + output = lastFrame(); + expect(output).toContain('Item 2'); + expect(output).toContain('Item 4'); + expect(output).not.toContain('Item 5'); // Item 5 should no longer be visible + }); + + it('should pin the scroll offset to the end if selection starts near the end', async () => { + // List length 10. Max items 3. Active index 9 (last item). + // Scroll offset should be 10 - 3 = 7. + // Visible items: 8, 9, 10. + const { lastFrame } = renderScrollableList(9); + + await waitFor(() => { + const output = lastFrame(); + expect(output).toContain('Item 10'); + expect(output).toContain('Item 8'); + expect(output).not.toContain('Item 7'); + }); + }); + + it('should handle dynamic scrolling through multiple activeIndex changes', async () => { + const { updateActiveIndex, lastFrame } = renderScrollableList(0); + + expect(lastFrame()).toContain('Item 1'); + expect(lastFrame()).toContain('Item 3'); + + // Scroll down gradually + await updateActiveIndex(2); // Still within window + expect(lastFrame()).toContain('Item 1'); + + await updateActiveIndex(3); // Should trigger scroll + let output = lastFrame(); + expect(output).toContain('Item 2'); + expect(output).toContain('Item 4'); + expect(output).not.toContain('Item 1'); + + await updateActiveIndex(5); // Scroll further + output = lastFrame(); + expect(output).toContain('Item 4'); + expect(output).toContain('Item 6'); + expect(output).not.toContain('Item 3'); + }); + + it('should correctly identify the selected item within the visible window', () => { + renderScrollableList(1); // activeIndex 1 = Item 2 + + expect(mockRenderItem).toHaveBeenCalledTimes(MAX_ITEMS); + + expect(mockRenderItem).toHaveBeenCalledWith( + expect.objectContaining({ value: 'Item 1' }), + expect.objectContaining({ isSelected: false }), + ); + + expect(mockRenderItem).toHaveBeenCalledWith( + expect.objectContaining({ value: 'Item 2' }), + expect.objectContaining({ isSelected: true }), + ); + }); + + it('should correctly identify the selected item when scrolled (high index)', async () => { + renderScrollableList(5); + + await waitFor(() => { + // Item 6 (index 5) should be selected + expect(mockRenderItem).toHaveBeenCalledWith( + expect.objectContaining({ value: 'Item 6' }), + expect.objectContaining({ isSelected: true }), + ); + }); + + // Item 4 (index 3) should not be selected + expect(mockRenderItem).toHaveBeenCalledWith( + expect.objectContaining({ value: 'Item 4' }), + expect.objectContaining({ isSelected: false }), + ); + }); + + it('should handle maxItemsToShow larger than the list length', () => { + // Test edge case where maxItemsToShow exceeds available items + const { lastFrame } = renderComponent( + { items: longList, maxItemsToShow: 15 }, + 0, + ); + const output = lastFrame(); + + // Should show all available items (10 items) + expect(output).toContain('Item 1'); + expect(output).toContain('Item 10'); + expect(mockRenderItem).toHaveBeenCalledTimes(10); + }); + }); + + describe('Scroll Arrows (showScrollArrows)', () => { + const longList = Array.from({ length: 10 }, (_, i) => ({ + value: `Item ${i + 1}`, + label: `Item ${i + 1}`, + })); + const MAX_ITEMS = 3; + + it('should not show arrows by default', () => { + const { lastFrame } = renderComponent({ + items: longList, + maxItemsToShow: MAX_ITEMS, + }); + const output = lastFrame(); + + expect(output).not.toContain('▲'); + expect(output).not.toContain('▼'); + }); + + it('should show arrows with correct colors when enabled (at the top)', async () => { + const { lastFrame } = renderComponent( + { + items: longList, + maxItemsToShow: MAX_ITEMS, + showScrollArrows: true, + }, + 0, + ); + + await waitFor(() => { + const output = lastFrame(); + // At the top, should show first 3 items + expect(output).toContain('Item 1'); + expect(output).toContain('Item 3'); + expect(output).not.toContain('Item 4'); + // Both arrows should be visible + expect(output).toContain('▲'); + expect(output).toContain('▼'); + }); + }); + + it('should show arrows and correct items when scrolled to the middle', async () => { + const { lastFrame } = renderComponent( + { items: longList, maxItemsToShow: MAX_ITEMS, showScrollArrows: true }, + 5, + ); + + await waitFor(() => { + const output = lastFrame(); + // After scrolling to middle, should see items around index 5 + expect(output).toContain('Item 4'); + expect(output).toContain('Item 6'); + expect(output).not.toContain('Item 3'); + expect(output).not.toContain('Item 7'); + // Both scroll arrows should be visible + expect(output).toContain('▲'); + expect(output).toContain('▼'); + }); + }); + + it('should show arrows and correct items when scrolled to the end', async () => { + const { lastFrame } = renderComponent( + { items: longList, maxItemsToShow: MAX_ITEMS, showScrollArrows: true }, + 9, + ); + + await waitFor(() => { + const output = lastFrame(); + // At the end, should show last 3 items + expect(output).toContain('Item 8'); + expect(output).toContain('Item 10'); + expect(output).not.toContain('Item 7'); + // Both arrows should be visible + expect(output).toContain('▲'); + expect(output).toContain('▼'); + }); + }); + + it('should show both arrows dimmed when list fits entirely', () => { + const { lastFrame } = renderComponent({ + items, + maxItemsToShow: 5, + showScrollArrows: true, + }); + + const output = lastFrame(); + // Should show all items since maxItemsToShow > items.length + expect(output).toContain('Item A'); + expect(output).toContain('Item B'); + expect(output).toContain('Item C'); + // Both arrows should be visible but dimmed (this test doesn't need waitFor since no scrolling occurs) + expect(output).toContain('▲'); + expect(output).toContain('▼'); + }); + }); +}); diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx new file mode 100644 index 0000000000..94c63271cb --- /dev/null +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx @@ -0,0 +1,174 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type React from 'react'; +import { useEffect, useState } from 'react'; +import { Text, Box } from 'ink'; +import { theme } from '../../semantic-colors.js'; +import { useSelectionList } from '../../hooks/useSelectionList.js'; + +export interface RenderItemContext { + isSelected: boolean; + titleColor: string; + numberColor: string; +} + +export interface BaseSelectionListProps> { + items: Array; + initialIndex?: number; + onSelect: (value: T) => void; + onHighlight?: (value: T) => void; + isFocused?: boolean; + showNumbers?: boolean; + showScrollArrows?: boolean; + maxItemsToShow?: number; + renderItem: ( + item: TItem & { value: T; disabled?: boolean }, + context: RenderItemContext, + ) => React.ReactNode; +} + +/** + * Base component for selection lists that provides common UI structure + * and keyboard navigation logic via the useSelectionList hook. + * + * This component handles: + * - Radio button indicators + * - Item numbering + * - Scrolling for long lists + * - Color theming based on selection/disabled state + * - Keyboard navigation and numeric selection + * + * Specific components should use this as a base and provide + * their own renderItem implementation for custom content. + */ +export function BaseSelectionList>({ + items, + initialIndex = 0, + onSelect, + onHighlight, + isFocused = true, + showNumbers = true, + showScrollArrows = false, + maxItemsToShow = 10, + renderItem, +}: BaseSelectionListProps): React.JSX.Element { + const { activeIndex } = useSelectionList({ + items, + initialIndex, + onSelect, + onHighlight, + isFocused, + showNumbers, + }); + + const [scrollOffset, setScrollOffset] = useState(0); + + // Handle scrolling for long lists + useEffect(() => { + const newScrollOffset = Math.max( + 0, + Math.min(activeIndex - maxItemsToShow + 1, items.length - maxItemsToShow), + ); + if (activeIndex < scrollOffset) { + setScrollOffset(activeIndex); + } else if (activeIndex >= scrollOffset + maxItemsToShow) { + setScrollOffset(newScrollOffset); + } + }, [activeIndex, items.length, scrollOffset, maxItemsToShow]); + + const visibleItems = items.slice(scrollOffset, scrollOffset + maxItemsToShow); + const numberColumnWidth = String(items.length).length; + + return ( + + {/* Use conditional coloring instead of conditional rendering */} + {showScrollArrows && ( + 0 ? theme.text.primary : theme.text.secondary} + > + ▲ + + )} + + {visibleItems.map((item, index) => { + const itemIndex = scrollOffset + index; + const isSelected = activeIndex === itemIndex; + + // Determine colors based on selection and disabled state + let titleColor = theme.text.primary; + let numberColor = theme.text.primary; + + if (isSelected) { + titleColor = theme.status.success; + numberColor = theme.status.success; + } else if (item.disabled) { + titleColor = theme.text.secondary; + numberColor = theme.text.secondary; + } + + if (!isFocused && !item.disabled) { + numberColor = theme.text.secondary; + } + + if (!showNumbers) { + numberColor = theme.text.secondary; + } + + const itemNumberText = `${String(itemIndex + 1).padStart( + numberColumnWidth, + )}.`; + + return ( + + {/* Radio button indicator */} + + + {isSelected ? '●' : ' '} + + + + {/* Item number */} + {showNumbers && ( + + {itemNumberText} + + )} + + {/* Custom content via render prop */} + + {renderItem(item, { + isSelected, + titleColor, + numberColor, + })} + + + ); + })} + + {showScrollArrows && ( + + ▼ + + )} + + ); +} diff --git a/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx b/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx index cb6db77f5e..b4595dc0b4 100644 --- a/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx +++ b/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx @@ -4,178 +4,193 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { renderWithProviders } from '../../../test-utils/render.js'; -import { waitFor } from '@testing-library/react'; +import type React from 'react'; import { RadioButtonSelect, type RadioSelectItem, + type RadioButtonSelectProps, } from './RadioButtonSelect.js'; -import { describe, it, expect, vi } from 'vitest'; +import { + BaseSelectionList, + type BaseSelectionListProps, + type RenderItemContext, +} from './BaseSelectionList.js'; -const ITEMS: Array> = [ - { label: 'Option 1', value: 'one' }, - { label: 'Option 2', value: 'two' }, - { label: 'Option 3', value: 'three', disabled: true }, -]; +vi.mock('./BaseSelectionList.js', () => ({ + BaseSelectionList: vi.fn(() => null), +})); -describe('', () => { - it('renders a list of items and matches snapshot', () => { - const { lastFrame } = renderWithProviders( - {}} isFocused={true} />, +vi.mock('../../semantic-colors.js', () => ({ + theme: { + text: { secondary: 'COLOR_SECONDARY' }, + }, +})); + +const MockedBaseSelectionList = vi.mocked( + BaseSelectionList, +) as unknown as ReturnType; + +type RadioRenderItemFn = ( + item: RadioSelectItem, + context: RenderItemContext, +) => React.JSX.Element; +const extractRenderItem = (): RadioRenderItemFn => { + const mockCalls = MockedBaseSelectionList.mock.calls; + + if (mockCalls.length === 0) { + throw new Error( + 'BaseSelectionList was not called. Ensure RadioButtonSelect is rendered before calling extractRenderItem.', ); - expect(lastFrame()).toMatchSnapshot(); + } + + const props = mockCalls[0][0] as BaseSelectionListProps< + string, + RadioSelectItem + >; + + if (typeof props.renderItem !== 'function') { + throw new Error('renderItem prop was not found on BaseSelectionList call.'); + } + + return props.renderItem as RadioRenderItemFn; +}; + +describe('RadioButtonSelect', () => { + const mockOnSelect = vi.fn(); + const mockOnHighlight = vi.fn(); + + const ITEMS: Array> = [ + { label: 'Option 1', value: 'one' }, + { label: 'Option 2', value: 'two' }, + { label: 'Option 3', value: 'three', disabled: true }, + ]; + + const renderComponent = ( + props: Partial> = {}, + ) => { + const defaultProps: RadioButtonSelectProps = { + items: ITEMS, + onSelect: mockOnSelect, + ...props, + }; + return renderWithProviders(); + }; + + beforeEach(() => { + vi.clearAllMocks(); }); - it('renders with the second item selected and matches snapshot', () => { - const { lastFrame } = renderWithProviders( - {}} />, - ); - expect(lastFrame()).toMatchSnapshot(); + describe('Prop forwarding to BaseSelectionList', () => { + it('should forward all props correctly when provided', () => { + const props = { + items: ITEMS, + initialIndex: 1, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + isFocused: false, + showScrollArrows: true, + maxItemsToShow: 5, + showNumbers: false, + }; + + renderComponent(props); + + expect(BaseSelectionList).toHaveBeenCalledTimes(1); + expect(BaseSelectionList).toHaveBeenCalledWith( + expect.objectContaining({ + ...props, + renderItem: expect.any(Function), + }), + undefined, + ); + }); + + it('should use default props if not provided', () => { + renderComponent({ + items: ITEMS, + onSelect: mockOnSelect, + }); + + expect(BaseSelectionList).toHaveBeenCalledWith( + expect.objectContaining({ + initialIndex: 0, + isFocused: true, + showScrollArrows: false, + maxItemsToShow: 10, + showNumbers: true, + }), + undefined, + ); + }); }); - it('renders with numbers hidden and matches snapshot', () => { - const { lastFrame } = renderWithProviders( - {}} - showNumbers={false} - />, - ); - expect(lastFrame()).toMatchSnapshot(); - }); + describe('renderItem implementation', () => { + let renderItem: RadioRenderItemFn; + const mockContext: RenderItemContext = { + isSelected: false, + titleColor: 'MOCK_TITLE_COLOR', + numberColor: 'MOCK_NUMBER_COLOR', + }; - it('renders with scroll arrows and matches snapshot', () => { - const manyItems = Array.from({ length: 20 }, (_, i) => ({ - label: `Item ${i + 1}`, - value: `item-${i + 1}`, - })); - const { lastFrame } = renderWithProviders( - {}} - showScrollArrows={true} - maxItemsToShow={5} - />, - ); - expect(lastFrame()).toMatchSnapshot(); - }); + beforeEach(() => { + renderComponent(); + renderItem = extractRenderItem(); + }); - it('renders with special theme display and matches snapshot', () => { - const themeItems: Array> = [ - { + it('should render the standard label display with correct color and truncation', () => { + const item = ITEMS[0]; + + const result = renderItem(item, mockContext); + + expect(result?.props?.color).toBe(mockContext.titleColor); + expect(result?.props?.children).toBe('Option 1'); + expect(result?.props?.wrap).toBe('truncate'); + }); + + it('should render the special theme display when theme props are present', () => { + const themeItem: RadioSelectItem = { label: 'Theme A (Light)', value: 'a-light', themeNameDisplay: 'Theme A', themeTypeDisplay: '(Light)', - }, - { - label: 'Theme B (Dark)', - value: 'b-dark', - themeNameDisplay: 'Theme B', - themeTypeDisplay: '(Dark)', - }, - ]; - const { lastFrame } = renderWithProviders( - {}} />, - ); - expect(lastFrame()).toMatchSnapshot(); - }); + }; - it('renders a list with >10 items and matches snapshot', () => { - const manyItems = Array.from({ length: 12 }, (_, i) => ({ - label: `Item ${i + 1}`, - value: `item-${i + 1}`, - })); - const { lastFrame } = renderWithProviders( - {}} />, - ); - expect(lastFrame()).toMatchSnapshot(); - }); + const result = renderItem(themeItem, mockContext); - it('renders nothing when no items are provided', () => { - const { lastFrame } = renderWithProviders( - {}} isFocused={true} />, - ); - expect(lastFrame()).toBe(''); - }); -}); + expect(result?.props?.color).toBe(mockContext.titleColor); + expect(result?.props?.wrap).toBe('truncate'); -describe('keyboard navigation', () => { - it('should call onSelect when "enter" is pressed', () => { - const onSelect = vi.fn(); - const { stdin } = renderWithProviders( - , - ); + const children = result?.props?.children; - stdin.write('\r'); + if (!Array.isArray(children) || children.length < 3) { + throw new Error( + 'Expected children to be an array with at least 3 elements for theme display', + ); + } - expect(onSelect).toHaveBeenCalledWith('one'); - }); + expect(children[0]).toBe('Theme A'); + expect(children[1]).toBe(' '); - describe('when isFocused is false', () => { - it('should not handle any keyboard input', () => { - const onSelect = vi.fn(); - const { stdin } = renderWithProviders( - , - ); - - stdin.write('\u001B[B'); // Down arrow - stdin.write('\u001B[A'); // Up arrow - stdin.write('\r'); // Enter - - expect(onSelect).not.toHaveBeenCalled(); - }); - }); - - describe.each([ - { description: 'when isFocused is true', isFocused: true }, - { description: 'when isFocused is omitted', isFocused: undefined }, - ])('$description', ({ isFocused }) => { - it('should navigate down with arrow key and select with enter', async () => { - const onSelect = vi.fn(); - const { stdin, lastFrame } = renderWithProviders( - , - ); - - stdin.write('\u001B[B'); // Down arrow - - await waitFor(() => { - expect(lastFrame()).toContain('● 2. Option 2'); - }); - - stdin.write('\r'); - - expect(onSelect).toHaveBeenCalledWith('two'); + const nestedTextElement = children[2] as React.ReactElement<{ + color?: string; + children?: React.ReactNode; + }>; + expect(nestedTextElement?.props?.color).toBe('COLOR_SECONDARY'); + expect(nestedTextElement?.props?.children).toBe('(Light)'); }); - it('should navigate up with arrow key and select with enter', async () => { - const onSelect = vi.fn(); - const { stdin, lastFrame } = renderWithProviders( - , - ); + it('should fall back to standard display if only one theme prop is present', () => { + const partialThemeItem: RadioSelectItem = { + label: 'Incomplete Theme', + value: 'incomplete', + themeNameDisplay: 'Only Name', + }; - stdin.write('\u001B[A'); // Up arrow + const result = renderItem(partialThemeItem, mockContext); - await waitFor(() => { - expect(lastFrame()).toContain('● 1. Option 1'); - }); - - stdin.write('\r'); - - expect(onSelect).toHaveBeenCalledWith('one'); + expect(result?.props?.children).toBe('Incomplete Theme'); }); }); }); diff --git a/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx b/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx index ab62e5d1b1..c3ea07c30c 100644 --- a/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx +++ b/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx @@ -5,10 +5,9 @@ */ import type React from 'react'; -import { useEffect, useState, useRef } from 'react'; -import { Text, Box } from 'ink'; +import { Text } from 'ink'; import { theme } from '../../semantic-colors.js'; -import { useKeypress } from '../../hooks/useKeypress.js'; +import { BaseSelectionList } from './BaseSelectionList.js'; /** * Represents a single option for the RadioButtonSelect. @@ -61,182 +60,33 @@ export function RadioButtonSelect({ maxItemsToShow = 10, showNumbers = true, }: RadioButtonSelectProps): React.JSX.Element { - const [activeIndex, setActiveIndex] = useState(initialIndex); - const [scrollOffset, setScrollOffset] = useState(0); - const [numberInput, setNumberInput] = useState(''); - const numberInputTimer = useRef(null); - useEffect(() => { - const newScrollOffset = Math.max( - 0, - Math.min(activeIndex - maxItemsToShow + 1, items.length - maxItemsToShow), - ); - if (activeIndex < scrollOffset) { - setScrollOffset(activeIndex); - } else if (activeIndex >= scrollOffset + maxItemsToShow) { - setScrollOffset(newScrollOffset); - } - }, [activeIndex, items.length, scrollOffset, maxItemsToShow]); - - useEffect( - () => () => { - if (numberInputTimer.current) { - clearTimeout(numberInputTimer.current); - } - }, - [], - ); - - useKeypress( - (key) => { - const { sequence, name } = key; - const isNumeric = showNumbers && /^[0-9]$/.test(sequence); - - // Any key press that is not a digit should clear the number input buffer. - if (!isNumeric && numberInputTimer.current) { - clearTimeout(numberInputTimer.current); - setNumberInput(''); - } - - if (name === 'k' || name === 'up') { - const newIndex = activeIndex > 0 ? activeIndex - 1 : items.length - 1; - setActiveIndex(newIndex); - onHighlight?.(items[newIndex]!.value); - return; - } - - if (name === 'j' || name === 'down') { - const newIndex = activeIndex < items.length - 1 ? activeIndex + 1 : 0; - setActiveIndex(newIndex); - onHighlight?.(items[newIndex]!.value); - return; - } - - if (name === 'return') { - onSelect(items[activeIndex]!.value); - return; - } - - // Handle numeric input for selection. - if (isNumeric) { - if (numberInputTimer.current) { - clearTimeout(numberInputTimer.current); - } - - const newNumberInput = numberInput + sequence; - setNumberInput(newNumberInput); - - const targetIndex = Number.parseInt(newNumberInput, 10) - 1; - - // A single '0' is not a valid selection since items are 1-indexed. - if (newNumberInput === '0') { - numberInputTimer.current = setTimeout(() => setNumberInput(''), 350); - return; - } - - if (targetIndex >= 0 && targetIndex < items.length) { - const targetItem = items[targetIndex]!; - setActiveIndex(targetIndex); - onHighlight?.(targetItem.value); - - // If the typed number can't be a prefix for another valid number, - // select it immediately. Otherwise, wait for more input. - const potentialNextNumber = Number.parseInt(newNumberInput + '0', 10); - if (potentialNextNumber > items.length) { - onSelect(targetItem.value); - setNumberInput(''); - } else { - numberInputTimer.current = setTimeout(() => { - onSelect(targetItem.value); - setNumberInput(''); - }, 350); // Debounce time for multi-digit input. - } - } else { - // The typed number is out of bounds, clear the buffer - setNumberInput(''); - } - } - }, - { isActive: !!(isFocused && items.length > 0) }, - ); - - const visibleItems = items.slice(scrollOffset, scrollOffset + maxItemsToShow); - return ( - - {showScrollArrows && ( - 0 ? theme.text.primary : theme.text.secondary} - > - ▲ - - )} - {visibleItems.map((item, index) => { - const itemIndex = scrollOffset + index; - const isSelected = activeIndex === itemIndex; - - let textColor = theme.text.primary; - let numberColor = theme.text.primary; - if (isSelected) { - textColor = theme.status.success; - numberColor = theme.status.success; - } else if (item.disabled) { - textColor = theme.text.secondary; - numberColor = theme.text.secondary; + > + items={items} + initialIndex={initialIndex} + onSelect={onSelect} + onHighlight={onHighlight} + isFocused={isFocused} + showNumbers={showNumbers} + showScrollArrows={showScrollArrows} + maxItemsToShow={maxItemsToShow} + renderItem={(item, { titleColor }) => { + // Handle special theme display case for ThemeDialog compatibility + if (item.themeNameDisplay && item.themeTypeDisplay) { + return ( + + {item.themeNameDisplay}{' '} + {item.themeTypeDisplay} + + ); } - - if (!showNumbers) { - numberColor = theme.text.secondary; - } - - const numberColumnWidth = String(items.length).length; - const itemNumberText = `${String(itemIndex + 1).padStart( - numberColumnWidth, - )}.`; - + // Regular label display return ( - - - - {isSelected ? '●' : ' '} - - - - {itemNumberText} - - {item.themeNameDisplay && item.themeTypeDisplay ? ( - - {item.themeNameDisplay}{' '} - - {item.themeTypeDisplay} - - - ) : ( - - {item.label} - - )} - + + {item.label} + ); - })} - {showScrollArrows && ( - - ▼ - - )} - + }} + /> ); } diff --git a/packages/cli/src/ui/components/shared/__snapshots__/RadioButtonSelect.test.tsx.snap b/packages/cli/src/ui/components/shared/__snapshots__/RadioButtonSelect.test.tsx.snap deleted file mode 100644 index aeb4ac16eb..0000000000 --- a/packages/cli/src/ui/components/shared/__snapshots__/RadioButtonSelect.test.tsx.snap +++ /dev/null @@ -1,47 +0,0 @@ -// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html - -exports[` > renders a list of items and matches snapshot 1`] = ` -"● 1. Option 1 - 2. Option 2 - 3. Option 3" -`; - -exports[` > renders a list with >10 items and matches snapshot 1`] = ` -"● 1. Item 1 - 2. Item 2 - 3. Item 3 - 4. Item 4 - 5. Item 5 - 6. Item 6 - 7. Item 7 - 8. Item 8 - 9. Item 9 - 10. Item 10" -`; - -exports[` > renders with numbers hidden and matches snapshot 1`] = ` -"● 1. Option 1 - 2. Option 2 - 3. Option 3" -`; - -exports[` > renders with scroll arrows and matches snapshot 1`] = ` -"▲ -● 1. Item 1 - 2. Item 2 - 3. Item 3 - 4. Item 4 - 5. Item 5 -▼" -`; - -exports[` > renders with special theme display and matches snapshot 1`] = ` -"● 1. Theme A (Light) - 2. Theme B (Dark)" -`; - -exports[` > renders with the second item selected and matches snapshot 1`] = ` -" 1. Option 1 -● 2. Option 2 - 3. Option 3" -`; diff --git a/packages/cli/src/ui/hooks/useSelectionList.test.ts b/packages/cli/src/ui/hooks/useSelectionList.test.ts new file mode 100644 index 0000000000..a5c4ea1882 --- /dev/null +++ b/packages/cli/src/ui/hooks/useSelectionList.test.ts @@ -0,0 +1,809 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { + useSelectionList, + type SelectionListItem, +} from './useSelectionList.js'; +import { useKeypress } from './useKeypress.js'; + +import type { KeypressHandler, Key } from '../contexts/KeypressContext.js'; + +type UseKeypressMockOptions = { isActive: boolean }; + +vi.mock('./useKeypress.js'); + +let activeKeypressHandler: KeypressHandler | null = null; + +describe('useSelectionList', () => { + const mockOnSelect = vi.fn(); + const mockOnHighlight = vi.fn(); + + const items: Array> = [ + { value: 'A' }, + { value: 'B', disabled: true }, + { value: 'C' }, + { value: 'D' }, + ]; + + beforeEach(() => { + activeKeypressHandler = null; + vi.mocked(useKeypress).mockImplementation( + (handler: KeypressHandler, options?: UseKeypressMockOptions) => { + if (options?.isActive) { + activeKeypressHandler = handler; + } else { + activeKeypressHandler = null; + } + }, + ); + mockOnSelect.mockClear(); + mockOnHighlight.mockClear(); + }); + + const pressKey = (name: string, sequence: string = name) => { + act(() => { + if (activeKeypressHandler) { + const key: Key = { + name, + sequence, + ctrl: false, + meta: false, + shift: false, + paste: false, + }; + activeKeypressHandler(key); + } else { + throw new Error( + `Test attempted to press key (${name}) but the keypress handler is not active. Ensure the hook is focused (isFocused=true) and the list is not empty.`, + ); + } + }); + }; + + describe('Initialization', () => { + it('should initialize with the default index (0) if enabled', () => { + const { result } = renderHook(() => + useSelectionList({ items, onSelect: mockOnSelect }), + ); + expect(result.current.activeIndex).toBe(0); + }); + + it('should initialize with the provided initialIndex if enabled', () => { + const { result } = renderHook(() => + useSelectionList({ + items, + initialIndex: 2, + onSelect: mockOnSelect, + }), + ); + expect(result.current.activeIndex).toBe(2); + }); + + it('should handle an empty list gracefully', () => { + const { result } = renderHook(() => + useSelectionList({ items: [], onSelect: mockOnSelect }), + ); + expect(result.current.activeIndex).toBe(0); + }); + + it('should find the next enabled item (downwards) if initialIndex is disabled', () => { + const { result } = renderHook(() => + useSelectionList({ + items, + initialIndex: 1, + onSelect: mockOnSelect, + }), + ); + expect(result.current.activeIndex).toBe(2); + }); + + it('should wrap around to find the next enabled item if initialIndex is disabled', () => { + const wrappingItems = [ + { value: 'A' }, + { value: 'B', disabled: true }, + { value: 'C', disabled: true }, + ]; + const { result } = renderHook(() => + useSelectionList({ + items: wrappingItems, + initialIndex: 2, + onSelect: mockOnSelect, + }), + ); + expect(result.current.activeIndex).toBe(0); + }); + + it('should default to 0 if initialIndex is out of bounds', () => { + const { result } = renderHook(() => + useSelectionList({ + items, + initialIndex: 10, + onSelect: mockOnSelect, + }), + ); + expect(result.current.activeIndex).toBe(0); + + const { result: resultNeg } = renderHook(() => + useSelectionList({ + items, + initialIndex: -1, + onSelect: mockOnSelect, + }), + ); + expect(resultNeg.current.activeIndex).toBe(0); + }); + + it('should stick to the initial index if all items are disabled', () => { + const allDisabled = [ + { value: 'A', disabled: true }, + { value: 'B', disabled: true }, + ]; + const { result } = renderHook(() => + useSelectionList({ + items: allDisabled, + initialIndex: 1, + onSelect: mockOnSelect, + }), + ); + expect(result.current.activeIndex).toBe(1); + }); + }); + + describe('Keyboard Navigation (Up/Down/J/K)', () => { + it('should move down with "j" and "down" keys, skipping disabled items', () => { + const { result } = renderHook(() => + useSelectionList({ items, onSelect: mockOnSelect }), + ); + expect(result.current.activeIndex).toBe(0); + pressKey('j'); + expect(result.current.activeIndex).toBe(2); + pressKey('down'); + expect(result.current.activeIndex).toBe(3); + }); + + it('should move up with "k" and "up" keys, skipping disabled items', () => { + const { result } = renderHook(() => + useSelectionList({ items, initialIndex: 3, onSelect: mockOnSelect }), + ); + expect(result.current.activeIndex).toBe(3); + pressKey('k'); + expect(result.current.activeIndex).toBe(2); + pressKey('up'); + expect(result.current.activeIndex).toBe(0); + }); + + it('should wrap navigation correctly', () => { + const { result } = renderHook(() => + useSelectionList({ + items, + initialIndex: items.length - 1, + onSelect: mockOnSelect, + }), + ); + expect(result.current.activeIndex).toBe(3); + pressKey('down'); + expect(result.current.activeIndex).toBe(0); + + pressKey('up'); + expect(result.current.activeIndex).toBe(3); + }); + + it('should call onHighlight when index changes', () => { + renderHook(() => + useSelectionList({ + items, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + }), + ); + pressKey('down'); + expect(mockOnHighlight).toHaveBeenCalledTimes(1); + expect(mockOnHighlight).toHaveBeenCalledWith('C'); + }); + + it('should not move or call onHighlight if navigation results in the same index (e.g., single item)', () => { + const singleItem = [{ value: 'A' }]; + const { result } = renderHook(() => + useSelectionList({ + items: singleItem, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + }), + ); + pressKey('down'); + expect(result.current.activeIndex).toBe(0); + expect(mockOnHighlight).not.toHaveBeenCalled(); + }); + + it('should not move or call onHighlight if all items are disabled', () => { + const allDisabled = [ + { value: 'A', disabled: true }, + { value: 'B', disabled: true }, + ]; + const { result } = renderHook(() => + useSelectionList({ + items: allDisabled, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + }), + ); + const initialIndex = result.current.activeIndex; + pressKey('down'); + expect(result.current.activeIndex).toBe(initialIndex); + expect(mockOnHighlight).not.toHaveBeenCalled(); + }); + }); + + describe('Selection (Enter)', () => { + it('should call onSelect when "return" is pressed on enabled item', () => { + renderHook(() => + useSelectionList({ + items, + initialIndex: 2, + onSelect: mockOnSelect, + }), + ); + pressKey('return'); + expect(mockOnSelect).toHaveBeenCalledTimes(1); + expect(mockOnSelect).toHaveBeenCalledWith('C'); + }); + + it('should not call onSelect if the active item is disabled', () => { + const { result } = renderHook(() => + useSelectionList({ + items, + onSelect: mockOnSelect, + }), + ); + + act(() => result.current.setActiveIndex(1)); + + pressKey('return'); + expect(mockOnSelect).not.toHaveBeenCalled(); + }); + }); + + describe('Keyboard Navigation Robustness (Rapid Input)', () => { + it('should handle rapid navigation and selection robustly (avoiding stale state)', () => { + const { result } = renderHook(() => + useSelectionList({ + items, // A, B(disabled), C, D. Initial index 0 (A). + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + }), + ); + + // Simulate rapid inputs with separate act blocks to allow effects to run + if (!activeKeypressHandler) throw new Error('Handler not active'); + + const handler = activeKeypressHandler; + + const press = (name: string) => { + const key: Key = { + name, + sequence: name, + ctrl: false, + meta: false, + shift: false, + paste: false, + }; + handler(key); + }; + + // 1. Press Down. Should move 0 (A) -> 2 (C). + act(() => { + press('down'); + }); + // 2. Press Down again. Should move 2 (C) -> 3 (D). + act(() => { + press('down'); + }); + // 3. Press Enter. Should select D. + act(() => { + press('return'); + }); + + expect(result.current.activeIndex).toBe(3); + + expect(mockOnHighlight).toHaveBeenCalledTimes(2); + expect(mockOnHighlight).toHaveBeenNthCalledWith(1, 'C'); + expect(mockOnHighlight).toHaveBeenNthCalledWith(2, 'D'); + + expect(mockOnSelect).toHaveBeenCalledTimes(1); + expect(mockOnSelect).toHaveBeenCalledWith('D'); + expect(mockOnSelect).not.toHaveBeenCalledWith('A'); + }); + + it('should handle ultra-rapid input (multiple presses in single act) without stale state', () => { + const { result } = renderHook(() => + useSelectionList({ + items, // A, B(disabled), C, D. Initial index 0 (A). + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + }), + ); + + // Simulate ultra-rapid inputs where all keypresses happen faster than React can re-render + act(() => { + if (!activeKeypressHandler) throw new Error('Handler not active'); + + const handler = activeKeypressHandler; + + const press = (name: string) => { + const key: Key = { + name, + sequence: name, + ctrl: false, + meta: false, + shift: false, + paste: false, + }; + handler(key); + }; + + // All presses happen in same render cycle - React batches the state updates + press('down'); // Should move 0 (A) -> 2 (C) + press('down'); // Should move 2 (C) -> 3 (D) + press('return'); // Should select D + }); + + expect(result.current.activeIndex).toBe(3); + + expect(mockOnHighlight).toHaveBeenCalledWith('D'); + expect(mockOnSelect).toHaveBeenCalledTimes(1); + expect(mockOnSelect).toHaveBeenCalledWith('D'); + }); + }); + + describe('Focus Management (isFocused)', () => { + it('should activate the keypress handler when focused (default) and items exist', () => { + const { result } = renderHook(() => + useSelectionList({ items, onSelect: mockOnSelect }), + ); + expect(activeKeypressHandler).not.toBeNull(); + pressKey('down'); + expect(result.current.activeIndex).toBe(2); + }); + + it('should not activate the keypress handler when isFocused is false', () => { + renderHook(() => + useSelectionList({ items, onSelect: mockOnSelect, isFocused: false }), + ); + expect(activeKeypressHandler).toBeNull(); + expect(() => pressKey('down')).toThrow(/keypress handler is not active/); + }); + + it('should not activate the keypress handler when items list is empty', () => { + renderHook(() => + useSelectionList({ + items: [], + onSelect: mockOnSelect, + isFocused: true, + }), + ); + expect(activeKeypressHandler).toBeNull(); + expect(() => pressKey('down')).toThrow(/keypress handler is not active/); + }); + + it('should activate/deactivate when isFocused prop changes', () => { + const { result, rerender } = renderHook( + (props: { isFocused: boolean }) => + useSelectionList({ items, onSelect: mockOnSelect, ...props }), + { initialProps: { isFocused: false } }, + ); + + expect(activeKeypressHandler).toBeNull(); + + rerender({ isFocused: true }); + expect(activeKeypressHandler).not.toBeNull(); + pressKey('down'); + expect(result.current.activeIndex).toBe(2); + + rerender({ isFocused: false }); + expect(activeKeypressHandler).toBeNull(); + expect(() => pressKey('down')).toThrow(/keypress handler is not active/); + }); + }); + + describe('Numeric Quick Selection (showNumbers=true)', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + const shortList = items; + const longList: Array> = Array.from( + { length: 15 }, + (_, i) => ({ value: `Item ${i + 1}` }), + ); + + const pressNumber = (num: string) => pressKey(num, num); + + it('should not respond to numbers if showNumbers is false (default)', () => { + const { result } = renderHook(() => + useSelectionList({ items: shortList, onSelect: mockOnSelect }), + ); + pressNumber('1'); + expect(result.current.activeIndex).toBe(0); + expect(mockOnSelect).not.toHaveBeenCalled(); + }); + + it('should select item immediately if the number cannot be extended (unambiguous)', () => { + const { result } = renderHook(() => + useSelectionList({ + items: shortList, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + showNumbers: true, + }), + ); + pressNumber('3'); + + expect(result.current.activeIndex).toBe(2); + expect(mockOnHighlight).toHaveBeenCalledWith('C'); + expect(mockOnSelect).toHaveBeenCalledTimes(1); + expect(mockOnSelect).toHaveBeenCalledWith('C'); + expect(vi.getTimerCount()).toBe(0); + }); + + it('should highlight and wait for timeout if the number can be extended (ambiguous)', () => { + const { result } = renderHook(() => + useSelectionList({ + items: longList, + initialIndex: 1, // Start at index 1 so pressing "1" (index 0) causes a change + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + showNumbers: true, + }), + ); + + pressNumber('1'); + + expect(result.current.activeIndex).toBe(0); + expect(mockOnHighlight).toHaveBeenCalledWith('Item 1'); + + expect(mockOnSelect).not.toHaveBeenCalled(); + expect(vi.getTimerCount()).toBe(1); + + act(() => { + vi.advanceTimersByTime(1000); + }); + + expect(mockOnSelect).toHaveBeenCalledTimes(1); + expect(mockOnSelect).toHaveBeenCalledWith('Item 1'); + }); + + it('should handle multi-digit input correctly', () => { + const { result } = renderHook(() => + useSelectionList({ + items: longList, + onSelect: mockOnSelect, + showNumbers: true, + }), + ); + + pressNumber('1'); + expect(mockOnSelect).not.toHaveBeenCalled(); + + pressNumber('2'); + + expect(result.current.activeIndex).toBe(11); + + expect(mockOnSelect).toHaveBeenCalledTimes(1); + expect(mockOnSelect).toHaveBeenCalledWith('Item 12'); + }); + + it('should reset buffer if input becomes invalid (out of bounds)', () => { + const { result } = renderHook(() => + useSelectionList({ + items: shortList, + onSelect: mockOnSelect, + showNumbers: true, + }), + ); + + pressNumber('5'); + + expect(result.current.activeIndex).toBe(0); + expect(mockOnSelect).not.toHaveBeenCalled(); + + pressNumber('3'); + expect(result.current.activeIndex).toBe(2); + expect(mockOnSelect).toHaveBeenCalledWith('C'); + }); + + it('should allow "0" as subsequent digit, but ignore as first digit', () => { + const { result } = renderHook(() => + useSelectionList({ + items: longList, + onSelect: mockOnSelect, + showNumbers: true, + }), + ); + + pressNumber('0'); + expect(result.current.activeIndex).toBe(0); + expect(mockOnSelect).not.toHaveBeenCalled(); + // Timer should be running to clear the '0' input buffer + expect(vi.getTimerCount()).toBe(1); + + // Press '1', then '0' (Item 10, index 9) + pressNumber('1'); + pressNumber('0'); + + expect(result.current.activeIndex).toBe(9); + expect(mockOnSelect).toHaveBeenCalledWith('Item 10'); + }); + + it('should clear the initial "0" input after timeout', () => { + renderHook(() => + useSelectionList({ + items: longList, + onSelect: mockOnSelect, + showNumbers: true, + }), + ); + + pressNumber('0'); + act(() => vi.advanceTimersByTime(1000)); // Timeout the '0' input + + pressNumber('1'); + expect(mockOnSelect).not.toHaveBeenCalled(); // Should be waiting for second digit + + act(() => vi.advanceTimersByTime(1000)); // Timeout '1' + expect(mockOnSelect).toHaveBeenCalledWith('Item 1'); + }); + + it('should highlight but not select a disabled item (immediate selection case)', () => { + const { result } = renderHook(() => + useSelectionList({ + items: shortList, // B (index 1, number 2) is disabled + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + showNumbers: true, + }), + ); + + pressNumber('2'); + + expect(result.current.activeIndex).toBe(1); + expect(mockOnHighlight).toHaveBeenCalledWith('B'); + + // Should not select immediately, even though 20 > 4 + expect(mockOnSelect).not.toHaveBeenCalled(); + }); + + it('should highlight but not select a disabled item (timeout case)', () => { + // Create a list where the ambiguous prefix points to a disabled item + const disabledAmbiguousList = [ + { value: 'Item 1 Disabled', disabled: true }, + ...longList.slice(1), + ]; + + const { result } = renderHook(() => + useSelectionList({ + items: disabledAmbiguousList, + onSelect: mockOnSelect, + showNumbers: true, + }), + ); + + pressNumber('1'); + expect(result.current.activeIndex).toBe(0); + expect(vi.getTimerCount()).toBe(1); + + act(() => { + vi.advanceTimersByTime(1000); + }); + + // Should not select after timeout + expect(mockOnSelect).not.toHaveBeenCalled(); + }); + + it('should clear the number buffer if a non-numeric key (e.g., navigation) is pressed', () => { + const { result } = renderHook(() => + useSelectionList({ + items: longList, + onSelect: mockOnSelect, + showNumbers: true, + }), + ); + + pressNumber('1'); + expect(vi.getTimerCount()).toBe(1); + + pressKey('down'); + + expect(result.current.activeIndex).toBe(1); + expect(vi.getTimerCount()).toBe(0); + + pressNumber('3'); + // Should select '3', not '13' + expect(result.current.activeIndex).toBe(2); + }); + + it('should clear the number buffer if "return" is pressed', () => { + renderHook(() => + useSelectionList({ + items: longList, + onSelect: mockOnSelect, + showNumbers: true, + }), + ); + + pressNumber('1'); + + pressKey('return'); + expect(mockOnSelect).toHaveBeenCalledTimes(1); + + expect(vi.getTimerCount()).toBe(0); + + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(mockOnSelect).toHaveBeenCalledTimes(1); + }); + }); + + describe('Reactivity (Dynamic Updates)', () => { + it('should update activeIndex when initialIndex prop changes', () => { + const { result, rerender } = renderHook( + ({ initialIndex }: { initialIndex: number }) => + useSelectionList({ + items, + onSelect: mockOnSelect, + initialIndex, + }), + { initialProps: { initialIndex: 0 } }, + ); + + rerender({ initialIndex: 2 }); + expect(result.current.activeIndex).toBe(2); + }); + + it('should validate index when initialIndex prop changes to a disabled item', () => { + const { result, rerender } = renderHook( + ({ initialIndex }: { initialIndex: number }) => + useSelectionList({ + items, + onSelect: mockOnSelect, + initialIndex, + }), + { initialProps: { initialIndex: 0 } }, + ); + + rerender({ initialIndex: 1 }); + + expect(result.current.activeIndex).toBe(2); + }); + + it('should adjust activeIndex if items change and the initialIndex is now out of bounds', () => { + const { result, rerender } = renderHook( + ({ items: testItems }: { items: Array> }) => + useSelectionList({ + onSelect: mockOnSelect, + initialIndex: 3, + items: testItems, + }), + { initialProps: { items } }, + ); + + expect(result.current.activeIndex).toBe(3); + + const shorterItems = [{ value: 'X' }, { value: 'Y' }]; + rerender({ items: shorterItems }); // Length 2 + + // The useEffect syncs based on the initialIndex (3) which is now out of bounds. It defaults to 0. + expect(result.current.activeIndex).toBe(0); + }); + + it('should adjust activeIndex if items change and the initialIndex becomes disabled', () => { + 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); + + const newItems = [ + { value: 'A' }, + { value: 'B', disabled: true }, + { value: 'C' }, + ]; + rerender({ items: newItems }); + + expect(result.current.activeIndex).toBe(2); + }); + + it('should reset to 0 if items change to an empty list', () => { + const { result, rerender } = renderHook( + ({ items: testItems }: { items: Array> }) => + useSelectionList({ + onSelect: mockOnSelect, + initialIndex: 2, + items: testItems, + }), + { initialProps: { items } }, + ); + + rerender({ items: [] }); + expect(result.current.activeIndex).toBe(0); + }); + }); + + describe('Manual Control', () => { + it('should allow manual setting of active index via setActiveIndex', () => { + const { result } = renderHook(() => + useSelectionList({ items, onSelect: mockOnSelect }), + ); + + act(() => { + result.current.setActiveIndex(3); + }); + expect(result.current.activeIndex).toBe(3); + + act(() => { + result.current.setActiveIndex(1); + }); + expect(result.current.activeIndex).toBe(1); + }); + }); + + describe('Cleanup', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('should clear timeout on unmount when timer is active', () => { + const longList: Array> = Array.from( + { length: 15 }, + (_, i) => ({ value: `Item ${i + 1}` }), + ); + + const { unmount } = renderHook(() => + useSelectionList({ + items: longList, + onSelect: mockOnSelect, + showNumbers: true, + }), + ); + + pressKey('1', '1'); + + expect(vi.getTimerCount()).toBe(1); + + act(() => { + vi.advanceTimersByTime(500); + }); + expect(mockOnSelect).not.toHaveBeenCalled(); + + unmount(); + + expect(vi.getTimerCount()).toBe(0); + + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(mockOnSelect).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/cli/src/ui/hooks/useSelectionList.ts b/packages/cli/src/ui/hooks/useSelectionList.ts new file mode 100644 index 0000000000..51d625dd4c --- /dev/null +++ b/packages/cli/src/ui/hooks/useSelectionList.ts @@ -0,0 +1,343 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useReducer, useRef, useEffect } from 'react'; +import { useKeypress } from './useKeypress.js'; + +export interface SelectionListItem { + value: T; + disabled?: boolean; +} + +export interface UseSelectionListOptions { + items: Array>; + initialIndex?: number; + onSelect: (value: T) => void; + onHighlight?: (value: T) => void; + isFocused?: boolean; + showNumbers?: boolean; +} + +export interface UseSelectionListResult { + activeIndex: number; + setActiveIndex: (index: number) => void; +} + +interface SelectionListState { + activeIndex: number; + pendingHighlight: boolean; + pendingSelect: boolean; +} + +type SelectionListAction = + | { + type: 'SET_ACTIVE_INDEX'; + payload: { + index: number; + items: Array>; + }; + } + | { + type: 'MOVE_UP'; + payload: { + items: Array>; + }; + } + | { + type: 'MOVE_DOWN'; + payload: { + items: Array>; + }; + } + | { + type: 'SELECT_CURRENT'; + payload: { + items: Array>; + }; + } + | { + type: 'INITIALIZE'; + payload: { initialIndex: number; items: Array> }; + } + | { + type: 'CLEAR_PENDING_FLAGS'; + }; + +const NUMBER_INPUT_TIMEOUT_MS = 1000; + +/** + * Helper function to find the next enabled index in a given direction, supporting wrapping. + */ +const findNextValidIndex = ( + currentIndex: number, + direction: 'up' | 'down', + items: Array>, +): number => { + const len = items.length; + if (len === 0) return currentIndex; + + let nextIndex = currentIndex; + const step = direction === 'down' ? 1 : -1; + + for (let i = 0; i < len; i++) { + // Calculate the next index, wrapping around if necessary. + // We add `len` before the modulo to ensure a positive result in JS for negative steps. + nextIndex = (nextIndex + step + len) % len; + + if (!items[nextIndex]?.disabled) { + return nextIndex; + } + } + + // If all items are disabled, return the original index + return currentIndex; +}; + +function selectionListReducer( + state: SelectionListState, + action: SelectionListAction, +): SelectionListState { + switch (action.type) { + case 'SET_ACTIVE_INDEX': { + const { index, items } = action.payload; + + // Only update if index actually changed and is valid + if (index === state.activeIndex) { + return state; + } + + if (index >= 0 && index < items.length) { + return { ...state, activeIndex: index, pendingHighlight: true }; + } + return state; + } + + case 'MOVE_UP': { + const { items } = action.payload; + const newIndex = findNextValidIndex(state.activeIndex, 'up', items); + if (newIndex !== state.activeIndex) { + return { ...state, activeIndex: newIndex, pendingHighlight: true }; + } + return state; + } + + case 'MOVE_DOWN': { + const { items } = action.payload; + const newIndex = findNextValidIndex(state.activeIndex, 'down', items); + if (newIndex !== state.activeIndex) { + return { ...state, activeIndex: newIndex, pendingHighlight: true }; + } + return state; + } + + case 'SELECT_CURRENT': { + return { ...state, pendingSelect: true }; + } + + case 'INITIALIZE': { + const { initialIndex, items } = action.payload; + + if (items.length === 0) { + const newIndex = 0; + return newIndex === state.activeIndex + ? state + : { ...state, activeIndex: newIndex }; + } + + 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 }; + } + + case 'CLEAR_PENDING_FLAGS': { + return { + ...state, + pendingHighlight: false, + pendingSelect: false, + }; + } + + default: { + const exhaustiveCheck: never = action; + console.error(`Unknown selection list action: ${exhaustiveCheck}`); + return state; + } + } +} + +/** + * A headless hook that provides keyboard navigation and selection logic + * for list-based selection components like radio buttons and menus. + * + * Features: + * - Keyboard navigation with j/k and arrow keys + * - Selection with Enter key + * - Numeric quick selection (when showNumbers is true) + * - Handles disabled items (skips them during navigation) + * - Wrapping navigation (last to first, first to last) + */ +export function useSelectionList({ + items, + initialIndex = 0, + onSelect, + onHighlight, + isFocused = true, + showNumbers = false, +}: UseSelectionListOptions): UseSelectionListResult { + const [state, dispatch] = useReducer(selectionListReducer, { + activeIndex: initialIndex, + pendingHighlight: false, + pendingSelect: false, + }); + const numberInputRef = useRef(''); + const numberInputTimer = useRef(null); + + // Initialize/synchronize state when initialIndex or items change + useEffect(() => { + dispatch({ type: 'INITIALIZE', payload: { initialIndex, items } }); + }, [initialIndex, items]); + + // Handle side effects based on state changes + useEffect(() => { + let needsClear = false; + + if (state.pendingHighlight && items[state.activeIndex]) { + onHighlight?.(items[state.activeIndex]!.value); + needsClear = true; + } + + if (state.pendingSelect && items[state.activeIndex]) { + const currentItem = items[state.activeIndex]; + if (currentItem && !currentItem.disabled) { + onSelect(currentItem.value); + } + needsClear = true; + } + + if (needsClear) { + dispatch({ type: 'CLEAR_PENDING_FLAGS' }); + } + }, [ + state.pendingHighlight, + state.pendingSelect, + state.activeIndex, + items, + onHighlight, + onSelect, + ]); + + useEffect( + () => () => { + if (numberInputTimer.current) { + clearTimeout(numberInputTimer.current); + } + }, + [], + ); + + useKeypress( + (key) => { + const { sequence, name } = key; + const isNumeric = showNumbers && /^[0-9]$/.test(sequence); + + // Clear number input buffer on non-numeric key press + if (!isNumeric && numberInputTimer.current) { + clearTimeout(numberInputTimer.current); + numberInputRef.current = ''; + } + + if (name === 'k' || name === 'up') { + dispatch({ type: 'MOVE_UP', payload: { items } }); + return; + } + + if (name === 'j' || name === 'down') { + dispatch({ type: 'MOVE_DOWN', payload: { items } }); + return; + } + + if (name === 'return') { + dispatch({ type: 'SELECT_CURRENT', payload: { items } }); + return; + } + + // Handle numeric input for quick selection + if (isNumeric) { + if (numberInputTimer.current) { + clearTimeout(numberInputTimer.current); + } + + const newNumberInput = numberInputRef.current + sequence; + numberInputRef.current = newNumberInput; + + const targetIndex = Number.parseInt(newNumberInput, 10) - 1; + + // Single '0' is invalid (1-indexed) + if (newNumberInput === '0') { + numberInputTimer.current = setTimeout(() => { + numberInputRef.current = ''; + }, NUMBER_INPUT_TIMEOUT_MS); + return; + } + + if (targetIndex >= 0 && targetIndex < items.length) { + dispatch({ + type: 'SET_ACTIVE_INDEX', + payload: { index: targetIndex, items }, + }); + + // If the number can't be a prefix for another valid number, select immediately + const potentialNextNumber = Number.parseInt(newNumberInput + '0', 10); + if (potentialNextNumber > items.length) { + dispatch({ + type: 'SELECT_CURRENT', + payload: { items }, + }); + numberInputRef.current = ''; + } else { + // Otherwise wait for more input or timeout + numberInputTimer.current = setTimeout(() => { + dispatch({ + type: 'SELECT_CURRENT', + payload: { items }, + }); + numberInputRef.current = ''; + }, NUMBER_INPUT_TIMEOUT_MS); + } + } else { + // Number is out of bounds + numberInputRef.current = ''; + } + } + }, + { isActive: !!(isFocused && items.length > 0) }, + ); + + const setActiveIndex = (index: number) => { + dispatch({ + type: 'SET_ACTIVE_INDEX', + payload: { index, items }, + }); + }; + + return { + activeIndex: state.activeIndex, + setActiveIndex, + }; +}