mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-07 20:00:37 -07:00
fix(ui): resolve sticky header regression in tool messages (#16514)
This commit is contained in:
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
|
||||
import type React from 'react';
|
||||
import { Box } from 'ink';
|
||||
import { Box, type DOMElement } from 'ink';
|
||||
import { theme } from '../semantic-colors.js';
|
||||
|
||||
export interface StickyHeaderProps {
|
||||
@@ -14,6 +14,7 @@ export interface StickyHeaderProps {
|
||||
isFirst: boolean;
|
||||
borderColor: string;
|
||||
borderDimColor: boolean;
|
||||
containerRef?: React.RefObject<DOMElement | null>;
|
||||
}
|
||||
|
||||
export const StickyHeader: React.FC<StickyHeaderProps> = ({
|
||||
@@ -22,8 +23,10 @@ export const StickyHeader: React.FC<StickyHeaderProps> = ({
|
||||
isFirst,
|
||||
borderColor,
|
||||
borderDimColor,
|
||||
containerRef,
|
||||
}) => (
|
||||
<Box
|
||||
ref={containerRef}
|
||||
sticky
|
||||
minHeight={1}
|
||||
flexShrink={0}
|
||||
|
||||
@@ -36,19 +36,33 @@ export interface ShellToolMessageProps extends ToolMessageProps {
|
||||
|
||||
export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
|
||||
name,
|
||||
|
||||
description,
|
||||
|
||||
resultDisplay,
|
||||
|
||||
status,
|
||||
|
||||
availableTerminalHeight,
|
||||
|
||||
terminalWidth,
|
||||
|
||||
emphasis = 'medium',
|
||||
|
||||
renderOutputAsMarkdown = true,
|
||||
|
||||
activeShellPtyId,
|
||||
|
||||
embeddedShellFocused,
|
||||
|
||||
ptyId,
|
||||
|
||||
config,
|
||||
|
||||
isFirst,
|
||||
|
||||
borderColor,
|
||||
|
||||
borderDimColor,
|
||||
}) => {
|
||||
const isThisShellFocused =
|
||||
@@ -60,8 +74,13 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
|
||||
embeddedShellFocused;
|
||||
|
||||
const { setEmbeddedShellFocused } = useUIActions();
|
||||
const containerRef = React.useRef<DOMElement>(null);
|
||||
|
||||
const headerRef = React.useRef<DOMElement>(null);
|
||||
|
||||
const contentRef = React.useRef<DOMElement>(null);
|
||||
|
||||
// The shell is focusable if it's the shell command, it's executing, and the interactive shell is enabled.
|
||||
|
||||
const isThisShellFocusable =
|
||||
(name === SHELL_COMMAND_NAME ||
|
||||
name === SHELL_NAME ||
|
||||
@@ -69,17 +88,18 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
|
||||
status === ToolCallStatus.Executing &&
|
||||
config?.getEnableInteractiveShell();
|
||||
|
||||
useMouseClick(
|
||||
containerRef,
|
||||
() => {
|
||||
if (isThisShellFocusable) {
|
||||
setEmbeddedShellFocused(true);
|
||||
}
|
||||
},
|
||||
{ isActive: !!isThisShellFocusable },
|
||||
);
|
||||
const handleFocus = () => {
|
||||
if (isThisShellFocusable) {
|
||||
setEmbeddedShellFocused(true);
|
||||
}
|
||||
};
|
||||
|
||||
useMouseClick(headerRef, handleFocus, { isActive: !!isThisShellFocusable });
|
||||
|
||||
useMouseClick(contentRef, handleFocus, { isActive: !!isThisShellFocusable });
|
||||
|
||||
const wasFocusedRef = React.useRef(false);
|
||||
|
||||
React.useEffect(() => {
|
||||
if (isThisShellFocused) {
|
||||
wasFocusedRef.current = true;
|
||||
@@ -87,12 +107,15 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
|
||||
if (embeddedShellFocused) {
|
||||
setEmbeddedShellFocused(false);
|
||||
}
|
||||
|
||||
wasFocusedRef.current = false;
|
||||
}
|
||||
}, [isThisShellFocused, embeddedShellFocused, setEmbeddedShellFocused]);
|
||||
|
||||
const [lastUpdateTime, setLastUpdateTime] = React.useState<Date | null>(null);
|
||||
|
||||
const [userHasFocused, setUserHasFocused] = React.useState(false);
|
||||
|
||||
const [showFocusHint, setShowFocusHint] = React.useState(false);
|
||||
|
||||
React.useEffect(() => {
|
||||
@@ -123,20 +146,23 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
|
||||
isThisShellFocusable && (showFocusHint || userHasFocused);
|
||||
|
||||
return (
|
||||
<Box ref={containerRef} flexDirection="column" width={terminalWidth}>
|
||||
<>
|
||||
<StickyHeader
|
||||
width={terminalWidth}
|
||||
isFirst={isFirst}
|
||||
borderColor={borderColor}
|
||||
borderDimColor={borderDimColor}
|
||||
containerRef={headerRef}
|
||||
>
|
||||
<ToolStatusIndicator status={status} name={name} />
|
||||
|
||||
<ToolInfo
|
||||
name={name}
|
||||
status={status}
|
||||
description={description}
|
||||
emphasis={emphasis}
|
||||
/>
|
||||
|
||||
{shouldShowFocusHint && (
|
||||
<Box marginLeft={1} flexShrink={0}>
|
||||
<Text color={theme.text.accent}>
|
||||
@@ -144,9 +170,12 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
|
||||
</Text>
|
||||
</Box>
|
||||
)}
|
||||
|
||||
{emphasis === 'high' && <TrailingIndicator />}
|
||||
</StickyHeader>
|
||||
|
||||
<Box
|
||||
ref={contentRef}
|
||||
width={terminalWidth}
|
||||
borderStyle="round"
|
||||
borderColor={borderColor}
|
||||
@@ -173,6 +202,6 @@ export const ShellToolMessage: React.FC<ShellToolMessageProps> = ({
|
||||
</Box>
|
||||
)}
|
||||
</Box>
|
||||
</Box>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -95,7 +95,10 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
||||
isThisShellFocusable && (showFocusHint || userHasFocused);
|
||||
|
||||
return (
|
||||
<Box flexDirection="column" width={terminalWidth}>
|
||||
// It is crucial we don't replace this <> with a Box because otherwise the
|
||||
// sticky header inside it would be sticky to that box rather than to the
|
||||
// parent component of this ToolMessage.
|
||||
<>
|
||||
<StickyHeader
|
||||
width={terminalWidth}
|
||||
isFirst={isFirst}
|
||||
@@ -145,6 +148,6 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
||||
</Box>
|
||||
)}
|
||||
</Box>
|
||||
</Box>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -0,0 +1,206 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { renderWithProviders } from '../../../test-utils/render.js';
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
import { ToolGroupMessage } from './ToolGroupMessage.js';
|
||||
import { ToolCallStatus } from '../../types.js';
|
||||
import {
|
||||
ScrollableList,
|
||||
type ScrollableListRef,
|
||||
} from '../shared/ScrollableList.js';
|
||||
import { Box, Text } from 'ink';
|
||||
import { act, useRef, useEffect } from 'react';
|
||||
import { waitFor } from '../../../test-utils/async.js';
|
||||
import { SHELL_COMMAND_NAME } from '../../constants.js';
|
||||
|
||||
// Mock child components that might be complex
|
||||
vi.mock('../TerminalOutput.js', () => ({
|
||||
TerminalOutput: () => <Text>MockTerminalOutput</Text>,
|
||||
}));
|
||||
|
||||
vi.mock('../AnsiOutput.js', () => ({
|
||||
AnsiOutputText: () => <Text>MockAnsiOutput</Text>,
|
||||
}));
|
||||
|
||||
vi.mock('../GeminiRespondingSpinner.js', () => ({
|
||||
GeminiRespondingSpinner: () => <Text>MockRespondingSpinner</Text>,
|
||||
}));
|
||||
|
||||
vi.mock('./DiffRenderer.js', () => ({
|
||||
DiffRenderer: () => <Text>MockDiff</Text>,
|
||||
}));
|
||||
|
||||
vi.mock('../../utils/MarkdownDisplay.js', () => ({
|
||||
MarkdownDisplay: ({ text }: { text: string }) => <Text>{text}</Text>,
|
||||
}));
|
||||
|
||||
describe('ToolMessage Sticky Header Regression', () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
const createToolCall = (id: string, name: string, resultPrefix: string) => ({
|
||||
callId: id,
|
||||
name,
|
||||
description: `Description for ${name}`,
|
||||
resultDisplay: Array.from(
|
||||
{ length: 10 },
|
||||
(_, i) => `${resultPrefix}-${String(i + 1).padStart(2, '0')}`,
|
||||
).join('\n'),
|
||||
status: ToolCallStatus.Success,
|
||||
confirmationDetails: undefined,
|
||||
renderOutputAsMarkdown: false,
|
||||
});
|
||||
|
||||
it('verifies that multiple ToolMessages in a ToolGroupMessage in a ScrollableList have sticky headers', async () => {
|
||||
const toolCalls = [
|
||||
createToolCall('1', 'tool-1', 'c1'),
|
||||
createToolCall('2', 'tool-2', 'c2'),
|
||||
];
|
||||
|
||||
const terminalWidth = 80;
|
||||
const terminalHeight = 5;
|
||||
|
||||
let listRef: ScrollableListRef<string> | null = null;
|
||||
|
||||
const TestComponent = () => {
|
||||
const internalRef = useRef<ScrollableListRef<string>>(null);
|
||||
useEffect(() => {
|
||||
listRef = internalRef.current;
|
||||
}, []);
|
||||
|
||||
return (
|
||||
<ScrollableList
|
||||
ref={internalRef}
|
||||
data={['item1']}
|
||||
renderItem={() => (
|
||||
<ToolGroupMessage
|
||||
groupId={1}
|
||||
toolCalls={toolCalls}
|
||||
terminalWidth={terminalWidth - 2} // Account for ScrollableList padding
|
||||
/>
|
||||
)}
|
||||
estimatedItemHeight={() => 30}
|
||||
keyExtractor={(item) => item}
|
||||
hasFocus={true}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
const { lastFrame } = renderWithProviders(
|
||||
<Box height={terminalHeight}>
|
||||
<TestComponent />
|
||||
</Box>,
|
||||
{
|
||||
width: terminalWidth,
|
||||
uiState: { terminalWidth },
|
||||
},
|
||||
);
|
||||
|
||||
// Initial state: tool-1 should be visible
|
||||
await waitFor(() => {
|
||||
expect(lastFrame()).toContain('tool-1');
|
||||
});
|
||||
expect(lastFrame()).toContain('Description for tool-1');
|
||||
expect(lastFrame()).toMatchSnapshot();
|
||||
|
||||
// Scroll down so that tool-1's header should be stuck
|
||||
await act(async () => {
|
||||
listRef?.scrollBy(5);
|
||||
});
|
||||
|
||||
// tool-1 header should still be visible because it is sticky
|
||||
await waitFor(() => {
|
||||
expect(lastFrame()).toContain('tool-1');
|
||||
});
|
||||
expect(lastFrame()).toContain('Description for tool-1');
|
||||
// Content lines 1-4 should be scrolled off
|
||||
expect(lastFrame()).not.toContain('c1-01');
|
||||
expect(lastFrame()).not.toContain('c1-04');
|
||||
// Line 6 and 7 should be visible (terminalHeight=5 means only 2 lines of content show below 3-line header)
|
||||
expect(lastFrame()).toContain('c1-06');
|
||||
expect(lastFrame()).toContain('c1-07');
|
||||
expect(lastFrame()).toMatchSnapshot();
|
||||
|
||||
// Scroll further so tool-1 is completely gone and tool-2's header should be stuck
|
||||
await act(async () => {
|
||||
listRef?.scrollBy(17);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(lastFrame()).toContain('tool-2');
|
||||
});
|
||||
expect(lastFrame()).toContain('Description for tool-2');
|
||||
// tool-1 should be gone now (both header and content)
|
||||
expect(lastFrame()).not.toContain('tool-1');
|
||||
expect(lastFrame()).toMatchSnapshot();
|
||||
});
|
||||
|
||||
it('verifies that ShellToolMessage in a ToolGroupMessage in a ScrollableList has sticky headers', async () => {
|
||||
const toolCalls = [
|
||||
{
|
||||
...createToolCall('1', SHELL_COMMAND_NAME, 'shell'),
|
||||
status: ToolCallStatus.Success,
|
||||
},
|
||||
];
|
||||
|
||||
const terminalWidth = 80;
|
||||
const terminalHeight = 5;
|
||||
|
||||
let listRef: ScrollableListRef<string> | null = null;
|
||||
|
||||
const TestComponent = () => {
|
||||
const internalRef = useRef<ScrollableListRef<string>>(null);
|
||||
useEffect(() => {
|
||||
listRef = internalRef.current;
|
||||
}, []);
|
||||
|
||||
return (
|
||||
<ScrollableList
|
||||
ref={internalRef}
|
||||
data={['item1']}
|
||||
renderItem={() => (
|
||||
<ToolGroupMessage
|
||||
groupId={1}
|
||||
toolCalls={toolCalls}
|
||||
terminalWidth={terminalWidth - 2}
|
||||
/>
|
||||
)}
|
||||
estimatedItemHeight={() => 30}
|
||||
keyExtractor={(item) => item}
|
||||
hasFocus={true}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
const { lastFrame } = renderWithProviders(
|
||||
<Box height={terminalHeight}>
|
||||
<TestComponent />
|
||||
</Box>,
|
||||
{
|
||||
width: terminalWidth,
|
||||
uiState: { terminalWidth },
|
||||
},
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(lastFrame()).toContain(SHELL_COMMAND_NAME);
|
||||
});
|
||||
expect(lastFrame()).toMatchSnapshot();
|
||||
|
||||
// Scroll down
|
||||
await act(async () => {
|
||||
listRef?.scrollBy(5);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(lastFrame()).toContain(SHELL_COMMAND_NAME);
|
||||
});
|
||||
expect(lastFrame()).toContain('shell-06');
|
||||
expect(lastFrame()).toMatchSnapshot();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,41 @@
|
||||
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
|
||||
|
||||
exports[`ToolMessage Sticky Header Regression > verifies that ShellToolMessage in a ToolGroupMessage in a ScrollableList has sticky headers 1`] = `
|
||||
"╭────────────────────────────────────────────────────────────────────────────╮ █
|
||||
│ ✓ Shell Command Description for Shell Command │ █
|
||||
│ │
|
||||
│ shell-01 │
|
||||
│ shell-02 │"
|
||||
`;
|
||||
|
||||
exports[`ToolMessage Sticky Header Regression > verifies that ShellToolMessage in a ToolGroupMessage in a ScrollableList has sticky headers 2`] = `
|
||||
"╭────────────────────────────────────────────────────────────────────────────╮
|
||||
│ ✓ Shell Command Description for Shell Command │ ▄
|
||||
│────────────────────────────────────────────────────────────────────────────│ █
|
||||
│ shell-06 │ ▀
|
||||
│ shell-07 │"
|
||||
`;
|
||||
|
||||
exports[`ToolMessage Sticky Header Regression > verifies that multiple ToolMessages in a ToolGroupMessage in a ScrollableList have sticky headers 1`] = `
|
||||
"╭────────────────────────────────────────────────────────────────────────────╮ █
|
||||
│ ✓ tool-1 Description for tool-1 │
|
||||
│ │
|
||||
│ c1-01 │
|
||||
│ c1-02 │"
|
||||
`;
|
||||
|
||||
exports[`ToolMessage Sticky Header Regression > verifies that multiple ToolMessages in a ToolGroupMessage in a ScrollableList have sticky headers 2`] = `
|
||||
"╭────────────────────────────────────────────────────────────────────────────╮
|
||||
│ ✓ tool-1 Description for tool-1 │ █
|
||||
│────────────────────────────────────────────────────────────────────────────│
|
||||
│ c1-06 │
|
||||
│ c1-07 │"
|
||||
`;
|
||||
|
||||
exports[`ToolMessage Sticky Header Regression > verifies that multiple ToolMessages in a ToolGroupMessage in a ScrollableList have sticky headers 3`] = `
|
||||
"│ │
|
||||
│ ✓ tool-2 Description for tool-2 │
|
||||
│────────────────────────────────────────────────────────────────────────────│
|
||||
│ c2-10 │
|
||||
╰────────────────────────────────────────────────────────────────────────────╯ █"
|
||||
`;
|
||||
Reference in New Issue
Block a user