From 6037cb5d60b8e31b92315dd86fc65dbc57316376 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Thu, 14 Aug 2025 16:48:54 -0700 Subject: [PATCH] Fix bug where RadioButtonSelect treated an omitted isFocus parameter (#6274) --- packages/cli/src/ui/IdeIntegrationNudge.tsx | 6 +- packages/cli/src/ui/components/AuthDialog.tsx | 1 - .../shared/RadioButtonSelect.test.tsx | 104 ++++++++++++++---- .../components/shared/RadioButtonSelect.tsx | 2 +- 4 files changed, 87 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/ui/IdeIntegrationNudge.tsx b/packages/cli/src/ui/IdeIntegrationNudge.tsx index 2be69ad7..906947f2 100644 --- a/packages/cli/src/ui/IdeIntegrationNudge.tsx +++ b/packages/cli/src/ui/IdeIntegrationNudge.tsx @@ -92,11 +92,7 @@ export function IdeIntegrationNudge({ {installText} - + ); } diff --git a/packages/cli/src/ui/components/AuthDialog.tsx b/packages/cli/src/ui/components/AuthDialog.tsx index 1262f894..c353727c 100644 --- a/packages/cli/src/ui/components/AuthDialog.tsx +++ b/packages/cli/src/ui/components/AuthDialog.tsx @@ -147,7 +147,6 @@ export function AuthDialog({ items={items} initialIndex={initialAuthIndex} onSelect={handleAuthSelect} - isFocused={true} /> {errorMessage && ( diff --git a/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx b/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx index 4b36fe3c..d4c13ba5 100644 --- a/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx +++ b/packages/cli/src/ui/components/shared/RadioButtonSelect.test.tsx @@ -5,11 +5,12 @@ */ import { render } from 'ink-testing-library'; +import { waitFor } from '@testing-library/react'; import { RadioButtonSelect, type RadioSelectItem, } from './RadioButtonSelect.js'; -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; const ITEMS: Array> = [ { label: 'Option 1', value: 'one' }, @@ -27,12 +28,7 @@ describe('', () => { it('renders with the second item selected and matches snapshot', () => { const { lastFrame } = render( - {}} - isFocused={true} - />, + {}} />, ); expect(lastFrame()).toMatchSnapshot(); }); @@ -42,7 +38,6 @@ describe('', () => { {}} - isFocused={true} showNumbers={false} />, ); @@ -58,7 +53,6 @@ describe('', () => { {}} - isFocused={true} showScrollArrows={true} maxItemsToShow={5} />, @@ -82,11 +76,7 @@ describe('', () => { }, ]; const { lastFrame } = render( - {}} - isFocused={true} - />, + {}} />, ); expect(lastFrame()).toMatchSnapshot(); }); @@ -97,11 +87,7 @@ describe('', () => { value: `item-${i + 1}`, })); const { lastFrame } = render( - {}} - isFocused={true} - />, + {}} />, ); expect(lastFrame()).toMatchSnapshot(); }); @@ -113,3 +99,83 @@ describe('', () => { expect(lastFrame()).toBe(''); }); }); + +describe('keyboard navigation', () => { + it('should call onSelect when "enter" is pressed', () => { + const onSelect = vi.fn(); + const { stdin } = render( + , + ); + + stdin.write('\r'); + + expect(onSelect).toHaveBeenCalledWith('one'); + }); + + describe('when isFocused is false', () => { + it('should not handle any keyboard input', () => { + const onSelect = vi.fn(); + const { stdin } = render( + , + ); + + 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 } = render( + , + ); + + stdin.write('\u001B[B'); // Down arrow + + await waitFor(() => { + expect(lastFrame()).toContain('● 2. Option 2'); + }); + + stdin.write('\r'); + + expect(onSelect).toHaveBeenCalledWith('two'); + }); + + it('should navigate up with arrow key and select with enter', async () => { + const onSelect = vi.fn(); + const { stdin, lastFrame } = render( + , + ); + + stdin.write('\u001B[A'); // Up arrow + + await waitFor(() => { + expect(lastFrame()).toContain('● 1. Option 1'); + }); + + stdin.write('\r'); + + expect(onSelect).toHaveBeenCalledWith('one'); + }); + }); +}); diff --git a/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx b/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx index 511d3847..746744e5 100644 --- a/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx +++ b/packages/cli/src/ui/components/shared/RadioButtonSelect.tsx @@ -55,7 +55,7 @@ export function RadioButtonSelect({ initialIndex = 0, onSelect, onHighlight, - isFocused, + isFocused = true, showScrollArrows = false, maxItemsToShow = 10, showNumbers = true,