From 9a8e5d3940f9465bb2e07dcf9c6b68e27bf1734e Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Fri, 20 Feb 2026 13:08:24 -0800 Subject: [PATCH] fix(cli): extensions dialog UX polish (#19685) --- .../cli/src/ui/components/SettingsDialog.tsx | 18 +--- .../components/shared/SearchableList.test.tsx | 9 +- .../ui/components/shared/SearchableList.tsx | 88 ++++++++++++------- .../SearchableList.test.tsx.snap | 60 +++++++++++-- .../views/ExtensionRegistryView.test.tsx | 2 + .../views/ExtensionRegistryView.tsx | 25 +++++- .../cli/src/ui/hooks/useRegistrySearch.ts | 40 ++++----- packages/cli/src/ui/hooks/useSearchBuffer.ts | 41 +++++++++ 8 files changed, 203 insertions(+), 80 deletions(-) create mode 100644 packages/cli/src/ui/hooks/useSearchBuffer.ts diff --git a/packages/cli/src/ui/components/SettingsDialog.tsx b/packages/cli/src/ui/components/SettingsDialog.tsx index fe3acbd1f1..e95692275a 100644 --- a/packages/cli/src/ui/components/SettingsDialog.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.tsx @@ -39,8 +39,8 @@ import { } from '../../config/settingsSchema.js'; import { coreEvents, debugLogger } from '@google/gemini-cli-core'; import type { Config } from '@google/gemini-cli-core'; -import { useUIState } from '../contexts/UIStateContext.js'; -import { useTextBuffer } from './shared/text-buffer.js'; + +import { useSearchBuffer } from '../hooks/useSearchBuffer.js'; import { BaseSettingsDialog, type SettingsDialogItem, @@ -207,20 +207,10 @@ export function SettingsDialog({ return max; }, [selectedScope, settings]); - // Get mainAreaWidth for search buffer viewport - const { mainAreaWidth } = useUIState(); - const viewportWidth = mainAreaWidth - 8; - // Search input buffer - const searchBuffer = useTextBuffer({ + const searchBuffer = useSearchBuffer({ initialText: '', - initialCursorOffset: 0, - viewport: { - width: viewportWidth, - height: 1, - }, - singleLine: true, - onChange: (text) => setSearchQuery(text), + onChange: setSearchQuery, }); // Generate items for BaseSettingsDialog diff --git a/packages/cli/src/ui/components/shared/SearchableList.test.tsx b/packages/cli/src/ui/components/shared/SearchableList.test.tsx index 42b118e251..e156c12695 100644 --- a/packages/cli/src/ui/components/shared/SearchableList.test.tsx +++ b/packages/cli/src/ui/components/shared/SearchableList.test.tsx @@ -131,8 +131,9 @@ describe('SearchableList', () => { await waitFor(() => { const frame = lastFrame(); - expect(frame).toContain('> Item Two'); + expect(frame).toContain('● Item Two'); }); + expect(lastFrame()).toMatchSnapshot(); await React.act(async () => { stdin.write('One'); @@ -143,6 +144,7 @@ describe('SearchableList', () => { expect(frame).toContain('Item One'); expect(frame).not.toContain('Item Two'); }); + expect(lastFrame()).toMatchSnapshot(); await React.act(async () => { // Backspace "One" (3 chars) @@ -152,9 +154,10 @@ describe('SearchableList', () => { await waitFor(() => { const frame = lastFrame(); expect(frame).toContain('Item Two'); - expect(frame).toContain('> Item One'); - expect(frame).not.toContain('> Item Two'); + expect(frame).toContain('● Item One'); + expect(frame).not.toContain('● Item Two'); }); + expect(lastFrame()).toMatchSnapshot(); }); it('should filter items based on search query', async () => { diff --git a/packages/cli/src/ui/components/shared/SearchableList.tsx b/packages/cli/src/ui/components/shared/SearchableList.tsx index a20a44be42..1611bc2842 100644 --- a/packages/cli/src/ui/components/shared/SearchableList.tsx +++ b/packages/cli/src/ui/components/shared/SearchableList.tsx @@ -112,13 +112,36 @@ export function SearchableList({ isFocused: true, showNumbers: false, wrapAround: true, + priority: true, }); + const [scrollOffsetState, setScrollOffsetState] = React.useState(0); + + // Compute effective scroll offset during render to avoid visual flicker + let scrollOffset = scrollOffsetState; + + if (activeIndex < scrollOffset) { + scrollOffset = activeIndex; + } else if (activeIndex >= scrollOffset + maxItemsToShow) { + scrollOffset = activeIndex - maxItemsToShow + 1; + } + + const maxScroll = Math.max(0, filteredItems.length - maxItemsToShow); + if (scrollOffset > maxScroll) { + scrollOffset = maxScroll; + } + + // Update state to match derived value if it changed + if (scrollOffsetState !== scrollOffset) { + setScrollOffsetState(scrollOffset); + } + // Reset selection to top when items change if requested const prevItemsRef = React.useRef(filteredItems); - React.useEffect(() => { + React.useLayoutEffect(() => { if (resetSelectionOnItemsChange && filteredItems !== prevItemsRef.current) { setActiveIndex(0); + setScrollOffsetState(0); } prevItemsRef.current = filteredItems; }, [filteredItems, setActiveIndex, resetSelectionOnItemsChange]); @@ -135,14 +158,6 @@ export function SearchableList({ { isActive: true }, ); - const scrollOffset = Math.max( - 0, - Math.min( - activeIndex - Math.floor(maxItemsToShow / 2), - Math.max(0, filteredItems.length - maxItemsToShow), - ), - ); - const visibleItems = filteredItems.slice( scrollOffset, scrollOffset + maxItemsToShow, @@ -153,21 +168,22 @@ export function SearchableList({ isActive: boolean, labelWidth: number, ) => ( - - - {isActive ? '> ' : ' '} - {item.label.padEnd(labelWidth)} - - {item.description && ( - + + + + {isActive ? '●' : ''} + + + + + {item.label.padEnd(labelWidth)} + + {item.description && ( {item.description} - - )} + )} + ); @@ -204,16 +220,28 @@ export function SearchableList({ No items found. ) : ( - visibleItems.map((item, index) => { - const isSelected = activeIndex === scrollOffset + index; - return ( - - {renderItem - ? renderItem(item, isSelected, maxLabelWidth) - : defaultRenderItem(item, isSelected, maxLabelWidth)} + <> + {filteredItems.length > maxItemsToShow && ( + + ▲ - ); - }) + )} + {visibleItems.map((item, index) => { + const isSelected = activeIndex === scrollOffset + index; + return ( + + {renderItem + ? renderItem(item, isSelected, maxLabelWidth) + : defaultRenderItem(item, isSelected, maxLabelWidth)} + + ); + })} + {filteredItems.length > maxItemsToShow && ( + + ▼ + + )} + )} diff --git a/packages/cli/src/ui/components/shared/__snapshots__/SearchableList.test.tsx.snap b/packages/cli/src/ui/components/shared/__snapshots__/SearchableList.test.tsx.snap index e596373e01..35f21daee3 100644 --- a/packages/cli/src/ui/components/shared/__snapshots__/SearchableList.test.tsx.snap +++ b/packages/cli/src/ui/components/shared/__snapshots__/SearchableList.test.tsx.snap @@ -7,13 +7,61 @@ exports[`SearchableList > should match snapshot 1`] = ` │ Search... │ ╰────────────────────────────────────────────────────────────────────────────────────────────────╯ - > Item One - Description for item one + ● Item One + Description for item one - Item Two - Description for item two + Item Two + Description for item two - Item Three - Description for item three + Item Three + Description for item three +" +`; + +exports[`SearchableList > should reset selection to top when items change if resetSelectionOnItemsChange is true 1`] = ` +" Test List + + ╭────────────────────────────────────────────────────────────────────────────────────────────────╮ + │ Search... │ + ╰────────────────────────────────────────────────────────────────────────────────────────────────╯ + + Item One + Description for item one + + ● Item Two + Description for item two + + Item Three + Description for item three +" +`; + +exports[`SearchableList > should reset selection to top when items change if resetSelectionOnItemsChange is true 2`] = ` +" Test List + + ╭────────────────────────────────────────────────────────────────────────────────────────────────╮ + │ One │ + ╰────────────────────────────────────────────────────────────────────────────────────────────────╯ + + ● Item One + Description for item one +" +`; + +exports[`SearchableList > should reset selection to top when items change if resetSelectionOnItemsChange is true 3`] = ` +" Test List + + ╭────────────────────────────────────────────────────────────────────────────────────────────────╮ + │ Search... │ + ╰────────────────────────────────────────────────────────────────────────────────────────────────╯ + + ● Item One + Description for item one + + Item Two + Description for item two + + Item Three + Description for item three " `; diff --git a/packages/cli/src/ui/components/views/ExtensionRegistryView.test.tsx b/packages/cli/src/ui/components/views/ExtensionRegistryView.test.tsx index 58f687eb6d..954dff1f07 100644 --- a/packages/cli/src/ui/components/views/ExtensionRegistryView.test.tsx +++ b/packages/cli/src/ui/components/views/ExtensionRegistryView.test.tsx @@ -126,6 +126,8 @@ describe('ExtensionRegistryView', () => { vi.mocked(useUIState).mockReturnValue({ mainAreaWidth: 100, + terminalHeight: 40, + staticExtraHeight: 5, } as unknown as ReturnType); vi.mocked(useConfig).mockReturnValue({ diff --git a/packages/cli/src/ui/components/views/ExtensionRegistryView.tsx b/packages/cli/src/ui/components/views/ExtensionRegistryView.tsx index 9a7c15144a..1f6fba96ea 100644 --- a/packages/cli/src/ui/components/views/ExtensionRegistryView.tsx +++ b/packages/cli/src/ui/components/views/ExtensionRegistryView.tsx @@ -22,6 +22,8 @@ import { useConfig } from '../../contexts/ConfigContext.js'; import type { ExtensionManager } from '../../../config/extension-manager.js'; import { useRegistrySearch } from '../../hooks/useRegistrySearch.js'; +import { useUIState } from '../../contexts/UIStateContext.js'; + interface ExtensionRegistryViewProps { onSelect?: (extension: RegistryExtension) => void; onClose?: () => void; @@ -39,6 +41,7 @@ export function ExtensionRegistryView({ }: ExtensionRegistryViewProps): React.JSX.Element { const { extensions, loading, error, search } = useExtensionRegistry(); const config = useConfig(); + const { terminalHeight, staticExtraHeight } = useUIState(); const { extensionsUpdateState } = useExtensionUpdates( extensionManager, @@ -83,7 +86,7 @@ export function ExtensionRegistryView({ - {isActive ? '> ' : ' '} + {isActive ? '● ' : ' '} @@ -164,6 +167,24 @@ export function ExtensionRegistryView({ [], ); + const maxItemsToShow = useMemo(() => { + // SearchableList layout overhead: + // Container paddingY: 0 + // Title (marginBottom 1): 2 + // Search buffer (border 2, marginBottom 1): 4 + // Header (marginBottom 1): 2 + // Footer (marginTop 1): 2 + // List item (marginBottom 1): 2 per item + // Total static height = 2 + 4 + 2 + 2 = 10 + const staticHeight = 10; + const availableTerminalHeight = terminalHeight - staticExtraHeight; + const remainingHeight = Math.max(0, availableTerminalHeight - staticHeight); + const itemHeight = 2; // Each item takes 2 lines (content + marginBottom 1) + + // Ensure we show at least a few items and not more than we have + return Math.max(4, Math.floor(remainingHeight / itemHeight)); + }, [terminalHeight, staticExtraHeight]); + if (loading) { return ( @@ -191,7 +212,7 @@ export function ExtensionRegistryView({ renderItem={renderItem} header={header} footer={footer} - maxItemsToShow={8} + maxItemsToShow={maxItemsToShow} useSearch={useRegistrySearch} onSearch={search} resetSelectionOnItemsChange={true} diff --git a/packages/cli/src/ui/hooks/useRegistrySearch.ts b/packages/cli/src/ui/hooks/useRegistrySearch.ts index e1a1c4191b..3a93c61a0a 100644 --- a/packages/cli/src/ui/hooks/useRegistrySearch.ts +++ b/packages/cli/src/ui/hooks/useRegistrySearch.ts @@ -4,16 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useEffect } from 'react'; -import { - useTextBuffer, - type TextBuffer, -} from '../components/shared/text-buffer.js'; -import { useUIState } from '../contexts/UIStateContext.js'; +import { useState, useEffect, useRef } from 'react'; +import type { TextBuffer } from '../components/shared/text-buffer.js'; import type { GenericListItem } from '../components/shared/SearchableList.js'; - -const MIN_VIEWPORT_WIDTH = 20; -const VIEWPORT_WIDTH_OFFSET = 8; +import { useSearchBuffer } from './useSearchBuffer.js'; export interface UseRegistrySearchResult { filteredItems: T[]; @@ -31,26 +25,22 @@ export function useRegistrySearch(props: { const { items, initialQuery = '', onSearch } = props; const [searchQuery, setSearchQuery] = useState(initialQuery); + const isFirstRender = useRef(true); + const onSearchRef = useRef(onSearch); + + onSearchRef.current = onSearch; useEffect(() => { - onSearch?.(searchQuery); - }, [searchQuery, onSearch]); + if (isFirstRender.current) { + isFirstRender.current = false; + return; + } + onSearchRef.current?.(searchQuery); + }, [searchQuery]); - const { mainAreaWidth } = useUIState(); - const viewportWidth = Math.max( - MIN_VIEWPORT_WIDTH, - mainAreaWidth - VIEWPORT_WIDTH_OFFSET, - ); - - const searchBuffer = useTextBuffer({ + const searchBuffer = useSearchBuffer({ initialText: searchQuery, - initialCursorOffset: searchQuery.length, - viewport: { - width: viewportWidth, - height: 1, - }, - singleLine: true, - onChange: (text) => setSearchQuery(text), + onChange: setSearchQuery, }); const maxLabelWidth = 0; diff --git a/packages/cli/src/ui/hooks/useSearchBuffer.ts b/packages/cli/src/ui/hooks/useSearchBuffer.ts new file mode 100644 index 0000000000..d1c8f9f8b8 --- /dev/null +++ b/packages/cli/src/ui/hooks/useSearchBuffer.ts @@ -0,0 +1,41 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + useTextBuffer, + type TextBuffer, +} from '../components/shared/text-buffer.js'; +import { useUIState } from '../contexts/UIStateContext.js'; + +const MIN_VIEWPORT_WIDTH = 20; +const VIEWPORT_WIDTH_OFFSET = 8; + +export interface UseSearchBufferProps { + initialText?: string; + onChange: (text: string) => void; +} + +export function useSearchBuffer({ + initialText = '', + onChange, +}: UseSearchBufferProps): TextBuffer { + const { mainAreaWidth } = useUIState(); + const viewportWidth = Math.max( + MIN_VIEWPORT_WIDTH, + mainAreaWidth - VIEWPORT_WIDTH_OFFSET, + ); + + return useTextBuffer({ + initialText, + initialCursorOffset: initialText.length, + viewport: { + width: viewportWidth, + height: 1, + }, + singleLine: true, + onChange, + }); +}