diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx index 9bc8f05298..d219b41ca8 100644 --- a/packages/cli/src/ui/components/SettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx @@ -307,6 +307,26 @@ describe('SettingsDialog', () => { // Use snapshot to capture visual layout including indicators expect(output).toMatchSnapshot(); }); + + it('should use almost full height of the window but no more when the window height is 25 rows', async () => { + const settings = createMockSettings(); + const onSelect = vi.fn(); + + // Render with a fixed height of 25 rows + const { lastFrame } = renderDialog(settings, onSelect, { + availableTerminalHeight: 25, + }); + + // Wait for the dialog to render + await waitFor(() => { + const output = lastFrame(); + expect(output).toBeDefined(); + const lines = output!.split('\n'); + + expect(lines.length).toBeGreaterThanOrEqual(24); + expect(lines.length).toBeLessThanOrEqual(25); + }); + }); }); describe('Setting Descriptions', () => { @@ -1072,6 +1092,87 @@ describe('SettingsDialog', () => { }); }); + describe('Restart and Search Conflict Regression', () => { + it('should prioritize restart request over search text box when showRestartPrompt is true', async () => { + vi.mocked(getSettingsSchema).mockReturnValue(TOOLS_SHELL_FAKE_SCHEMA); + const settings = createMockSettings(); + const onRestartRequest = vi.fn(); + + const { stdin, lastFrame, unmount } = renderDialog(settings, vi.fn(), { + onRestartRequest, + }); + + // Wait for initial render + await waitFor(() => expect(lastFrame()).toContain('Show Color')); + + // Navigate to "Enable Interactive Shell" (second item in TOOLS_SHELL_FAKE_SCHEMA) + act(() => { + stdin.write(TerminalKeys.DOWN_ARROW); + }); + + // Wait for navigation to complete + await waitFor(() => + expect(lastFrame()).toContain('● Enable Interactive Shell'), + ); + + // Toggle it to trigger restart required + act(() => { + stdin.write(TerminalKeys.ENTER); + }); + + await waitFor(() => { + expect(lastFrame()).toContain( + 'To see changes, Gemini CLI must be restarted', + ); + }); + + // Press 'r' - it should call onRestartRequest, NOT be handled by search + act(() => { + stdin.write('r'); + }); + + await waitFor(() => { + expect(onRestartRequest).toHaveBeenCalled(); + }); + + unmount(); + }); + + it('should hide search box when showRestartPrompt is true', async () => { + vi.mocked(getSettingsSchema).mockReturnValue(TOOLS_SHELL_FAKE_SCHEMA); + const settings = createMockSettings(); + + const { stdin, lastFrame, unmount } = renderDialog(settings, vi.fn()); + + // Search box should be visible initially (searchPlaceholder) + expect(lastFrame()).toContain('Search to filter'); + + // Navigate to "Enable Interactive Shell" and toggle it + act(() => { + stdin.write(TerminalKeys.DOWN_ARROW); + }); + + await waitFor(() => + expect(lastFrame()).toContain('● Enable Interactive Shell'), + ); + + act(() => { + stdin.write(TerminalKeys.ENTER); + }); + + await waitFor(() => { + expect(lastFrame()).toContain( + 'To see changes, Gemini CLI must be restarted', + ); + }); + + // Search box should now be hidden + expect(lastFrame()).not.toContain('Search to filter'); + + unmount(); + }); + }); + describe('String Settings Editing', () => { it('should allow editing and committing a string setting', async () => { let settings = createMockSettings({ 'a.string.setting': 'initial' }); diff --git a/packages/cli/src/ui/components/SettingsDialog.tsx b/packages/cli/src/ui/components/SettingsDialog.tsx index f41d9cd2ed..76c6a27e6e 100644 --- a/packages/cli/src/ui/components/SettingsDialog.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.tsx @@ -601,6 +601,7 @@ export function SettingsDialog({ (key: Key, _currentItem: SettingsDialogItem | undefined): boolean => { // 'r' key for restart if (showRestartPrompt && key.sequence === 'r') { + saveRestartRequiredSettings(); setShowRestartPrompt(false); setModifiedSettings(new Set()); setRestartRequiredSettings(new Set()); @@ -609,80 +610,90 @@ export function SettingsDialog({ } return false; }, - [showRestartPrompt, onRestartRequest], + [showRestartPrompt, onRestartRequest, saveRestartRequiredSettings], ); // Calculate effective max items and scope visibility based on terminal height - const { effectiveMaxItemsToShow, showScopeSelection } = useMemo(() => { - // Only show scope selector if we have a workspace - const hasWorkspace = settings.workspace.path !== undefined; + const { effectiveMaxItemsToShow, showScopeSelection, showSearch } = + useMemo(() => { + // Only show scope selector if we have a workspace + const hasWorkspace = settings.workspace.path !== undefined; - if (!availableTerminalHeight) { - return { - effectiveMaxItemsToShow: Math.min(MAX_ITEMS_TO_SHOW, items.length), - showScopeSelection: hasWorkspace, - }; - } + // Search box is hidden when restart prompt is shown to save space and avoid key conflicts + const shouldShowSearch = !showRestartPrompt; - // Layout constants - const DIALOG_PADDING = 2; // Top and bottom borders - const SETTINGS_TITLE_HEIGHT = 1; - const SEARCH_BOX_HEIGHT = 3; - const SCROLL_ARROWS_HEIGHT = 2; - const SPACING_HEIGHT = 2; - const SCOPE_SELECTION_HEIGHT = 4; - const BOTTOM_HELP_TEXT_HEIGHT = 1; - const RESTART_PROMPT_HEIGHT = showRestartPrompt ? 1 : 0; - const ITEM_HEIGHT = 3; // Label + description + spacing - - const currentAvailableHeight = availableTerminalHeight - DIALOG_PADDING; - - const baseFixedHeight = - SETTINGS_TITLE_HEIGHT + - SEARCH_BOX_HEIGHT + - SCROLL_ARROWS_HEIGHT + - SPACING_HEIGHT + - BOTTOM_HELP_TEXT_HEIGHT + - RESTART_PROMPT_HEIGHT; - - // Calculate max items with scope selector - const heightWithScope = baseFixedHeight + SCOPE_SELECTION_HEIGHT; - const availableForItemsWithScope = currentAvailableHeight - heightWithScope; - const maxItemsWithScope = Math.max( - 1, - Math.floor(availableForItemsWithScope / ITEM_HEIGHT), - ); - - // Calculate max items without scope selector - const availableForItemsWithoutScope = - currentAvailableHeight - baseFixedHeight; - const maxItemsWithoutScope = Math.max( - 1, - Math.floor(availableForItemsWithoutScope / ITEM_HEIGHT), - ); - - // In small terminals, hide scope selector if it would allow more items to show - let shouldShowScope = hasWorkspace; - let maxItems = maxItemsWithScope; - - if (hasWorkspace && availableTerminalHeight < 25) { - // Hide scope selector if it gains us more than 1 extra item - if (maxItemsWithoutScope > maxItemsWithScope + 1) { - shouldShowScope = false; - maxItems = maxItemsWithoutScope; + if (!availableTerminalHeight) { + return { + effectiveMaxItemsToShow: Math.min(MAX_ITEMS_TO_SHOW, items.length), + showScopeSelection: hasWorkspace, + showSearch: shouldShowSearch, + }; } - } - return { - effectiveMaxItemsToShow: Math.min(maxItems, items.length), - showScopeSelection: shouldShowScope, - }; - }, [ - availableTerminalHeight, - items.length, - settings.workspace.path, - showRestartPrompt, - ]); + // Layout constants based on BaseSettingsDialog structure: + // 4 for border (2) and padding (2) + const DIALOG_PADDING = 4; + const SETTINGS_TITLE_HEIGHT = 1; + // 3 for box + 1 for marginTop + 1 for spacing after + const SEARCH_SECTION_HEIGHT = shouldShowSearch ? 5 : 0; + const SCROLL_ARROWS_HEIGHT = 2; + const ITEMS_SPACING_AFTER = 1; + // 1 for Label + 3 for Scope items + 1 for spacing after + const SCOPE_SECTION_HEIGHT = hasWorkspace ? 5 : 0; + const HELP_TEXT_HEIGHT = 1; + const RESTART_PROMPT_HEIGHT = showRestartPrompt ? 1 : 0; + const ITEM_HEIGHT = 3; // Label + description + spacing + + const currentAvailableHeight = availableTerminalHeight - DIALOG_PADDING; + + const baseFixedHeight = + SETTINGS_TITLE_HEIGHT + + SEARCH_SECTION_HEIGHT + + SCROLL_ARROWS_HEIGHT + + ITEMS_SPACING_AFTER + + HELP_TEXT_HEIGHT + + RESTART_PROMPT_HEIGHT; + + // Calculate max items with scope selector + const heightWithScope = baseFixedHeight + SCOPE_SECTION_HEIGHT; + const availableForItemsWithScope = + currentAvailableHeight - heightWithScope; + const maxItemsWithScope = Math.max( + 1, + Math.floor(availableForItemsWithScope / ITEM_HEIGHT), + ); + + // Calculate max items without scope selector + const availableForItemsWithoutScope = + currentAvailableHeight - baseFixedHeight; + const maxItemsWithoutScope = Math.max( + 1, + Math.floor(availableForItemsWithoutScope / ITEM_HEIGHT), + ); + + // In small terminals, hide scope selector if it would allow more items to show + let shouldShowScope = hasWorkspace; + let maxItems = maxItemsWithScope; + + if (hasWorkspace && availableTerminalHeight < 25) { + // Hide scope selector if it gains us more than 1 extra item + if (maxItemsWithoutScope > maxItemsWithScope + 1) { + shouldShowScope = false; + maxItems = maxItemsWithoutScope; + } + } + + return { + effectiveMaxItemsToShow: Math.min(maxItems, items.length), + showScopeSelection: shouldShowScope, + showSearch: shouldShowSearch, + }; + }, [ + availableTerminalHeight, + items.length, + settings.workspace.path, + showRestartPrompt, + ]); // Footer content for restart prompt const footerContent = showRestartPrompt ? ( @@ -695,7 +706,8 @@ export function SettingsDialog({ return (