From c7c82adf63d455c43b75cad6376cda62108bc771 Mon Sep 17 00:00:00 2001 From: Keith Guerin Date: Thu, 12 Mar 2026 20:17:16 -0700 Subject: [PATCH] fix(ui): polish loading indicator text and reorder witty phrases --- .../ui/components/LoadingIndicator.test.tsx | 69 +++++++++++++++---- .../src/ui/components/LoadingIndicator.tsx | 20 ++---- 2 files changed, 62 insertions(+), 27 deletions(-) diff --git a/packages/cli/src/ui/components/LoadingIndicator.test.tsx b/packages/cli/src/ui/components/LoadingIndicator.test.tsx index 8f6e8d2966..1aba99ebc1 100644 --- a/packages/cli/src/ui/components/LoadingIndicator.test.tsx +++ b/packages/cli/src/ui/components/LoadingIndicator.test.tsx @@ -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('', () => { const defaultProps = { - currentLoadingPhrase: 'Working...', + currentLoadingPhrase: 'Thinking...', elapsedTime: 5, }; @@ -71,7 +71,7 @@ describe('', () => { 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('', () => { 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('', () => { 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('', () => { 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('', () => { ); await waitUntilReady(); const output = lastFrame(); - expect(output).toContain('Working...'); + expect(output).toContain('Thinking...'); unmount(); }); @@ -266,7 +266,7 @@ describe('', () => { 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('', () => { ); 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('', () => { ); 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('', () => { 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('', () => { // 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('', () => { 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( + , + 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( + , + 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(); + }); }); }); diff --git a/packages/cli/src/ui/components/LoadingIndicator.tsx b/packages/cli/src/ui/components/LoadingIndicator.tsx index ef0bf3c7d0..40354843fb 100644 --- a/packages/cli/src/ui/components/LoadingIndicator.tsx +++ b/packages/cli/src/ui/components/LoadingIndicator.tsx @@ -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 = ({ ? (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 = ({ !forceRealStatusOnly && showWit && wittyPhrase && - primaryText === GENERIC_WORKING_LABEL ? ( + primaryText === 'Thinking...' ? ( {wittyPhrase} @@ -111,7 +102,6 @@ export const LoadingIndicator: React.FC = ({ {primaryText && ( - {thinkingIndicator} {primaryText} {primaryText === INTERACTIVE_SHELL_WAITING_PHRASE && ( @@ -122,13 +112,13 @@ export const LoadingIndicator: React.FC = ({ )} )} - {wittyPhraseNode} {cancelAndTimerContent && ( <> {cancelAndTimerContent} )} + {wittyPhraseNode} ); } @@ -154,7 +144,6 @@ export const LoadingIndicator: React.FC = ({ {primaryText && ( - {thinkingIndicator} {primaryText} {primaryText === INTERACTIVE_SHELL_WAITING_PHRASE && ( @@ -165,13 +154,13 @@ export const LoadingIndicator: React.FC = ({ )} )} - {wittyPhraseNode} {!isNarrow && cancelAndTimerContent && ( <> {cancelAndTimerContent} )} + {!isNarrow && wittyPhraseNode} {!isNarrow && {/* Spacer */}} {!isNarrow && rightContent && {rightContent}} @@ -181,6 +170,7 @@ export const LoadingIndicator: React.FC = ({ {cancelAndTimerContent} )} + {isNarrow && wittyPhraseNode} {isNarrow && rightContent && {rightContent}} );