Jacob314/add radio button keys (#10083)

2026-03-19 18:40:57 -07:00 · 2025-09-28 14:50:47 -07:00
parent 1bd75f060d
commit 62ba330612
26 changed files with 263 additions and 112 deletions
--- 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) => {
        <Box paddingY={1}>
          <RadioButtonSelect
            items={[
-              { label: 'Yes', value: true },
-              { label: 'No', value: false },
+              { label: 'Yes', value: true, key: 'Yes' },
+              { label: 'No', value: false, key: 'No' },
            ]}
            onSelect={(value: boolean) => {
              uiState.confirmationRequest!.onConfirm(value);
--- 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}
--- 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<FolderTrustDialogProps> = ({
    {
      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)",
    },
  ];

--- 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',
    },
  ];

--- 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,
  },
 ];

--- 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,
  },
 ];

--- 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',
          },
        ],
      }),
--- 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',
    },
  ];

--- 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);
--- 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)',
    },
  ];

--- 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,
    })),
  ];

--- a/packages/cli/src/ui/components/WorkspaceMigrationDialog.tsx
+++ b/packages/cli/src/ui/components/WorkspaceMigrationDialog.tsx
@@ -98,8 +98,8 @@ export function WorkspaceMigrationDialog(props: {
      <Box marginTop={1}>
        <RadioButtonSelect
          items={[
-            { label: 'Install all', value: 'migrate' },
-            { label: 'Skip', value: 'skip' },
+            { label: 'Install all', value: 'migrate', key: 'migrate' },
+            { label: 'Skip', value: 'skip', key: 'skip' },
          ]}
          onSelect={(value: string) => {
            if (value === 'migrate') {
--- 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)',
    });
  }

--- 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<BaseSelectionListProps<string, { label: string }>> = {},
+    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) => (
-        <Text color={context.titleColor}>{item.label}</Text>
-      ),
+      (
+        item: { value: string; label: string; disabled?: boolean; key: string },
+        context: RenderItemContext,
+      ) => <Text color={context.titleColor}>{item.label}</Text>,
    );

-    const defaultProps: BaseSelectionListProps<string, { label: string }> = {
+    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<string, { label: string }> =
-        {
-          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;

--- 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<T, TItem = Record<string, unknown>> {
-  items: Array<TItem & { value: T; disabled?: boolean }>;
+export interface BaseSelectionListProps<
+  T,
+  TItem extends SelectionListItem<T> = SelectionListItem<T>,
+> {
+  items: TItem[];
  initialIndex?: number;
  onSelect: (value: T) => void;
  onHighlight?: (value: T) => void;
@@ -25,10 +30,7 @@ export interface BaseSelectionListProps<T, TItem = Record<string, unknown>> {
  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<T, TItem = Record<string, unknown>> {
 * Specific components should use this as a base and provide
 * their own renderItem implementation for custom content.
 */
-export function BaseSelectionList<T, TItem = Record<string, unknown>>({
+export function BaseSelectionList<
+  T,
+  TItem extends SelectionListItem<T> = SelectionListItem<T>,
+>({
  items,
  initialIndex = 0,
  onSelect,
@@ -123,7 +128,7 @@ export function BaseSelectionList<T, TItem = Record<string, unknown>>({
        )}.`;

        return (
-          <Box key={itemIndex} alignItems="flex-start">
+          <Box key={item.key} alignItems="flex-start">
            {/* Radio button indicator */}
            <Box minWidth={2} flexShrink={0}>
              <Text
--- a/packages/cli/src/ui/components/shared/DescriptiveRadioButtonSelect.test.tsx
+++ b/packages/cli/src/ui/components/shared/DescriptiveRadioButtonSelect.test.tsx
@@ -40,13 +40,24 @@ describe('DescriptiveRadioButtonSelect', () => {
  const mockOnHighlight = vi.fn();

  const ITEMS: Array<DescriptiveRadioSelectItem<string>> = [
-    { 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',
    },
  ];

--- 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<T> {
-  value: T;
+export interface DescriptiveRadioSelectItem<T> extends SelectionListItem<T> {
  title: string;
  description: string;
-  disabled?: boolean;
 }

 export interface DescriptiveRadioButtonSelectProps<T> {
@@ -61,7 +60,7 @@ export function DescriptiveRadioButtonSelect<T>({
      showScrollArrows={showScrollArrows}
      maxItemsToShow={maxItemsToShow}
      renderItem={(item, { titleColor }) => (
-        <Box flexDirection="column">
+        <Box flexDirection="column" key={item.key}>
          <Text color={titleColor}>{item.title}</Text>
          <Text color={theme.text.secondary}>{item.description}</Text>
        </Box>
--- 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<RadioSelectItem<string>> = [
-    { 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);
--- 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<T> {
+export interface RadioSelectItem<T> extends SelectionListItem<T> {
  label: string;
-  value: T;
-  disabled?: boolean;
  themeNameDisplay?: string;
  themeTypeDisplay?: string;
 }
@@ -74,7 +73,7 @@ export function RadioButtonSelect<T>({
        // Handle special theme display case for ThemeDialog compatibility
        if (item.themeNameDisplay && item.themeTypeDisplay) {
          return (
-            <Text color={titleColor} wrap="truncate">
+            <Text color={titleColor} wrap="truncate" key={item.key}>
              {item.themeNameDisplay}{' '}
              <Text color={theme.text.secondary}>{item.themeTypeDisplay}</Text>
            </Text>
--- 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,