Refactor: Improve UI rendering and address code review comments

This commit addresses several code review comments primarily focused on improving the rendering and stability of the CLI UI.

Key changes include:
- Passing `isPending` and `availableTerminalHeight` props to `MarkdownDisplay` to enable more intelligent rendering of content, especially for pending messages and code blocks.
- Adjusting height calculations in `ToolGroupMessage` and `ToolMessage` to more accurately reflect available space.
- Refining the logic in `App.tsx` for measuring and utilizing terminal height, including renaming `footerRef` to `mainControlsRef` for clarity.
- Ensuring consistent prop drilling for `isPending` and `availableTerminalHeight` through `HistoryItemDisplay`, `GeminiMessage`, and `GeminiMessageContent`.
- In `MarkdownDisplay`, when `isPending` is true and content exceeds `availableTerminalHeight`, the code block will now be truncated with a "... generating more ..." message. If there's insufficient space even for the
message, a simpler "... code is being written ..." will be shown.
This commit is contained in:
Taylor Mullen 2025-05-15 22:56:03 -07:00 committed by N. Taylor Mullen
parent 33743d347b
commit 9c46acc793
7 changed files with 114 additions and 29 deletions

View File

@ -5,8 +5,9 @@
*/ */
import { useCallback, useEffect, useMemo, useState, useRef } from 'react'; import { useCallback, useEffect, useMemo, useState, useRef } from 'react';
import { Box, DOMElement, measureElement, Static, Text, useStdout } from 'ink'; import { Box, DOMElement, measureElement, Static, Text } from 'ink';
import { StreamingState, type HistoryItem } from './types.js'; import { StreamingState, type HistoryItem } from './types.js';
import { useTerminalSize } from './hooks/useTerminalSize.js';
import { useGeminiStream } from './hooks/useGeminiStream.js'; import { useGeminiStream } from './hooks/useGeminiStream.js';
import { useLoadingIndicator } from './hooks/useLoadingIndicator.js'; import { useLoadingIndicator } from './hooks/useLoadingIndicator.js';
import { useThemeCommand } from './hooks/useThemeCommand.js'; import { useThemeCommand } from './hooks/useThemeCommand.js';
@ -56,8 +57,7 @@ export const App = ({
const [debugMessage, setDebugMessage] = useState<string>(''); const [debugMessage, setDebugMessage] = useState<string>('');
const [showHelp, setShowHelp] = useState<boolean>(false); const [showHelp, setShowHelp] = useState<boolean>(false);
const [themeError, setThemeError] = useState<string | null>(null); const [themeError, setThemeError] = useState<string | null>(null);
const [availableTerminalHeight, setAvailableTerminalHeight] = const [footerHeight, setFooterHeight] = useState<number>(0);
useState<number>(0);
const { const {
isThemeDialogOpen, isThemeDialogOpen,
openThemeDialog, openThemeDialog,
@ -195,26 +195,24 @@ export const App = ({
// --- Render Logic --- // --- Render Logic ---
// Get terminal dimensions const { rows: terminalHeight, columns: terminalWidth } = useTerminalSize();
const mainControlsRef = useRef<DOMElement>(null);
const { stdout } = useStdout();
const terminalWidth = stdout?.columns ?? 80;
const terminalHeight = stdout?.rows ?? 24;
const footerRef = useRef<DOMElement>(null);
const pendingHistoryItemRef = useRef<DOMElement>(null); const pendingHistoryItemRef = useRef<DOMElement>(null);
// Calculate width for suggestions, leave some padding // Calculate width for suggestions, leave some padding
const suggestionsWidth = Math.max(60, Math.floor(terminalWidth * 0.8)); const suggestionsWidth = Math.max(60, Math.floor(terminalWidth * 0.8));
useEffect(() => { useEffect(() => {
const staticExtraHeight = /* margins and padding */ 3; if (mainControlsRef.current) {
const fullFooterMeasurement = measureElement(footerRef.current!); const fullFooterMeasurement = measureElement(mainControlsRef.current);
const fullFooterHeight = fullFooterMeasurement.height; setFooterHeight(fullFooterMeasurement.height);
}
}, [terminalHeight]); // Re-calculate if terminalHeight changes, as it might affect footer's rendered height.
setAvailableTerminalHeight( const availableTerminalHeight = useMemo(() => {
terminalHeight - fullFooterHeight - staticExtraHeight, const staticExtraHeight = /* margins and padding */ 3;
); return terminalHeight - footerHeight - staticExtraHeight;
}, [terminalHeight]); }, [terminalHeight, footerHeight]);
useEffect(() => { useEffect(() => {
if (!pendingHistoryItem) { if (!pendingHistoryItem) {
@ -260,7 +258,14 @@ export const App = ({
<Header /> <Header />
<Tips /> <Tips />
</Box>, </Box>,
...history.map((h) => <HistoryItemDisplay availableTerminalHeight={availableTerminalHeight} key={h.id} item={h} />), ...history.map((h) => (
<HistoryItemDisplay
availableTerminalHeight={availableTerminalHeight}
key={h.id}
item={h}
isPending={false}
/>
)),
]} ]}
> >
{(item) => item} {(item) => item}
@ -272,12 +277,13 @@ export const App = ({
// TODO(taehykim): It seems like references to ids aren't necessary in // TODO(taehykim): It seems like references to ids aren't necessary in
// HistoryItemDisplay. Refactor later. Use a fake id for now. // HistoryItemDisplay. Refactor later. Use a fake id for now.
item={{ ...pendingHistoryItem, id: 0 }} item={{ ...pendingHistoryItem, id: 0 }}
isPending={true}
/> />
</Box> </Box>
)} )}
{showHelp && <Help commands={slashCommands} />} {showHelp && <Help commands={slashCommands} />}
<Box flexDirection="column" ref={footerRef}> <Box flexDirection="column" ref={mainControlsRef}>
{startupWarnings.length > 0 && ( {startupWarnings.length > 0 && (
<Box <Box
borderStyle="round" borderStyle="round"

View File

@ -17,18 +17,30 @@ import { Box } from 'ink';
interface HistoryItemDisplayProps { interface HistoryItemDisplayProps {
item: HistoryItem; item: HistoryItem;
availableTerminalHeight: number; availableTerminalHeight: number;
isPending: boolean;
} }
export const HistoryItemDisplay: React.FC<HistoryItemDisplayProps> = ({ export const HistoryItemDisplay: React.FC<HistoryItemDisplayProps> = ({
item, item,
availableTerminalHeight, availableTerminalHeight,
isPending,
}) => ( }) => (
<Box flexDirection="column" key={item.id}> <Box flexDirection="column" key={item.id}>
{/* Render standard message types */} {/* Render standard message types */}
{item.type === 'user' && <UserMessage text={item.text} />} {item.type === 'user' && <UserMessage text={item.text} />}
{item.type === 'gemini' && <GeminiMessage text={item.text} />} {item.type === 'gemini' && (
<GeminiMessage
text={item.text}
isPending={isPending}
availableTerminalHeight={availableTerminalHeight}
/>
)}
{item.type === 'gemini_content' && ( {item.type === 'gemini_content' && (
<GeminiMessageContent text={item.text} /> <GeminiMessageContent
text={item.text}
isPending={isPending}
availableTerminalHeight={availableTerminalHeight}
/>
)} )}
{item.type === 'info' && <InfoMessage text={item.text} />} {item.type === 'info' && <InfoMessage text={item.text} />}
{item.type === 'error' && <ErrorMessage text={item.text} />} {item.type === 'error' && <ErrorMessage text={item.text} />}

View File

@ -11,9 +11,15 @@ import { Colors } from '../../colors.js';
interface GeminiMessageProps { interface GeminiMessageProps {
text: string; text: string;
isPending: boolean;
availableTerminalHeight: number;
} }
export const GeminiMessage: React.FC<GeminiMessageProps> = ({ text }) => { export const GeminiMessage: React.FC<GeminiMessageProps> = ({
text,
isPending,
availableTerminalHeight,
}) => {
const prefix = '✦ '; const prefix = '✦ ';
const prefixWidth = prefix.length; const prefixWidth = prefix.length;
@ -23,7 +29,11 @@ export const GeminiMessage: React.FC<GeminiMessageProps> = ({ text }) => {
<Text color={Colors.AccentPurple}>{prefix}</Text> <Text color={Colors.AccentPurple}>{prefix}</Text>
</Box> </Box>
<Box flexGrow={1} flexDirection="column"> <Box flexGrow={1} flexDirection="column">
<MarkdownDisplay text={text} /> <MarkdownDisplay
text={text}
isPending={isPending}
availableTerminalHeight={availableTerminalHeight}
/>
</Box> </Box>
</Box> </Box>
); );

View File

@ -10,6 +10,8 @@ import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js';
interface GeminiMessageContentProps { interface GeminiMessageContentProps {
text: string; text: string;
isPending: boolean;
availableTerminalHeight: number;
} }
/* /*
@ -20,13 +22,19 @@ interface GeminiMessageContentProps {
*/ */
export const GeminiMessageContent: React.FC<GeminiMessageContentProps> = ({ export const GeminiMessageContent: React.FC<GeminiMessageContentProps> = ({
text, text,
isPending,
availableTerminalHeight,
}) => { }) => {
const originalPrefix = '✦ '; const originalPrefix = '✦ ';
const prefixWidth = originalPrefix.length; const prefixWidth = originalPrefix.length;
return ( return (
<Box flexDirection="column" paddingLeft={prefixWidth}> <Box flexDirection="column" paddingLeft={prefixWidth}>
<MarkdownDisplay text={text} /> <MarkdownDisplay
text={text}
isPending={isPending}
availableTerminalHeight={availableTerminalHeight}
/>
</Box> </Box>
); );
}; };

View File

@ -29,7 +29,6 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
const borderColor = hasPending ? Colors.AccentYellow : Colors.SubtleComment; const borderColor = hasPending ? Colors.AccentYellow : Colors.SubtleComment;
const staticHeight = /* border */ 2 + /* marginBottom */ 1; const staticHeight = /* border */ 2 + /* marginBottom */ 1;
availableTerminalHeight -= staticHeight;
return ( return (
<Box <Box
@ -58,7 +57,7 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
resultDisplay={tool.resultDisplay} resultDisplay={tool.resultDisplay}
status={tool.status} status={tool.status}
confirmationDetails={tool.confirmationDetails} confirmationDetails={tool.confirmationDetails}
availableTerminalHeight={availableTerminalHeight} availableTerminalHeight={availableTerminalHeight - staticHeight}
/> />
{tool.status === ToolCallStatus.Confirming && {tool.status === ToolCallStatus.Confirming &&
tool.confirmationDetails && ( tool.confirmationDetails && (

View File

@ -26,7 +26,6 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
const statusIndicatorWidth = 3; const statusIndicatorWidth = 3;
const hasResult = resultDisplay && resultDisplay.toString().trim().length > 0; const hasResult = resultDisplay && resultDisplay.toString().trim().length > 0;
const staticHeight = /* Header */ 1; const staticHeight = /* Header */ 1;
availableTerminalHeight -= staticHeight;
let displayableResult = resultDisplay; let displayableResult = resultDisplay;
let hiddenLines = 0; let hiddenLines = 0;
@ -37,7 +36,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
const lines = resultDisplay.split('\n'); const lines = resultDisplay.split('\n');
// Estimate available height for this specific tool message content area // Estimate available height for this specific tool message content area
// This is a rough estimate; ideally, we'd have a more precise measurement. // This is a rough estimate; ideally, we'd have a more precise measurement.
const contentHeightEstimate = availableTerminalHeight - 5; // Subtracting lines for tool name, status, padding etc. const contentHeightEstimate = availableTerminalHeight - staticHeight - 5; // Subtracting lines for tool name, status, padding etc.
if (lines.length > contentHeightEstimate && contentHeightEstimate > 0) { if (lines.length > contentHeightEstimate && contentHeightEstimate > 0) {
displayableResult = lines.slice(0, contentHeightEstimate).join('\n'); displayableResult = lines.slice(0, contentHeightEstimate).join('\n');
hiddenLines = lines.length - contentHeightEstimate; hiddenLines = lines.length - contentHeightEstimate;
@ -83,7 +82,11 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
<Box flexDirection="column"> <Box flexDirection="column">
{typeof displayableResult === 'string' && ( {typeof displayableResult === 'string' && (
<Box flexDirection="column"> <Box flexDirection="column">
<MarkdownDisplay text={displayableResult} /> <MarkdownDisplay
text={displayableResult}
isPending={false}
availableTerminalHeight={availableTerminalHeight}
/>
</Box> </Box>
)} )}
{typeof displayableResult === 'object' && ( {typeof displayableResult === 'object' && (

View File

@ -11,6 +11,8 @@ import { colorizeCode } from './CodeColorizer.js';
interface MarkdownDisplayProps { interface MarkdownDisplayProps {
text: string; text: string;
isPending: boolean;
availableTerminalHeight: number;
} }
// Constants for Markdown parsing and rendering // Constants for Markdown parsing and rendering
@ -26,7 +28,11 @@ const CODE_BLOCK_PADDING = 1;
const LIST_ITEM_PREFIX_PADDING = 1; const LIST_ITEM_PREFIX_PADDING = 1;
const LIST_ITEM_TEXT_FLEX_GROW = 1; const LIST_ITEM_TEXT_FLEX_GROW = 1;
const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({ text }) => { const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
text,
isPending,
availableTerminalHeight,
}) => {
if (!text) return <></>; if (!text) return <></>;
const lines = text.split('\n'); const lines = text.split('\n');
@ -57,6 +63,8 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({ text }) => {
key={key} key={key}
content={codeBlockContent} content={codeBlockContent}
lang={codeBlockLang} lang={codeBlockLang}
isPending={isPending}
availableTerminalHeight={availableTerminalHeight}
/>, />,
); );
inCodeBlock = false; inCodeBlock = false;
@ -176,6 +184,8 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({ text }) => {
key="line-eof" key="line-eof"
content={codeBlockContent} content={codeBlockContent}
lang={codeBlockLang} lang={codeBlockLang}
isPending={isPending}
availableTerminalHeight={availableTerminalHeight}
/>, />,
); );
} }
@ -317,12 +327,49 @@ const RenderInline = React.memo(RenderInlineInternal);
interface RenderCodeBlockProps { interface RenderCodeBlockProps {
content: string[]; content: string[];
lang: string | null; lang: string | null;
isPending: boolean;
availableTerminalHeight: number;
} }
const RenderCodeBlockInternal: React.FC<RenderCodeBlockProps> = ({ const RenderCodeBlockInternal: React.FC<RenderCodeBlockProps> = ({
content, content,
lang, lang,
isPending,
availableTerminalHeight,
}) => { }) => {
const MIN_LINES_FOR_MESSAGE = 1; // Minimum lines to show before the "generating more" message
const RESERVED_LINES = 2; // Lines reserved for the message itself and potential padding
const MAX_CODE_LINES_WHEN_PENDING = Math.max(
0,
availableTerminalHeight - CODE_BLOCK_PADDING * 2 - RESERVED_LINES,
);
if (isPending) {
if (content.length > MAX_CODE_LINES_WHEN_PENDING) {
if (MAX_CODE_LINES_WHEN_PENDING < MIN_LINES_FOR_MESSAGE) {
// Not enough space to even show the message meaningfully
return (
<Box padding={CODE_BLOCK_PADDING}>
<Text color={Colors.SubtleComment}>
... code is being written ...
</Text>
</Box>
);
}
const truncatedContent = content.slice(0, MAX_CODE_LINES_WHEN_PENDING);
const colorizedTruncatedCode = colorizeCode(
truncatedContent.join('\n'),
lang,
);
return (
<Box flexDirection="column" padding={CODE_BLOCK_PADDING}>
{colorizedTruncatedCode}
<Text color={Colors.SubtleComment}>... generating more ...</Text>
</Box>
);
}
}
const fullContent = content.join('\n'); const fullContent = content.join('\n');
const colorizedCode = colorizeCode(fullContent, lang); const colorizedCode = colorizeCode(fullContent, lang);