diff --git a/packages/cli/src/ui/IdeIntegrationNudge.tsx b/packages/cli/src/ui/IdeIntegrationNudge.tsx index 5ed6237e85..2dfa6a263f 100644 --- a/packages/cli/src/ui/IdeIntegrationNudge.tsx +++ b/packages/cli/src/ui/IdeIntegrationNudge.tsx @@ -50,6 +50,7 @@ export function IdeIntegrationNudge({ userSelection: 'yes', isExtensionPreInstalled, }, + key: 'Yes', }, { label: 'No (esc)', @@ -57,6 +58,7 @@ export function IdeIntegrationNudge({ userSelection: 'no', isExtensionPreInstalled, }, + key: 'No (esc)', }, { label: "No, don't ask again", @@ -64,6 +66,7 @@ export function IdeIntegrationNudge({ userSelection: 'dismiss', isExtensionPreInstalled, }, + key: "No, don't ask again", }, ]; diff --git a/packages/cli/src/ui/auth/AuthDialog.test.tsx b/packages/cli/src/ui/auth/AuthDialog.test.tsx index 1c3a017352..25446bf818 100644 --- a/packages/cli/src/ui/auth/AuthDialog.test.tsx +++ b/packages/cli/src/ui/auth/AuthDialog.test.tsx @@ -110,6 +110,7 @@ describe('AuthDialog', () => { expect(items).toContainEqual({ label: 'Use Cloud Shell user credentials', value: AuthType.CLOUD_SHELL, + key: AuthType.CLOUD_SHELL, }); }); diff --git a/packages/cli/src/ui/auth/AuthDialog.tsx b/packages/cli/src/ui/auth/AuthDialog.tsx index 3342fb6d21..7eb2c8ccd6 100644 --- a/packages/cli/src/ui/auth/AuthDialog.tsx +++ b/packages/cli/src/ui/auth/AuthDialog.tsx @@ -40,20 +40,27 @@ export function AuthDialog({ { label: 'Login with Google', value: AuthType.LOGIN_WITH_GOOGLE, + key: AuthType.LOGIN_WITH_GOOGLE, }, ...(process.env['CLOUD_SHELL'] === 'true' ? [ { label: 'Use Cloud Shell user credentials', value: AuthType.CLOUD_SHELL, + key: AuthType.CLOUD_SHELL, }, ] : []), { label: 'Use Gemini API Key', value: AuthType.USE_GEMINI, + key: AuthType.USE_GEMINI, + }, + { + label: 'Vertex AI', + value: AuthType.USE_VERTEX_AI, + key: AuthType.USE_VERTEX_AI, }, - { label: 'Vertex AI', value: AuthType.USE_VERTEX_AI }, ]; if (settings.merged.security?.auth?.enforcedType) { diff --git a/packages/cli/src/ui/components/DialogManager.tsx b/packages/cli/src/ui/components/DialogManager.tsx index 3117692c1b..9486f54792 100644 --- a/packages/cli/src/ui/components/DialogManager.tsx +++ b/packages/cli/src/ui/components/DialogManager.tsx @@ -105,8 +105,8 @@ export const DialogManager = ({ addItem }: DialogManagerProps) => { { uiState.confirmationRequest!.onConfirm(value); diff --git a/packages/cli/src/ui/components/EditorSettingsDialog.tsx b/packages/cli/src/ui/components/EditorSettingsDialog.tsx index 9ed601e72f..c09cbc47da 100644 --- a/packages/cli/src/ui/components/EditorSettingsDialog.tsx +++ b/packages/cli/src/ui/components/EditorSettingsDialog.tsx @@ -65,8 +65,16 @@ export function EditorSettingsDialog({ } const scopeItems = [ - { label: 'User Settings', value: SettingScope.User }, - { label: 'Workspace Settings', value: SettingScope.Workspace }, + { + label: 'User Settings', + value: SettingScope.User, + key: SettingScope.User, + }, + { + label: 'Workspace Settings', + value: SettingScope.Workspace, + key: SettingScope.Workspace, + }, ]; const handleEditorSelect = (editorType: EditorType | 'not_set') => { @@ -127,6 +135,7 @@ export function EditorSettingsDialog({ label: item.name, value: item.type, disabled: item.disabled, + key: item.type, }))} initialIndex={editorIndex} onSelect={handleEditorSelect} diff --git a/packages/cli/src/ui/components/FolderTrustDialog.tsx b/packages/cli/src/ui/components/FolderTrustDialog.tsx index f5e31897d7..3292b22751 100644 --- a/packages/cli/src/ui/components/FolderTrustDialog.tsx +++ b/packages/cli/src/ui/components/FolderTrustDialog.tsx @@ -57,14 +57,17 @@ export const FolderTrustDialog: React.FC = ({ { label: `Trust folder (${dirName})`, value: FolderTrustChoice.TRUST_FOLDER, + key: `Trust folder (${dirName})`, }, { label: `Trust parent folder (${parentFolder})`, value: FolderTrustChoice.TRUST_PARENT, + key: `Trust parent folder (${parentFolder})`, }, { label: "Don't trust (esc)", value: FolderTrustChoice.DO_NOT_TRUST, + key: "Don't trust (esc)", }, ]; diff --git a/packages/cli/src/ui/components/LoopDetectionConfirmation.tsx b/packages/cli/src/ui/components/LoopDetectionConfirmation.tsx index c644c8866a..2439367d84 100644 --- a/packages/cli/src/ui/components/LoopDetectionConfirmation.tsx +++ b/packages/cli/src/ui/components/LoopDetectionConfirmation.tsx @@ -38,12 +38,14 @@ export function LoopDetectionConfirmation({ value: { userSelection: 'keep', }, + key: 'Keep loop detection enabled (esc)', }, { label: 'Disable loop detection for this session', value: { userSelection: 'disable', }, + key: 'Disable loop detection for this session', }, ]; diff --git a/packages/cli/src/ui/components/ModelDialog.tsx b/packages/cli/src/ui/components/ModelDialog.tsx index 3198ac2cc6..6879b5a32d 100644 --- a/packages/cli/src/ui/components/ModelDialog.tsx +++ b/packages/cli/src/ui/components/ModelDialog.tsx @@ -29,21 +29,25 @@ const MODEL_OPTIONS = [ value: DEFAULT_GEMINI_MODEL_AUTO, title: 'Auto (recommended)', description: 'Let the system choose the best model for your task', + key: DEFAULT_GEMINI_MODEL_AUTO, }, { value: DEFAULT_GEMINI_MODEL, title: 'Pro', description: 'For complex tasks that require deep reasoning and creativity', + key: DEFAULT_GEMINI_MODEL, }, { value: DEFAULT_GEMINI_FLASH_MODEL, title: 'Flash', description: 'For tasks that need a balance of speed and reasoning', + key: DEFAULT_GEMINI_FLASH_MODEL, }, { value: DEFAULT_GEMINI_FLASH_LITE_MODEL, title: 'Flash-Lite', description: 'For simple tasks that need to be done quickly', + key: DEFAULT_GEMINI_FLASH_LITE_MODEL, }, ]; diff --git a/packages/cli/src/ui/components/PermissionsModifyTrustDialog.tsx b/packages/cli/src/ui/components/PermissionsModifyTrustDialog.tsx index 4704c64b6d..94da207842 100644 --- a/packages/cli/src/ui/components/PermissionsModifyTrustDialog.tsx +++ b/packages/cli/src/ui/components/PermissionsModifyTrustDialog.tsx @@ -23,14 +23,17 @@ const TRUST_LEVEL_ITEMS = [ { label: 'Trust this folder', value: TrustLevel.TRUST_FOLDER, + key: TrustLevel.TRUST_FOLDER, }, { label: 'Trust parent folder', value: TrustLevel.TRUST_PARENT, + key: TrustLevel.TRUST_PARENT, }, { label: "Don't trust", value: TrustLevel.DO_NOT_TRUST, + key: TrustLevel.DO_NOT_TRUST, }, ]; diff --git a/packages/cli/src/ui/components/ProQuotaDialog.test.tsx b/packages/cli/src/ui/components/ProQuotaDialog.test.tsx index c3a1afda6a..0670af5ebc 100644 --- a/packages/cli/src/ui/components/ProQuotaDialog.test.tsx +++ b/packages/cli/src/ui/components/ProQuotaDialog.test.tsx @@ -38,10 +38,12 @@ describe('ProQuotaDialog', () => { { label: 'Change auth (executes the /auth command)', value: 'auth', + key: 'auth', }, { label: `Continue with gemini-2.5-flash`, value: 'continue', + key: 'continue', }, ], }), diff --git a/packages/cli/src/ui/components/ProQuotaDialog.tsx b/packages/cli/src/ui/components/ProQuotaDialog.tsx index 17e5965e0b..0f3c4a555a 100644 --- a/packages/cli/src/ui/components/ProQuotaDialog.tsx +++ b/packages/cli/src/ui/components/ProQuotaDialog.tsx @@ -24,10 +24,12 @@ export function ProQuotaDialog({ { label: 'Change auth (executes the /auth command)', value: 'auth' as const, + key: 'auth', }, { label: `Continue with ${fallbackModel}`, value: 'continue' as const, + key: 'continue', }, ]; diff --git a/packages/cli/src/ui/components/SettingsDialog.tsx b/packages/cli/src/ui/components/SettingsDialog.tsx index d58fe3ea13..96216189c8 100644 --- a/packages/cli/src/ui/components/SettingsDialog.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.tsx @@ -358,7 +358,10 @@ export function SettingsDialog({ }; // Scope selector items - const scopeItems = getScopeItems(); + const scopeItems = getScopeItems().map((item) => ({ + ...item, + key: item.value, + })); const handleScopeHighlight = (scope: SettingScope) => { setSelectedScope(scope); diff --git a/packages/cli/src/ui/components/ShellConfirmationDialog.tsx b/packages/cli/src/ui/components/ShellConfirmationDialog.tsx index bd5ef15400..a9e9cd47e7 100644 --- a/packages/cli/src/ui/components/ShellConfirmationDialog.tsx +++ b/packages/cli/src/ui/components/ShellConfirmationDialog.tsx @@ -53,14 +53,17 @@ export const ShellConfirmationDialog: React.FC< { label: 'Yes, allow once', value: ToolConfirmationOutcome.ProceedOnce, + key: 'Yes, allow once', }, { label: 'Yes, allow always for this session', value: ToolConfirmationOutcome.ProceedAlways, + key: 'Yes, allow always for this session', }, { label: 'No (esc)', value: ToolConfirmationOutcome.Cancel, + key: 'No (esc)', }, ]; diff --git a/packages/cli/src/ui/components/ThemeDialog.tsx b/packages/cli/src/ui/components/ThemeDialog.tsx index 497ba1c654..468ec88885 100644 --- a/packages/cli/src/ui/components/ThemeDialog.tsx +++ b/packages/cli/src/ui/components/ThemeDialog.tsx @@ -63,12 +63,14 @@ export function ThemeDialog({ value: theme.name, themeNameDisplay: theme.name, themeTypeDisplay: capitalize(theme.type), + key: theme.name, })), ...customThemeNames.map((name) => ({ label: name, value: name, themeNameDisplay: name, themeTypeDisplay: 'Custom', + key: name, })), ]; diff --git a/packages/cli/src/ui/components/WorkspaceMigrationDialog.tsx b/packages/cli/src/ui/components/WorkspaceMigrationDialog.tsx index ea89e194fd..35977e7268 100644 --- a/packages/cli/src/ui/components/WorkspaceMigrationDialog.tsx +++ b/packages/cli/src/ui/components/WorkspaceMigrationDialog.tsx @@ -98,8 +98,8 @@ export function WorkspaceMigrationDialog(props: { { if (value === 'migrate') { diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index dfcaddfedf..bc00cefecd 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -150,23 +150,27 @@ export const ToolConfirmationMessage: React.FC< options.push({ label: 'Yes, allow once', value: ToolConfirmationOutcome.ProceedOnce, + key: 'Yes, allow once', }); if (isTrustedFolder) { options.push({ label: 'Yes, allow always', value: ToolConfirmationOutcome.ProceedAlways, + key: 'Yes, allow always', }); } if (!config.getIdeMode() || !isDiffingEnabled) { options.push({ label: 'Modify with external editor', value: ToolConfirmationOutcome.ModifyWithEditor, + key: 'Modify with external editor', }); } options.push({ label: 'No, suggest changes (esc)', value: ToolConfirmationOutcome.Cancel, + key: 'No, suggest changes (esc)', }); bodyContent = ( @@ -185,16 +189,19 @@ export const ToolConfirmationMessage: React.FC< options.push({ label: 'Yes, allow once', value: ToolConfirmationOutcome.ProceedOnce, + key: 'Yes, allow once', }); if (isTrustedFolder) { options.push({ label: `Yes, allow always ...`, value: ToolConfirmationOutcome.ProceedAlways, + key: `Yes, allow always ...`, }); } options.push({ label: 'No, suggest changes (esc)', value: ToolConfirmationOutcome.Cancel, + key: 'No, suggest changes (esc)', }); let bodyContentHeight = availableBodyContentHeight(); @@ -225,16 +232,19 @@ export const ToolConfirmationMessage: React.FC< options.push({ label: 'Yes, allow once', value: ToolConfirmationOutcome.ProceedOnce, + key: 'Yes, allow once', }); if (isTrustedFolder) { options.push({ label: 'Yes, allow always', value: ToolConfirmationOutcome.ProceedAlways, + key: 'Yes, allow always', }); } options.push({ label: 'No, suggest changes (esc)', value: ToolConfirmationOutcome.Cancel, + key: 'No, suggest changes (esc)', }); bodyContent = ( @@ -270,20 +280,24 @@ export const ToolConfirmationMessage: React.FC< options.push({ label: 'Yes, allow once', value: ToolConfirmationOutcome.ProceedOnce, + key: 'Yes, allow once', }); if (isTrustedFolder) { options.push({ label: `Yes, always allow tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"`, value: ToolConfirmationOutcome.ProceedAlwaysTool, // Cast until types are updated + key: `Yes, always allow tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"`, }); options.push({ label: `Yes, always allow all tools from server "${mcpProps.serverName}"`, value: ToolConfirmationOutcome.ProceedAlwaysServer, + key: `Yes, always allow all tools from server "${mcpProps.serverName}"`, }); } options.push({ label: 'No, suggest changes (esc)', value: ToolConfirmationOutcome.Cancel, + key: 'No, suggest changes (esc)', }); } diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx index ebe52e0d27..6d432956c8 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx @@ -35,16 +35,20 @@ describe('BaseSelectionList', () => { 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' }, + { value: 'A', label: 'Item A', key: 'A' }, + { value: 'B', label: 'Item B', disabled: true, key: 'B' }, + { value: 'C', label: 'Item C', key: 'C' }, ]; // Helper to render the component with default props const renderComponent = ( - props: Partial> = {}, + props: Partial< + BaseSelectionListProps< + string, + { value: string; label: string; disabled?: boolean; key: string } + > + > = {}, activeIndex: number = 0, ) => { vi.mocked(useSelectionList).mockReturnValue({ @@ -53,12 +57,16 @@ describe('BaseSelectionList', () => { }); mockRenderItem.mockImplementation( - (item: (typeof items)[0], context: RenderItemContext) => ( - {item.label} - ), + ( + item: { value: string; label: string; disabled?: boolean; key: string }, + context: RenderItemContext, + ) => {item.label}, ); - const defaultProps: BaseSelectionListProps = { + const defaultProps: BaseSelectionListProps< + string, + { value: string; label: string; disabled?: boolean; key: string } + > = { items, onSelect: mockOnSelect, onHighlight: mockOnHighlight, @@ -216,6 +224,7 @@ describe('BaseSelectionList', () => { const longList = Array.from({ length: 15 }, (_, i) => ({ value: `Item ${i + 1}`, label: `Item ${i + 1}`, + key: `Item ${i + 1}`, })); // We must increase maxItemsToShow (default 10) to see the 10th item and beyond @@ -249,19 +258,22 @@ describe('BaseSelectionList', () => { const longList = Array.from({ length: 10 }, (_, i) => ({ value: `Item ${i + 1}`, label: `Item ${i + 1}`, + key: `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, - }; + const componentProps: BaseSelectionListProps< + string, + { value: string; label: string; key: string } + > = { + items: longList, + maxItemsToShow: MAX_ITEMS, + onSelect: mockOnSelect, + onHighlight: mockOnHighlight, + renderItem: mockRenderItem, + }; vi.mocked(useSelectionList).mockReturnValue({ activeIndex: initialActiveIndex, @@ -428,6 +440,7 @@ describe('BaseSelectionList', () => { const longList = Array.from({ length: 10 }, (_, i) => ({ value: `Item ${i + 1}`, label: `Item ${i + 1}`, + key: `Item ${i + 1}`, })); const MAX_ITEMS = 3; diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx index 94c63271cb..8071582f75 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx @@ -10,14 +10,19 @@ import { Text, Box } from 'ink'; import { theme } from '../../semantic-colors.js'; import { useSelectionList } from '../../hooks/useSelectionList.js'; +import type { SelectionListItem } from '../../hooks/useSelectionList.js'; + export interface RenderItemContext { isSelected: boolean; titleColor: string; numberColor: string; } -export interface BaseSelectionListProps> { - items: Array; +export interface BaseSelectionListProps< + T, + TItem extends SelectionListItem = SelectionListItem, +> { + items: TItem[]; initialIndex?: number; onSelect: (value: T) => void; onHighlight?: (value: T) => void; @@ -25,10 +30,7 @@ export interface BaseSelectionListProps> { showNumbers?: boolean; showScrollArrows?: boolean; maxItemsToShow?: number; - renderItem: ( - item: TItem & { value: T; disabled?: boolean }, - context: RenderItemContext, - ) => React.ReactNode; + renderItem: (item: TItem, context: RenderItemContext) => React.ReactNode; } /** @@ -45,7 +47,10 @@ export interface BaseSelectionListProps> { * Specific components should use this as a base and provide * their own renderItem implementation for custom content. */ -export function BaseSelectionList>({ +export function BaseSelectionList< + T, + TItem extends SelectionListItem = SelectionListItem, +>({ items, initialIndex = 0, onSelect, @@ -123,7 +128,7 @@ export function BaseSelectionList>({ )}.`; return ( - + {/* Radio button indicator */} { const mockOnHighlight = vi.fn(); const ITEMS: Array> = [ - { title: 'Foo Title', description: 'This is Foo.', value: 'foo' }, - { title: 'Bar Title', description: 'This is Bar.', value: 'bar' }, + { + title: 'Foo Title', + description: 'This is Foo.', + value: 'foo', + key: 'foo', + }, + { + title: 'Bar Title', + description: 'This is Bar.', + value: 'bar', + key: 'bar', + }, { title: 'Baz Title', description: 'This is Baz.', value: 'baz', disabled: true, + key: 'baz', }, ]; diff --git a/packages/cli/src/ui/components/shared/DescriptiveRadioButtonSelect.tsx b/packages/cli/src/ui/components/shared/DescriptiveRadioButtonSelect.tsx index a45daa30f9..3cc563283d 100644 --- a/packages/cli/src/ui/components/shared/DescriptiveRadioButtonSelect.tsx +++ b/packages/cli/src/ui/components/shared/DescriptiveRadioButtonSelect.tsx @@ -8,12 +8,11 @@ import type React from 'react'; import { Text, Box } from 'ink'; import { theme } from '../../semantic-colors.js'; import { BaseSelectionList } from './BaseSelectionList.js'; +import type { SelectionListItem } from '../../hooks/useSelectionList.js'; -export interface DescriptiveRadioSelectItem { - value: T; +export interface DescriptiveRadioSelectItem extends SelectionListItem { title: string; description: string; - disabled?: boolean; } export interface DescriptiveRadioButtonSelectProps { @@ -61,7 +60,7 @@ export function DescriptiveRadioButtonSelect({ showScrollArrows={showScrollArrows} maxItemsToShow={maxItemsToShow} renderItem={(item, { titleColor }) => ( - + {item.title} {item.description} diff --git a/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx b/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx index b4595dc0b4..1b15bc6f9e 100644 --- a/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx +++ b/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx @@ -62,9 +62,9 @@ describe('RadioButtonSelect', () => { const mockOnHighlight = vi.fn(); const ITEMS: Array> = [ - { label: 'Option 1', value: 'one' }, - { label: 'Option 2', value: 'two' }, - { label: 'Option 3', value: 'three', disabled: true }, + { label: 'Option 1', value: 'one', key: 'one' }, + { label: 'Option 2', value: 'two', key: 'two' }, + { label: 'Option 3', value: 'three', disabled: true, key: 'three' }, ]; const renderComponent = ( @@ -155,6 +155,7 @@ describe('RadioButtonSelect', () => { value: 'a-light', themeNameDisplay: 'Theme A', themeTypeDisplay: '(Light)', + key: 'a-light', }; const result = renderItem(themeItem, mockContext); @@ -186,6 +187,7 @@ describe('RadioButtonSelect', () => { label: 'Incomplete Theme', value: 'incomplete', themeNameDisplay: 'Only Name', + key: 'incomplete', }; const result = renderItem(partialThemeItem, mockContext); diff --git a/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx b/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx index c3ea07c30c..02fc85228c 100644 --- a/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx +++ b/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx @@ -8,15 +8,14 @@ import type React from 'react'; import { Text } from 'ink'; import { theme } from '../../semantic-colors.js'; import { BaseSelectionList } from './BaseSelectionList.js'; +import type { SelectionListItem } from '../../hooks/useSelectionList.js'; /** * Represents a single option for the RadioButtonSelect. * Requires a label for display and a value to be returned on selection. */ -export interface RadioSelectItem { +export interface RadioSelectItem extends SelectionListItem { label: string; - value: T; - disabled?: boolean; themeNameDisplay?: string; themeTypeDisplay?: string; } @@ -74,7 +73,7 @@ export function RadioButtonSelect({ // Handle special theme display case for ThemeDialog compatibility if (item.themeNameDisplay && item.themeTypeDisplay) { return ( - + {item.themeNameDisplay}{' '} {item.themeTypeDisplay} diff --git a/packages/cli/src/ui/components/shared/ScopeSelector.tsx b/packages/cli/src/ui/components/shared/ScopeSelector.tsx index 8066d8c9ee..30aa1e403f 100644 --- a/packages/cli/src/ui/components/shared/ScopeSelector.tsx +++ b/packages/cli/src/ui/components/shared/ScopeSelector.tsx @@ -27,7 +27,10 @@ export function ScopeSelector({ isFocused, initialScope, }: ScopeSelectorProps): React.JSX.Element { - const scopeItems = getScopeItems(); + const scopeItems = getScopeItems().map((item) => ({ + ...item, + key: item.value, + })); const initialIndex = scopeItems.findIndex( (item) => item.value === initialScope, diff --git a/packages/cli/src/ui/hooks/useSelectionList.test.ts b/packages/cli/src/ui/hooks/useSelectionList.test.ts index b0d8cdc981..8383d89c9c 100644 --- a/packages/cli/src/ui/hooks/useSelectionList.test.ts +++ b/packages/cli/src/ui/hooks/useSelectionList.test.ts @@ -25,10 +25,10 @@ describe('useSelectionList', () => { const mockOnHighlight = vi.fn(); const items: Array> = [ - { value: 'A' }, - { value: 'B', disabled: true }, - { value: 'C' }, - { value: 'D' }, + { value: 'A', key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, + { value: 'C', key: 'C' }, + { value: 'D', key: 'D' }, ]; beforeEach(() => { @@ -105,9 +105,9 @@ describe('useSelectionList', () => { 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 }, + { value: 'A', key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, + { value: 'C', disabled: true, key: 'C' }, ]; const { result } = renderHook(() => useSelectionList({ @@ -141,8 +141,8 @@ describe('useSelectionList', () => { it('should stick to the initial index if all items are disabled', () => { const allDisabled = [ - { value: 'A', disabled: true }, - { value: 'B', disabled: true }, + { value: 'A', disabled: true, key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, ]; const { result } = renderHook(() => useSelectionList({ @@ -208,7 +208,7 @@ describe('useSelectionList', () => { }); it('should not move or call onHighlight if navigation results in the same index (e.g., single item)', () => { - const singleItem = [{ value: 'A' }]; + const singleItem = [{ value: 'A', key: 'A' }]; const { result } = renderHook(() => useSelectionList({ items: singleItem, @@ -223,8 +223,8 @@ describe('useSelectionList', () => { it('should not move or call onHighlight if all items are disabled', () => { const allDisabled = [ - { value: 'A', disabled: true }, - { value: 'B', disabled: true }, + { value: 'A', disabled: true, key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, ]; const { result } = renderHook(() => useSelectionList({ @@ -423,7 +423,7 @@ describe('useSelectionList', () => { const shortList = items; const longList: Array> = Array.from( { length: 15 }, - (_, i) => ({ value: `Item ${i + 1}` }), + (_, i) => ({ value: `Item ${i + 1}`, key: `Item ${i + 1}` }), ); const pressNumber = (num: string) => pressKey(num, num); @@ -585,7 +585,7 @@ describe('useSelectionList', () => { 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 }, + { value: 'Item 1 Disabled', disabled: true, key: 'Item 1 Disabled' }, ...longList.slice(1), ]; @@ -670,6 +670,28 @@ describe('useSelectionList', () => { expect(result.current.activeIndex).toBe(2); }); + it('should respect a new initialIndex even after user interaction', () => { + const { result, rerender } = renderHook( + ({ initialIndex }: { initialIndex: number }) => + useSelectionList({ + items, + onSelect: mockOnSelect, + initialIndex, + }), + { initialProps: { initialIndex: 0 } }, + ); + + // User navigates, changing the active index + pressKey('down'); + expect(result.current.activeIndex).toBe(2); + + // The component re-renders with a new initial index + rerender({ initialIndex: 3 }); + + // The hook should now respect the new initial index + expect(result.current.activeIndex).toBe(3); + }); + it('should validate index when initialIndex prop changes to a disabled item', () => { const { result, rerender } = renderHook( ({ initialIndex }: { initialIndex: number }) => @@ -699,7 +721,10 @@ describe('useSelectionList', () => { expect(result.current.activeIndex).toBe(3); - const shorterItems = [{ value: 'X' }, { value: 'Y' }]; + const shorterItems = [ + { value: 'X', key: 'X' }, + { value: 'Y', key: 'Y' }, + ]; rerender({ items: shorterItems }); // Length 2 // The useEffect syncs based on the initialIndex (3) which is now out of bounds. It defaults to 0. @@ -707,7 +732,11 @@ describe('useSelectionList', () => { }); it('should adjust activeIndex if items change and the initialIndex becomes disabled', () => { - const initialItems = [{ value: 'A' }, { value: 'B' }, { value: 'C' }]; + const initialItems = [ + { value: 'A', key: 'A' }, + { value: 'B', key: 'B' }, + { value: 'C', key: 'C' }, + ]; const { result, rerender } = renderHook( ({ items: testItems }: { items: Array> }) => useSelectionList({ @@ -721,9 +750,9 @@ describe('useSelectionList', () => { expect(result.current.activeIndex).toBe(1); const newItems = [ - { value: 'A' }, - { value: 'B', disabled: true }, - { value: 'C' }, + { value: 'A', key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, + { value: 'C', key: 'C' }, ]; rerender({ items: newItems }); @@ -747,10 +776,10 @@ describe('useSelectionList', () => { it('should not reset activeIndex when items are deeply equal', () => { const initialItems = [ - { value: 'A' }, - { value: 'B', disabled: true }, - { value: 'C' }, - { value: 'D' }, + { value: 'A', key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, + { value: 'C', key: 'C' }, + { value: 'D', key: 'D' }, ]; const { result, rerender } = renderHook( @@ -775,10 +804,10 @@ describe('useSelectionList', () => { // Create new array with same content (deeply equal but not identical) const newItems = [ - { value: 'A' }, - { value: 'B', disabled: true }, - { value: 'C' }, - { value: 'D' }, + { value: 'A', key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, + { value: 'C', key: 'C' }, + { value: 'D', key: 'D' }, ]; rerender({ items: newItems }); @@ -791,10 +820,10 @@ describe('useSelectionList', () => { it('should update activeIndex when items change structurally', () => { const initialItems = [ - { value: 'A' }, - { value: 'B', disabled: true }, - { value: 'C' }, - { value: 'D' }, + { value: 'A', key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, + { value: 'C', key: 'C' }, + { value: 'D', key: 'D' }, ]; const { result, rerender } = renderHook( @@ -812,7 +841,11 @@ describe('useSelectionList', () => { mockOnHighlight.mockClear(); // Change item values (not deeply equal) - const newItems = [{ value: 'X' }, { value: 'Y' }, { value: 'Z' }]; + const newItems = [ + { value: 'X', key: 'X' }, + { value: 'Y', key: 'Y' }, + { value: 'Z', key: 'Z' }, + ]; rerender({ items: newItems }); @@ -821,7 +854,11 @@ describe('useSelectionList', () => { }); it('should handle partial changes in items array', () => { - const initialItems = [{ value: 'A' }, { value: 'B' }, { value: 'C' }]; + const initialItems = [ + { value: 'A', key: 'A' }, + { value: 'B', key: 'B' }, + { value: 'C', key: 'C' }, + ]; const { result, rerender } = renderHook( ({ items: testItems }: { items: Array> }) => @@ -837,9 +874,9 @@ describe('useSelectionList', () => { // Change only one item's disabled status const newItems = [ - { value: 'A' }, - { value: 'B', disabled: true }, - { value: 'C' }, + { value: 'A', key: 'A' }, + { value: 'B', disabled: true, key: 'B' }, + { value: 'C', key: 'C' }, ]; rerender({ items: newItems }); @@ -847,6 +884,37 @@ describe('useSelectionList', () => { // Should find next valid index since current became disabled expect(result.current.activeIndex).toBe(2); }); + + it('should update selection when a new item is added to the start of the list', () => { + const initialItems = [ + { value: 'A', key: 'A' }, + { value: 'B', key: 'B' }, + { value: 'C', key: 'C' }, + ]; + + const { result, rerender } = renderHook( + ({ items: testItems }: { items: Array> }) => + useSelectionList({ + onSelect: mockOnSelect, + items: testItems, + }), + { initialProps: { items: initialItems } }, + ); + + pressKey('down'); + expect(result.current.activeIndex).toBe(1); + + const newItems = [ + { value: 'D', key: 'D' }, + { value: 'A', key: 'A' }, + { value: 'B', key: 'B' }, + { value: 'C', key: 'C' }, + ]; + + rerender({ items: newItems }); + + expect(result.current.activeIndex).toBe(2); + }); }); describe('Manual Control', () => { @@ -879,7 +947,7 @@ describe('useSelectionList', () => { it('should clear timeout on unmount when timer is active', () => { const longList: Array> = Array.from( { length: 15 }, - (_, i) => ({ value: `Item ${i + 1}` }), + (_, i) => ({ value: `Item ${i + 1}`, key: `Item ${i + 1}` }), ); const { unmount } = renderHook(() => diff --git a/packages/cli/src/ui/hooks/useSelectionList.ts b/packages/cli/src/ui/hooks/useSelectionList.ts index 2ac45a73f8..5e9a7f181f 100644 --- a/packages/cli/src/ui/hooks/useSelectionList.ts +++ b/packages/cli/src/ui/hooks/useSelectionList.ts @@ -8,6 +8,7 @@ import { useReducer, useRef, useEffect } from 'react'; import { useKeypress } from './useKeypress.js'; export interface SelectionListItem { + key: string; value: T; disabled?: boolean; } @@ -70,27 +71,6 @@ 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. */ @@ -122,11 +102,20 @@ const findNextValidIndex = ( const computeInitialIndex = ( initialIndex: number, items: Array>, + initialKey?: string, ): number => { if (items.length === 0) { return 0; } + if (initialKey !== undefined) { + for (let i = 0; i < items.length; i++) { + if (items[i]!.key === initialKey && !items[i]!.disabled) { + return i; + } + } + } + let targetIndex = initialIndex; if (targetIndex < 0 || targetIndex >= items.length) { @@ -184,13 +173,17 @@ function selectionListReducer( case 'INITIALIZE': { const { initialIndex, items } = action.payload; - if ( - state.initialIndex === initialIndex && - areItemsEqual(state.items, items) - ) { + const activeKey = + initialIndex === state.initialIndex && + state.activeIndex !== state.initialIndex + ? state.items[state.activeIndex]?.key + : undefined; + + if (items === state.items && initialIndex === state.initialIndex) { return state; } - const targetIndex = computeInitialIndex(initialIndex, items); + + const targetIndex = computeInitialIndex(initialIndex, items, activeKey); return { ...state, diff --git a/packages/cli/src/ui/privacy/CloudFreePrivacyNotice.tsx b/packages/cli/src/ui/privacy/CloudFreePrivacyNotice.tsx index ba32537e17..f6bf0d4b80 100644 --- a/packages/cli/src/ui/privacy/CloudFreePrivacyNotice.tsx +++ b/packages/cli/src/ui/privacy/CloudFreePrivacyNotice.tsx @@ -68,8 +68,8 @@ export const CloudFreePrivacyNotice = ({ } const items = [ - { label: 'Yes', value: true }, - { label: 'No', value: false }, + { label: 'Yes', value: true, key: 'true' }, + { label: 'No', value: false, key: 'false' }, ]; return (