fix(cli): extensions dialog UX polish (#19685)

2026-03-10 22:21:22 -07:00 · 2026-02-20 13:08:24 -08:00
parent 089aec8b8d
commit 9a8e5d3940
8 changed files with 203 additions and 80 deletions
--- 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
--- 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 () => {
--- 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<T extends GenericListItem>({
    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<T extends GenericListItem>({
    { 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<T extends GenericListItem>({
    isActive: boolean,
    labelWidth: number,
  ) => (
-    <Box flexDirection="column">
-      <Text
-        color={isActive ? theme.status.success : theme.text.primary}
-        bold={isActive}
-      >
-        {isActive ? '> ' : '  '}
-        {item.label.padEnd(labelWidth)}
-      </Text>
-      {item.description && (
-        <Box marginLeft={2}>
+    <Box flexDirection="row" alignItems="flex-start">
+      <Box minWidth={2} flexShrink={0}>
+        <Text color={isActive ? theme.status.success : theme.text.secondary}>
+          {isActive ? '●' : ''}
+        </Text>
+      </Box>
+      <Box flexDirection="column" flexGrow={1} minWidth={0}>
+        <Text color={isActive ? theme.status.success : theme.text.primary}>
+          {item.label.padEnd(labelWidth)}
+        </Text>
+        {item.description && (
          <Text color={theme.text.secondary} wrap="truncate-end">
            {item.description}
          </Text>
-        </Box>
-      )}
+        )}
+      </Box>
    </Box>
  );

@@ -204,16 +220,28 @@ export function SearchableList<T extends GenericListItem>({
            <Text color={theme.text.secondary}>No items found.</Text>
          </Box>
        ) : (
-          visibleItems.map((item, index) => {
-            const isSelected = activeIndex === scrollOffset + index;
-            return (
-              <Box key={item.key} marginBottom={1}>
-                {renderItem
-                  ? renderItem(item, isSelected, maxLabelWidth)
-                  : defaultRenderItem(item, isSelected, maxLabelWidth)}
+          <>
+            {filteredItems.length > maxItemsToShow && (
+              <Box marginX={1}>
+                <Text color={theme.text.secondary}>▲</Text>
              </Box>
-            );
-          })
+            )}
+            {visibleItems.map((item, index) => {
+              const isSelected = activeIndex === scrollOffset + index;
+              return (
+                <Box key={item.key} marginBottom={1} marginX={1}>
+                  {renderItem
+                    ? renderItem(item, isSelected, maxLabelWidth)
+                    : defaultRenderItem(item, isSelected, maxLabelWidth)}
+                </Box>
+              );
+            })}
+            {filteredItems.length > maxItemsToShow && (
+              <Box marginX={1}>
+                <Text color={theme.text.secondary}>▼</Text>
+              </Box>
+            )}
+          </>
        )}
      </Box>

--- 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
 "
 `;
--- 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<typeof useUIState>);

    vi.mocked(useConfig).mockReturnValue({
--- 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({
              <Text
                color={isActive ? theme.status.success : theme.text.secondary}
              >
-                {isActive ? '> ' : '  '}
+                {isActive ? '● ' : '  '}
              </Text>
            </Box>
            <Box flexShrink={0}>
@@ -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 (
      <Box padding={1}>
@@ -191,7 +212,7 @@ export function ExtensionRegistryView({
      renderItem={renderItem}
      header={header}
      footer={footer}
-      maxItemsToShow={8}
+      maxItemsToShow={maxItemsToShow}
      useSearch={useRegistrySearch}
      onSearch={search}
      resetSelectionOnItemsChange={true}
--- 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<T extends GenericListItem> {
  filteredItems: T[];
@@ -31,26 +25,22 @@ export function useRegistrySearch<T extends GenericListItem>(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;
--- a/packages/cli/src/ui/hooks/useSearchBuffer.ts
+++ 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,
+  });
+}