mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-13 05:12:55 -07:00
fix: optimize height calculations for ask_user dialog (#19017)
This commit is contained in:
@@ -7,7 +7,7 @@
|
|||||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
import { Box } from 'ink';
|
import { Box } from 'ink';
|
||||||
import { ToolConfirmationQueue } from './ToolConfirmationQueue.js';
|
import { ToolConfirmationQueue } from './ToolConfirmationQueue.js';
|
||||||
import { StreamingState } from '../types.js';
|
import { StreamingState, ToolCallStatus } from '../types.js';
|
||||||
import { renderWithProviders } from '../../test-utils/render.js';
|
import { renderWithProviders } from '../../test-utils/render.js';
|
||||||
import { waitFor } from '../../test-utils/async.js';
|
import { waitFor } from '../../test-utils/async.js';
|
||||||
import { type Config, CoreToolCallStatus } from '@google/gemini-cli-core';
|
import { type Config, CoreToolCallStatus } from '@google/gemini-cli-core';
|
||||||
@@ -223,6 +223,58 @@ describe('ToolConfirmationQueue', () => {
|
|||||||
expect(lastFrame()).toMatchSnapshot();
|
expect(lastFrame()).toMatchSnapshot();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('provides more height for ask_user by subtracting less overhead', async () => {
|
||||||
|
const confirmingTool = {
|
||||||
|
tool: {
|
||||||
|
callId: 'call-1',
|
||||||
|
name: 'ask_user',
|
||||||
|
description: 'ask user',
|
||||||
|
status: ToolCallStatus.Confirming,
|
||||||
|
confirmationDetails: {
|
||||||
|
type: 'ask_user' as const,
|
||||||
|
questions: [
|
||||||
|
{
|
||||||
|
type: 'choice',
|
||||||
|
header: 'Height Test',
|
||||||
|
question: 'Line 1\nLine 2\nLine 3\nLine 4\nLine 5\nLine 6',
|
||||||
|
options: [{ label: 'Option 1', description: 'Desc' }],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
index: 1,
|
||||||
|
total: 1,
|
||||||
|
};
|
||||||
|
|
||||||
|
const { lastFrame } = renderWithProviders(
|
||||||
|
<ToolConfirmationQueue
|
||||||
|
confirmingTool={confirmingTool as unknown as ConfirmingToolState}
|
||||||
|
/>,
|
||||||
|
{
|
||||||
|
config: mockConfig,
|
||||||
|
uiState: {
|
||||||
|
terminalWidth: 80,
|
||||||
|
terminalHeight: 40,
|
||||||
|
availableTerminalHeight: 20,
|
||||||
|
constrainHeight: true,
|
||||||
|
streamingState: StreamingState.WaitingForConfirmation,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
// Calculation:
|
||||||
|
// availableTerminalHeight: 20 -> maxHeight: 19 (20-1)
|
||||||
|
// hideToolIdentity is true for ask_user -> subtracts 4 instead of 6
|
||||||
|
// availableContentHeight = 19 - 4 = 15
|
||||||
|
// ToolConfirmationMessage handlesOwnUI=true -> returns full 15
|
||||||
|
// AskUserDialog uses 15 lines to render its multi-line question and options.
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(lastFrame()).toContain('Line 6');
|
||||||
|
expect(lastFrame()).not.toContain('lines hidden');
|
||||||
|
});
|
||||||
|
expect(lastFrame()).toMatchSnapshot();
|
||||||
|
});
|
||||||
|
|
||||||
it('does not render expansion hint when constrainHeight is false', () => {
|
it('does not render expansion hint when constrainHeight is false', () => {
|
||||||
const longDiff = 'line\n'.repeat(50);
|
const longDiff = 'line\n'.repeat(50);
|
||||||
const confirmingTool = {
|
const confirmingTool = {
|
||||||
|
|||||||
@@ -60,6 +60,12 @@ export const ToolConfirmationQueue: React.FC<ToolConfirmationQueueProps> = ({
|
|||||||
? Math.max(uiAvailableHeight - 1, 4)
|
? Math.max(uiAvailableHeight - 1, 4)
|
||||||
: Math.floor(terminalHeight * 0.5);
|
: Math.floor(terminalHeight * 0.5);
|
||||||
|
|
||||||
|
const isRoutine =
|
||||||
|
tool.confirmationDetails?.type === 'ask_user' ||
|
||||||
|
tool.confirmationDetails?.type === 'exit_plan_mode';
|
||||||
|
const borderColor = isRoutine ? theme.status.success : theme.status.warning;
|
||||||
|
const hideToolIdentity = isRoutine;
|
||||||
|
|
||||||
// ToolConfirmationMessage needs to know the height available for its OWN content.
|
// ToolConfirmationMessage needs to know the height available for its OWN content.
|
||||||
// We subtract the lines used by the Queue wrapper:
|
// We subtract the lines used by the Queue wrapper:
|
||||||
// - 2 lines for the rounded border
|
// - 2 lines for the rounded border
|
||||||
@@ -67,15 +73,9 @@ export const ToolConfirmationQueue: React.FC<ToolConfirmationQueueProps> = ({
|
|||||||
// - 2 lines for Tool Identity (text + margin)
|
// - 2 lines for Tool Identity (text + margin)
|
||||||
const availableContentHeight =
|
const availableContentHeight =
|
||||||
constrainHeight && !isAlternateBuffer
|
constrainHeight && !isAlternateBuffer
|
||||||
? Math.max(maxHeight - 6, 4)
|
? Math.max(maxHeight - (hideToolIdentity ? 4 : 6), 4)
|
||||||
: undefined;
|
: undefined;
|
||||||
|
|
||||||
const isRoutine =
|
|
||||||
tool.confirmationDetails?.type === 'ask_user' ||
|
|
||||||
tool.confirmationDetails?.type === 'exit_plan_mode';
|
|
||||||
const borderColor = isRoutine ? theme.status.success : theme.status.warning;
|
|
||||||
const hideToolIdentity = isRoutine;
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<OverflowProvider>
|
<OverflowProvider>
|
||||||
<Box flexDirection="column" width={mainAreaWidth} flexShrink={0}>
|
<Box flexDirection="column" width={mainAreaWidth} flexShrink={0}>
|
||||||
|
|||||||
@@ -40,6 +40,25 @@ exports[`ToolConfirmationQueue > does not render expansion hint when constrainHe
|
|||||||
╰──────────────────────────────────────────────────────────────────────────────╯"
|
╰──────────────────────────────────────────────────────────────────────────────╯"
|
||||||
`;
|
`;
|
||||||
|
|
||||||
|
exports[`ToolConfirmationQueue > provides more height for ask_user by subtracting less overhead 1`] = `
|
||||||
|
"╭──────────────────────────────────────────────────────────────────────────────╮
|
||||||
|
│ Answer Questions │
|
||||||
|
│ │
|
||||||
|
│ Line 1 │
|
||||||
|
│ Line 2 │
|
||||||
|
│ Line 3 │
|
||||||
|
│ Line 4 │
|
||||||
|
│ Line 5 │
|
||||||
|
│ Line 6 │
|
||||||
|
│ │
|
||||||
|
│ ● 1. Option 1 │
|
||||||
|
│ Desc │
|
||||||
|
│ 2. Enter a custom value │
|
||||||
|
│ │
|
||||||
|
│ Enter to select · ↑/↓ to navigate · Esc to cancel │
|
||||||
|
╰──────────────────────────────────────────────────────────────────────────────╯"
|
||||||
|
`;
|
||||||
|
|
||||||
exports[`ToolConfirmationQueue > renders AskUser tool confirmation with Success color 1`] = `
|
exports[`ToolConfirmationQueue > renders AskUser tool confirmation with Success color 1`] = `
|
||||||
"╭──────────────────────────────────────────────────────────────────────────────╮
|
"╭──────────────────────────────────────────────────────────────────────────────╮
|
||||||
│ Answer Questions │
|
│ Answer Questions │
|
||||||
|
|||||||
@@ -235,6 +235,10 @@ export const ToolConfirmationMessage: React.FC<
|
|||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (handlesOwnUI) {
|
||||||
|
return availableTerminalHeight;
|
||||||
|
}
|
||||||
|
|
||||||
// Calculate the vertical space (in lines) consumed by UI elements
|
// Calculate the vertical space (in lines) consumed by UI elements
|
||||||
// surrounding the main body content.
|
// surrounding the main body content.
|
||||||
const PADDING_OUTER_Y = 2; // Main container has `padding={1}` (top & bottom).
|
const PADDING_OUTER_Y = 2; // Main container has `padding={1}` (top & bottom).
|
||||||
@@ -253,7 +257,7 @@ export const ToolConfirmationMessage: React.FC<
|
|||||||
1; // Reserve one line for 'ShowMoreLines' hint
|
1; // Reserve one line for 'ShowMoreLines' hint
|
||||||
|
|
||||||
return Math.max(availableTerminalHeight - surroundingElementsHeight, 1);
|
return Math.max(availableTerminalHeight - surroundingElementsHeight, 1);
|
||||||
}, [availableTerminalHeight, getOptions]);
|
}, [availableTerminalHeight, getOptions, handlesOwnUI]);
|
||||||
|
|
||||||
const { question, bodyContent, options } = useMemo(() => {
|
const { question, bodyContent, options } = useMemo(() => {
|
||||||
let bodyContent: React.ReactNode | null = null;
|
let bodyContent: React.ReactNode | null = null;
|
||||||
|
|||||||
Reference in New Issue
Block a user