fix(ui): polish loading indicator text and reorder witty phrases

This commit is contained in:
Keith Guerin
2026-03-12 20:17:16 -07:00
parent 663005a259
commit c7c82adf63
2 changed files with 62 additions and 27 deletions

View File

@@ -10,7 +10,7 @@ import { Text } from 'ink';
import { LoadingIndicator } from './LoadingIndicator.js';
import { StreamingContext } from '../contexts/StreamingContext.js';
import { StreamingState } from '../types.js';
import { vi } from 'vitest';
import { describe, it, expect, vi } from 'vitest';
import * as useTerminalSize from '../hooks/useTerminalSize.js';
// Mock GeminiRespondingSpinner
@@ -50,7 +50,7 @@ const renderWithContext = (
describe('<LoadingIndicator />', () => {
const defaultProps = {
currentLoadingPhrase: 'Working...',
currentLoadingPhrase: 'Thinking...',
elapsedTime: 5,
};
@@ -71,7 +71,7 @@ describe('<LoadingIndicator />', () => {
await waitUntilReady();
const output = lastFrame();
expect(output).toContain('MockRespondingSpinner');
expect(output).toContain('Working...');
expect(output).toContain('Thinking...');
expect(output).toContain('esc to cancel, 5s');
});
@@ -108,7 +108,7 @@ describe('<LoadingIndicator />', () => {
it('should display the elapsedTime correctly when Responding', async () => {
const props = {
currentLoadingPhrase: 'Working...',
currentLoadingPhrase: 'Thinking...',
elapsedTime: 60,
};
const { lastFrame, unmount, waitUntilReady } = renderWithContext(
@@ -122,7 +122,7 @@ describe('<LoadingIndicator />', () => {
it('should display the elapsedTime correctly in human-readable format', async () => {
const props = {
currentLoadingPhrase: 'Working...',
currentLoadingPhrase: 'Thinking...',
elapsedTime: 125,
};
const { lastFrame, unmount, waitUntilReady } = renderWithContext(
@@ -229,7 +229,7 @@ describe('<LoadingIndicator />', () => {
it('should display fallback phrase if thought is empty', async () => {
const props = {
thought: null,
currentLoadingPhrase: 'Working...',
currentLoadingPhrase: 'Thinking...',
elapsedTime: 5,
};
const { lastFrame, unmount, waitUntilReady } = renderWithContext(
@@ -238,7 +238,7 @@ describe('<LoadingIndicator />', () => {
);
await waitUntilReady();
const output = lastFrame();
expect(output).toContain('Working...');
expect(output).toContain('Thinking...');
unmount();
});
@@ -266,7 +266,7 @@ describe('<LoadingIndicator />', () => {
unmount();
});
it('should prepend "Thinking... " if the subject does not start with "Thinking"', async () => {
it('should NOT prepend "Thinking... " even if the subject does not start with "Thinking"', async () => {
const props = {
thought: {
subject: 'Planning the response...',
@@ -280,7 +280,8 @@ describe('<LoadingIndicator />', () => {
);
await waitUntilReady();
const output = lastFrame();
expect(output).toContain('Thinking... Planning the response...');
expect(output).toContain('Planning the response...');
expect(output).not.toContain('Thinking... ');
unmount();
});
@@ -299,7 +300,6 @@ describe('<LoadingIndicator />', () => {
);
await waitUntilReady();
const output = lastFrame();
expect(output).toContain('Thinking... ');
expect(output).toContain('This should be displayed');
expect(output).not.toContain('This should not be displayed');
unmount();
@@ -349,7 +349,7 @@ describe('<LoadingIndicator />', () => {
const output = lastFrame();
// Check for single line output
expect(output?.trim().includes('\n')).toBe(false);
expect(output).toContain('Working...');
expect(output).toContain('Thinking...');
expect(output).toContain('esc to cancel, 5s');
expect(output).toContain('Right');
unmount();
@@ -373,7 +373,7 @@ describe('<LoadingIndicator />', () => {
// 3. Right Content
expect(lines).toHaveLength(3);
if (lines) {
expect(lines[0]).toContain('Working...');
expect(lines[0]).toContain('Thinking...');
expect(lines[0]).not.toContain('esc to cancel, 5s');
expect(lines[1]).toContain('esc to cancel, 5s');
expect(lines[2]).toContain('Right');
@@ -402,5 +402,50 @@ describe('<LoadingIndicator />', () => {
expect(lastFrame()?.includes('\n')).toBe(true);
unmount();
});
it('should render witty phrase after cancel and timer hint in wide layout', async () => {
const { lastFrame, unmount, waitUntilReady } = renderWithContext(
<LoadingIndicator
elapsedTime={5}
wittyPhrase="I am witty"
showWit={true}
currentLoadingPhrase="Thinking..."
/>,
StreamingState.Responding,
120,
);
await waitUntilReady();
const output = lastFrame();
// Sequence should be: Primary Text -> Cancel/Timer -> Witty Phrase
expect(output).toContain('Thinking... (esc to cancel, 5s) I am witty');
unmount();
});
it('should render witty phrase after cancel and timer hint in narrow layout', async () => {
const { lastFrame, unmount, waitUntilReady } = renderWithContext(
<LoadingIndicator
elapsedTime={5}
wittyPhrase="I am witty"
showWit={true}
currentLoadingPhrase="Thinking..."
/>,
StreamingState.Responding,
79,
);
await waitUntilReady();
const output = lastFrame();
const lines = output?.trim().split('\n');
// Expecting 3 lines:
// 1. Spinner + Primary Text
// 2. Cancel + Timer
// 3. Witty Phrase
expect(lines).toHaveLength(3);
if (lines) {
expect(lines[0]).toContain('Thinking...');
expect(lines[1]).toContain('esc to cancel, 5s');
expect(lines[2]).toContain('I am witty');
}
unmount();
});
});
});

View File

@@ -15,7 +15,6 @@ import { formatDuration } from '../utils/formatters.js';
import { useTerminalSize } from '../hooks/useTerminalSize.js';
import { isNarrowWidth } from '../utils/isNarrowWidth.js';
import { INTERACTIVE_SHELL_WAITING_PHRASE } from '../hooks/usePhraseCycler.js';
import { GENERIC_WORKING_LABEL } from '../textConstants.js';
interface LoadingIndicatorProps {
currentLoadingPhrase?: string;
@@ -67,16 +66,8 @@ export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
? (thoughtLabel ?? thought.subject)
: currentLoadingPhrase ||
(streamingState === StreamingState.Responding
? GENERIC_WORKING_LABEL
? 'Thinking...'
: undefined);
const hasThoughtIndicator =
currentLoadingPhrase !== INTERACTIVE_SHELL_WAITING_PHRASE &&
Boolean(thought?.subject?.trim());
// Avoid "Thinking... Thinking..." duplication if primaryText already starts with "Thinking"
const thinkingIndicator =
hasThoughtIndicator && !primaryText?.startsWith('Thinking')
? 'Thinking... '
: '';
const cancelAndTimerContent =
showCancelAndTimer &&
@@ -88,7 +79,7 @@ export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
!forceRealStatusOnly &&
showWit &&
wittyPhrase &&
primaryText === GENERIC_WORKING_LABEL ? (
primaryText === 'Thinking...' ? (
<Box marginLeft={1}>
<Text color={theme.text.secondary} dimColor italic>
{wittyPhrase}
@@ -111,7 +102,6 @@ export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
{primaryText && (
<Box flexShrink={1}>
<Text color={theme.text.primary} italic wrap="truncate-end">
{thinkingIndicator}
{primaryText}
</Text>
{primaryText === INTERACTIVE_SHELL_WAITING_PHRASE && (
@@ -122,13 +112,13 @@ export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
)}
</Box>
)}
{wittyPhraseNode}
{cancelAndTimerContent && (
<>
<Box flexShrink={0} width={1} />
<Text color={theme.text.secondary}>{cancelAndTimerContent}</Text>
</>
)}
{wittyPhraseNode}
</Box>
);
}
@@ -154,7 +144,6 @@ export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
{primaryText && (
<Box flexShrink={1}>
<Text color={theme.text.primary} italic wrap="truncate-end">
{thinkingIndicator}
{primaryText}
</Text>
{primaryText === INTERACTIVE_SHELL_WAITING_PHRASE && (
@@ -165,13 +154,13 @@ export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
)}
</Box>
)}
{wittyPhraseNode}
{!isNarrow && cancelAndTimerContent && (
<>
<Box flexShrink={0} width={1} />
<Text color={theme.text.secondary}>{cancelAndTimerContent}</Text>
</>
)}
{!isNarrow && wittyPhraseNode}
</Box>
{!isNarrow && <Box flexGrow={1}>{/* Spacer */}</Box>}
{!isNarrow && rightContent && <Box>{rightContent}</Box>}
@@ -181,6 +170,7 @@ export const LoadingIndicator: React.FC<LoadingIndicatorProps> = ({
<Text color={theme.text.secondary}>{cancelAndTimerContent}</Text>
</Box>
)}
{isNarrow && wittyPhraseNode}
{isNarrow && rightContent && <Box>{rightContent}</Box>}
</Box>
);