From 05aa1465fea80a498059e704b26a62a90371e6f1 Mon Sep 17 00:00:00 2001 From: Adib234 <30782825+Adib234@users.noreply.github.com> Date: Tue, 14 Apr 2026 15:07:00 -0400 Subject: [PATCH] feat(cli): enable mouse clicking for cursor positioning in AskUser multi-line answers (#24630) --- .../cli/src/ui/auth/ApiAuthDialog.test.tsx | 27 ++- .../cli/src/ui/components/AskUserDialog.tsx | 51 +++-- .../shared/BaseSelectionList.test.tsx | 81 ++++++- .../components/shared/BaseSelectionList.tsx | 197 ++++++++++++------ .../ui/components/shared/TextInput.test.tsx | 39 ++++ .../src/ui/components/shared/TextInput.tsx | 22 +- 6 files changed, 321 insertions(+), 96 deletions(-) diff --git a/packages/cli/src/ui/auth/ApiAuthDialog.test.tsx b/packages/cli/src/ui/auth/ApiAuthDialog.test.tsx index d46e0295a1..12d01fac4c 100644 --- a/packages/cli/src/ui/auth/ApiAuthDialog.test.tsx +++ b/packages/cli/src/ui/auth/ApiAuthDialog.test.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { render } from '../../test-utils/render.js'; +import { renderWithProviders } from '../../test-utils/render.js'; import { waitFor } from '../../test-utils/async.js'; import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; import { ApiAuthDialog } from './ApiAuthDialog.js'; @@ -40,11 +40,16 @@ vi.mock('../components/shared/text-buffer.js', async (importOriginal) => { }; }); -vi.mock('../contexts/UIStateContext.js', () => ({ - useUIState: vi.fn(() => ({ - terminalWidth: 80, - })), -})); +vi.mock('../contexts/UIStateContext.js', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + useUIState: vi.fn(() => ({ + terminalWidth: 80, + })), + }; +}); const mockedUseKeypress = useKeypress as Mock; const mockedUseTextBuffer = useTextBuffer as Mock; @@ -73,7 +78,7 @@ describe('ApiAuthDialog', () => { }); it('renders correctly', async () => { - const { lastFrame, unmount } = await render( + const { lastFrame, unmount } = await renderWithProviders( , ); expect(lastFrame()).toMatchSnapshot(); @@ -81,7 +86,7 @@ describe('ApiAuthDialog', () => { }); it('renders with a defaultValue', async () => { - const { unmount } = await render( + const { unmount } = await renderWithProviders( { 'calls $expectedCall.name when $keyName is pressed', async ({ keyName, sequence, expectedCall, args }) => { mockBuffer.text = 'submitted-key'; // Set for the onSubmit case - const { unmount } = await render( + const { unmount } = await renderWithProviders( , ); // calls[0] is the ApiAuthDialog's useKeypress (Ctrl+C handler) @@ -133,7 +138,7 @@ describe('ApiAuthDialog', () => { ); it('displays an error message', async () => { - const { lastFrame, unmount } = await render( + const { lastFrame, unmount } = await renderWithProviders( { }); it('calls clearApiKey and clears buffer when Ctrl+C is pressed', async () => { - const { unmount } = await render( + const { unmount } = await renderWithProviders( , ); // Call 0 is ApiAuthDialog (isActive: true) diff --git a/packages/cli/src/ui/components/AskUserDialog.tsx b/packages/cli/src/ui/components/AskUserDialog.tsx index 295d54eb73..7e1dbf9c00 100644 --- a/packages/cli/src/ui/components/AskUserDialog.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.tsx @@ -13,7 +13,8 @@ import { useReducer, useContext, } from 'react'; -import { Box, Text } from 'ink'; +import { Box, Text, type DOMElement } from 'ink'; +import { useMouseClick } from '../hooks/useMouseClick.js'; import { theme } from '../semantic-colors.js'; import { checkExhaustive, type Question } from '@google/gemini-cli-core'; import { BaseSelectionList } from './shared/BaseSelectionList.js'; @@ -85,6 +86,24 @@ function autoBoldIfPlain(text: string): string { return text; } +const ClickableCheckbox: React.FC<{ + isChecked: boolean; + onClick: () => void; +}> = ({ isChecked, onClick }) => { + const ref = useRef(null); + useMouseClick(ref, () => { + onClick(); + }); + + return ( + + + [{isChecked ? 'x' : ' '}] + + + ); +}; + interface AskUserDialogState { answers: { [key: string]: string }; isEditingCustomOption: boolean; @@ -919,13 +938,14 @@ const ChoiceQuestionView: React.FC = ({ return ( {showCheck && ( - - [{isChecked ? 'x' : ' '}] - + { + if (!context.isSelected) { + handleSelect(optionItem); + } + }} + /> )} = ({ {showCheck && ( - - [{isChecked ? 'x' : ' '}] - + { + if (!context.isSelected) { + handleSelect(optionItem); + } + }} + /> )} {' '} diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx index b873de80d9..49bcb92728 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx @@ -4,7 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { act } from 'react'; import { renderWithProviders } from '../../../test-utils/render.js'; import { BaseSelectionList, @@ -14,8 +15,10 @@ import { import { useSelectionList } from '../../hooks/useSelectionList.js'; import { Text } from 'ink'; import type { theme } from '../../semantic-colors.js'; +import { useMouseClick } from '../../hooks/useMouseClick.js'; vi.mock('../../hooks/useSelectionList.js'); +vi.mock('../../hooks/useMouseClick.js'); const mockTheme = { text: { primary: 'COLOR_PRIMARY', secondary: 'COLOR_SECONDARY' }, @@ -35,6 +38,7 @@ describe('BaseSelectionList', () => { const mockOnSelect = vi.fn(); const mockOnHighlight = vi.fn(); const mockRenderItem = vi.fn(); + const mockSetActiveIndex = vi.fn(); const items = [ { value: 'A', label: 'Item A', key: 'A' }, @@ -54,7 +58,7 @@ describe('BaseSelectionList', () => { ) => { vi.mocked(useSelectionList).mockReturnValue({ activeIndex, - setActiveIndex: vi.fn(), + setActiveIndex: mockSetActiveIndex, }); mockRenderItem.mockImplementation( @@ -484,6 +488,79 @@ describe('BaseSelectionList', () => { }); }); + describe('Mouse Interaction', () => { + it('should register mouse click handler for each item', async () => { + const { unmount } = await renderComponent(); + + // items are A, B (disabled), C + expect(useMouseClick).toHaveBeenCalledTimes(3); + unmount(); + }); + + it('should update activeIndex on first click and call onSelect on second click', async () => { + const { unmount, waitUntilReady } = await renderComponent(); + await waitUntilReady(); + + // items[0] is 'A' (enabled) + // items[1] is 'B' (disabled) + // items[2] is 'C' (enabled) + + // Get the mouse click handler for the third item (index 2) + const mouseClickHandler = (useMouseClick as Mock).mock.calls[2][1]; + + // First click on item C + act(() => { + mouseClickHandler(); + }); + + expect(mockSetActiveIndex).toHaveBeenCalledWith(2); + expect(mockOnSelect).not.toHaveBeenCalled(); + + // Now simulate being on item C (isSelected = true) + // Rerender or update mocks for the next check + await renderComponent({}, 2); + + // Get the updated mouse click handler for item C + // useMouseClick is called 3 more times on rerender + const updatedMouseClickHandler = (useMouseClick as Mock).mock.calls[5][1]; + + // Second click on item C + act(() => { + updatedMouseClickHandler(); + }); + + expect(mockOnSelect).toHaveBeenCalledWith('C'); + unmount(); + }); + + it('should not call onSelect when a disabled item is clicked', async () => { + const { unmount, waitUntilReady } = await renderComponent(); + await waitUntilReady(); + + // items[1] is 'B' (disabled) + const mouseClickHandler = (useMouseClick as Mock).mock.calls[1][1]; + + act(() => { + mouseClickHandler(); + }); + + expect(mockSetActiveIndex).not.toHaveBeenCalled(); + expect(mockOnSelect).not.toHaveBeenCalled(); + unmount(); + }); + + it('should pass isActive: isFocused to useMouseClick', async () => { + const { unmount } = await renderComponent({ isFocused: false }); + + expect(useMouseClick).toHaveBeenCalledWith( + expect.any(Object), + expect.any(Function), + { isActive: false }, + ); + unmount(); + }); + }); + describe('Scroll Arrows (showScrollArrows)', () => { const longList = Array.from({ length: 10 }, (_, i) => ({ value: `Item ${i + 1}`, diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx index 455069f03f..353fca196f 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx @@ -5,13 +5,14 @@ */ import type React from 'react'; -import { useState } from 'react'; -import { Text, Box } from 'ink'; +import { useState, useRef } from 'react'; +import { Text, Box, type DOMElement } from 'ink'; import { theme } from '../../semantic-colors.js'; import { useSelectionList, type SelectionListItem, } from '../../hooks/useSelectionList.js'; +import { useMouseClick } from '../../hooks/useMouseClick.js'; export interface RenderItemContext { isSelected: boolean; @@ -38,6 +39,119 @@ export interface BaseSelectionListProps< renderItem: (item: TItem, context: RenderItemContext) => React.ReactNode; } +interface SelectionListItemRowProps< + T, + TItem extends SelectionListItem = SelectionListItem, +> { + item: TItem; + itemIndex: number; + isSelected: boolean; + isFocused: boolean; + showNumbers: boolean; + selectedIndicator: string; + numberColumnWidth: number; + onSelect: (value: T) => void; + setActiveIndex: (index: number) => void; + renderItem: (item: TItem, context: RenderItemContext) => React.ReactNode; +} + +function SelectionListItemRow< + T, + TItem extends SelectionListItem = SelectionListItem, +>({ + item, + itemIndex, + isSelected, + isFocused, + showNumbers, + selectedIndicator, + numberColumnWidth, + onSelect, + setActiveIndex, + renderItem, +}: SelectionListItemRowProps) { + const containerRef = useRef(null); + + useMouseClick( + containerRef, + () => { + if (!item.disabled) { + if (isSelected) { + // Second click on the same item triggers submission + onSelect(item.value); + } else { + // First click highlights the item + setActiveIndex(itemIndex); + } + } + }, + { isActive: isFocused }, + ); + + let titleColor = theme.text.primary; + let numberColor = theme.text.primary; + + if (isSelected) { + titleColor = theme.ui.focus; + numberColor = theme.ui.focus; + } 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 ? selectedIndicator : ' '} + + + + {/* Item number */} + {showNumbers && !item.hideNumber && ( + + {itemNumberText} + + )} + + {/* Custom content via render prop */} + + {renderItem(item, { + isSelected, + titleColor, + numberColor, + })} + + + ); +} + /** * Base component for selection lists that provides common UI structure * and keyboard navigation logic via the useSelectionList hook. @@ -70,7 +184,7 @@ export function BaseSelectionList< selectedIndicator = '●', renderItem, }: BaseSelectionListProps): React.JSX.Element { - const { activeIndex } = useSelectionList({ + const { activeIndex, setActiveIndex } = useSelectionList({ items, initialIndex, onSelect, @@ -107,10 +221,12 @@ export function BaseSelectionList< ); const numberColumnWidth = String(items.length).length; + const showArrows = showScrollArrows && items.length > maxItemsToShow; + return ( {/* Use conditional coloring instead of conditional rendering */} - {showScrollArrows && items.length > maxItemsToShow && ( + {showArrows && ( 0 @@ -126,71 +242,24 @@ export function BaseSelectionList< const itemIndex = effectiveScrollOffset + 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.ui.focus; - numberColor = theme.ui.focus; - } 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 ? selectedIndicator : ' '} - - - - {/* Item number */} - {showNumbers && !item.hideNumber && ( - - {itemNumberText} - - )} - - {/* Custom content via render prop */} - - {renderItem(item, { - isSelected, - titleColor, - numberColor, - })} - - + item={item} + itemIndex={itemIndex} + isSelected={isSelected} + isFocused={isFocused} + showNumbers={showNumbers} + selectedIndicator={selectedIndicator} + numberColumnWidth={numberColumnWidth} + onSelect={onSelect} + setActiveIndex={setActiveIndex} + renderItem={renderItem} + /> ); })} - {showScrollArrows && items.length > maxItemsToShow && ( + {showArrows && ( ({ useKeypress: vi.fn(), })); +vi.mock('../../hooks/useMouseClick.js', () => ({ + useMouseClick: vi.fn(), +})); + vi.mock('./text-buffer.js', async (importOriginal) => { const actual = await importOriginal(); const mockTextBuffer = { @@ -69,6 +74,7 @@ vi.mock('./text-buffer.js', async (importOriginal) => { const mockedUseKeypress = useKeypress as Mock; const mockedUseTextBuffer = useTextBuffer as Mock; +const mockedUseMouseClick = useMouseClick as Mock; describe('TextInput', () => { const onCancel = vi.fn(); @@ -84,6 +90,7 @@ describe('TextInput', () => { cursor: [0, 0], visualCursor: [0, 0], viewportVisualLines: [''], + visualScrollRow: 0, pastedContent: {} as Record, handleInput: vi.fn((key) => { if (key.sequence) { @@ -408,4 +415,36 @@ describe('TextInput', () => { expect(lastFrame()).toContain('line2'); unmount(); }); + + it('registers mouse click handler for free-form text input', async () => { + const { unmount } = await render( + , + ); + + expect(mockedUseMouseClick).toHaveBeenCalledWith( + expect.any(Object), + expect.any(Function), + expect.objectContaining({ isActive: true, name: 'left-press' }), + ); + unmount(); + }); + + it('registers mouse click handler for placeholder view', async () => { + mockBuffer.text = ''; + const { unmount } = await render( + , + ); + + expect(mockedUseMouseClick).toHaveBeenCalledWith( + expect.any(Object), + expect.any(Function), + expect.objectContaining({ isActive: true, name: 'left-press' }), + ); + unmount(); + }); }); diff --git a/packages/cli/src/ui/components/shared/TextInput.tsx b/packages/cli/src/ui/components/shared/TextInput.tsx index 479757d8f3..4cb8a6c94c 100644 --- a/packages/cli/src/ui/components/shared/TextInput.tsx +++ b/packages/cli/src/ui/components/shared/TextInput.tsx @@ -5,8 +5,8 @@ */ import type React from 'react'; -import { useCallback } from 'react'; -import { Text, Box } from 'ink'; +import { useCallback, useRef } from 'react'; +import { Text, Box, type DOMElement } from 'ink'; import { useKeypress, type Key } from '../../hooks/useKeypress.js'; import chalk from 'chalk'; import { theme } from '../../semantic-colors.js'; @@ -14,6 +14,7 @@ import { expandPastePlaceholders, type TextBuffer } from './text-buffer.js'; import { cpSlice, cpIndexToOffset } from '../../utils/textUtils.js'; import { Command } from '../../key/keyMatchers.js'; import { useKeyMatchers } from '../../hooks/useKeyMatchers.js'; +import { useMouseClick } from '../../hooks/useMouseClick.js'; export interface TextInputProps { buffer: TextBuffer; @@ -31,6 +32,8 @@ export function TextInput({ focus = true, }: TextInputProps): React.JSX.Element { const keyMatchers = useKeyMatchers(); + const containerRef = useRef(null); + const { text, handleInput, @@ -40,6 +43,17 @@ export function TextInput({ } = buffer; const [cursorVisualRowAbsolute, cursorVisualColAbsolute] = visualCursor; + useMouseClick( + containerRef, + (_event, relativeX, relativeY) => { + if (focus) { + const visRowAbsolute = visualScrollRow + relativeY; + buffer.moveToVisualPosition(visRowAbsolute, relativeX); + } + }, + { isActive: focus, name: 'left-press' }, + ); + const handleKeyPress = useCallback( (key: Key) => { if (key.name === 'escape' && onCancel) { @@ -64,7 +78,7 @@ export function TextInput({ if (showPlaceholder) { return ( - + {focus ? ( {chalk.inverse(placeholder[0] || ' ')} @@ -78,7 +92,7 @@ export function TextInput({ } return ( - + {viewportVisualLines.map((lineText, idx) => { const currentVisualRow = visualScrollRow + idx; const isCursorLine =