From e1e863dba2b1677727d3e22880e29120b268b577 Mon Sep 17 00:00:00 2001 From: Keith Guerin Date: Sat, 28 Feb 2026 23:37:40 -0800 Subject: [PATCH] feat(cli): unify loading phrase settings into single layout control - Replace ui.loadingPhrases and ui.wittyPhrasePosition with ui.loadingPhraseLayout - Supported layouts: none, tips, wit_status, wit_inline, wit_ambient, all_inline, all_ambient - Set 'all_inline' as the new default (tips on right, wit inline) - Update migration logic to map deprecated enableLoadingPhrases to 'none' - Synchronize all unit tests and documentation with the new configuration model --- _footer-ui.md | 24 +++++++----- packages/cli/src/config/settings.test.ts | 24 ++++++------ packages/cli/src/config/settings.ts | 17 +++++--- .../cli/src/config/settingsSchema.test.ts | 19 +++++---- packages/cli/src/config/settingsSchema.ts | 35 ++++++----------- packages/cli/src/ui/AppContainer.tsx | 2 +- .../cli/src/ui/components/Composer.test.tsx | 4 +- packages/cli/src/ui/components/Composer.tsx | 30 ++++++++++---- .../src/ui/hooks/useLoadingIndicator.test.tsx | 16 ++++---- .../cli/src/ui/hooks/useLoadingIndicator.ts | 6 +-- .../cli/src/ui/hooks/usePhraseCycler.test.tsx | 8 ++-- packages/cli/src/ui/hooks/usePhraseCycler.ts | 39 ++++++++++++------- 12 files changed, 126 insertions(+), 98 deletions(-) diff --git a/_footer-ui.md b/_footer-ui.md index 40d685adf6..b19644f72a 100644 --- a/_footer-ui.md +++ b/_footer-ui.md @@ -730,17 +730,20 @@ better discoverability of features: wit based on the available terminal width, ensuring that only phrases that fit without colliding with the system status are selected. -### 4. Witty Phrase Positioning +### 4. Loading Phrase Layout -A new setting `ui.wittyPhrasePosition` allows controlling where entertainment -phrases are displayed: +A single setting `ui.loadingPhraseLayout` now controls both the content and the +position of loading phrases: -- **`status`**: Replaces the status text when the model is thinking but hasn't - emitted a specific thought yet. -- **`inline` (Default)**: Appends the witty phrase in gray immediately following - the real system status (e.g., `⠏ Searching... Loading wit.exe`). -- **`ambient`**: Displays witty phrases on the far right, interspersed with - tips. +- **`none`**: Pure status only (no tips or witty phrases). +- **`tips`**: Informative tips only (displayed on the far right). +- **`wit_status`**: Witty phrases only, replacing the status text (Legacy + behavior). +- **`wit_inline`**: Witty phrases only, following the status in gray. +- **`wit_ambient`**: Witty phrases only, displayed on the far right. +- **`all_inline` (Default)**: Informative tips on the right, witty phrases + inline (after status). +- **`all_ambient`**: Both tips and witty phrases cycle on the far right. --- @@ -755,12 +758,13 @@ review against the updated specification. - **Divider Options:** - `New Layout`: Divider above everything. - `New Layout (Divider Down)`: Divider between status and indicators. +- **Loading Phrases:** Unified via `ui.loadingPhraseLayout` (Defaults to + `all_inline`). - **Input State:** Drafted text remains visible during tool approval; the input box is greyed out and focus is removed. - **Toasts:** Claims 100% width, left-aligned, prominent warning color. Overrides ambient tips. - **Hooks:** Uses `↪` (Before) / `↩` (After) icons. Text is white and italic. -- **Witty Phrases:** Default to `inline` position (gray text after status). - **Responsive:** - Tips/Wit disappear on narrow windows or if they collide with long statuses. - Status text wraps onto multiple lines only when the window is narrow. diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 8fd0bd81b0..292692d0b0 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -2082,17 +2082,17 @@ describe('Settings Loading and Merging', () => { }), ); - // Check that enableLoadingPhrases: false was further migrated to loadingPhrases: 'off' + // Check that enableLoadingPhrases: false was further migrated to loadingPhraseLayout: 'none' expect(setValueSpy).toHaveBeenCalledWith( SettingScope.User, 'ui', expect.objectContaining({ - loadingPhrases: 'off', + loadingPhraseLayout: 'none', }), ); }); - it('should migrate enableLoadingPhrases: false to loadingPhrases: off', () => { + it('should migrate enableLoadingPhrases: false to loadingPhraseLayout: none', () => { const userSettingsContent = { ui: { accessibility: { @@ -2110,12 +2110,12 @@ describe('Settings Loading and Merging', () => { SettingScope.User, 'ui', expect.objectContaining({ - loadingPhrases: 'off', + loadingPhraseLayout: 'none', }), ); }); - it('should not migrate enableLoadingPhrases: true to loadingPhrases', () => { + it('should not migrate enableLoadingPhrases: true to loadingPhraseLayout', () => { const userSettingsContent = { ui: { accessibility: { @@ -2129,18 +2129,18 @@ describe('Settings Loading and Merging', () => { migrateDeprecatedSettings(loadedSettings); - // Should not set loadingPhrases when enableLoadingPhrases is true + // Should not set loadingPhraseLayout when enableLoadingPhrases is true const uiCalls = setValueSpy.mock.calls.filter((call) => call[1] === 'ui'); for (const call of uiCalls) { const uiValue = call[2] as Record; - expect(uiValue).not.toHaveProperty('loadingPhrases'); + expect(uiValue).not.toHaveProperty('loadingPhraseLayout'); } }); - it('should not overwrite existing loadingPhrases during migration', () => { + it('should not overwrite existing loadingPhraseLayout during migration', () => { const userSettingsContent = { ui: { - loadingPhrases: 'witty', + loadingPhraseLayout: 'wit_inline', accessibility: { enableLoadingPhrases: false, }, @@ -2152,12 +2152,12 @@ describe('Settings Loading and Merging', () => { migrateDeprecatedSettings(loadedSettings); - // Should not overwrite existing loadingPhrases + // Should not overwrite existing loadingPhraseLayout const uiCalls = setValueSpy.mock.calls.filter((call) => call[1] === 'ui'); for (const call of uiCalls) { const uiValue = call[2] as Record; - if (uiValue['loadingPhrases'] !== undefined) { - expect(uiValue['loadingPhrases']).toBe('witty'); + if (uiValue['loadingPhraseLayout'] !== undefined) { + expect(uiValue['loadingPhraseLayout']).toBe('wit_inline'); } } }); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 657968a3b6..7533996d70 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -165,10 +165,17 @@ export interface SummarizeToolOutputSettings { tokenBudget?: number; } -export type LoadingPhrasesMode = 'tips' | 'witty' | 'all' | 'off'; +export type LoadingPhrasesMode = + | 'none' + | 'tips' + | 'wit_status' + | 'wit_inline' + | 'wit_ambient' + | 'all_inline' + | 'all_ambient'; export interface AccessibilitySettings { - /** @deprecated Use ui.loadingPhrases instead. */ + /** @deprecated Use ui.loadingPhraseLayout instead. */ enableLoadingPhrases?: boolean; screenReader?: boolean; } @@ -912,14 +919,14 @@ export function migrateDeprecatedSettings( } } - // Migrate enableLoadingPhrases: false → loadingPhrases: 'off' + // Migrate enableLoadingPhrases: false → loadingPhraseLayout: 'none' const enableLP = newAccessibility['enableLoadingPhrases']; if ( typeof enableLP === 'boolean' && - newUi['loadingPhrases'] === undefined + newUi['loadingPhraseLayout'] === undefined ) { if (!enableLP) { - newUi['loadingPhrases'] = 'off'; + newUi['loadingPhraseLayout'] = 'none'; loadedSettings.setValue(scope, 'ui', newUi); if (!settingsFile.readOnly) { anyModified = true; diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 17a916213f..5bdd175a2f 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -83,16 +83,21 @@ describe('SettingsSchema', () => { ).toBe('boolean'); }); - it('should have loadingPhrases enum property', () => { - const definition = getSettingsSchema().ui?.properties?.loadingPhrases; + it('should have loadingPhraseLayout enum property', () => { + const definition = + getSettingsSchema().ui?.properties?.loadingPhraseLayout; expect(definition).toBeDefined(); expect(definition?.type).toBe('enum'); - expect(definition?.default).toBe('tips'); - expect(definition?.options?.map((o) => o.value)).toEqual([ + expect(definition?.default).toBe('all_inline'); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect(definition?.options?.map((o: any) => o.value)).toEqual([ + 'none', 'tips', - 'witty', - 'all', - 'off', + 'wit_status', + 'wit_inline', + 'wit_ambient', + 'all_inline', + 'all_ambient', ]); }); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 574017c217..332fb827bf 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -633,18 +633,23 @@ const SETTINGS_SCHEMA = { { value: 'new_divider_down', label: 'New Layout (Divider Down)' }, ], }, - wittyPhrasePosition: { + loadingPhraseLayout: { type: 'enum', - label: 'Witty Phrase Position', + label: 'Loading Phrase Layout', category: 'UI', requiresRestart: false, - default: 'inline', - description: 'Where to show witty phrases while waiting.', + default: 'all_inline', + description: + 'Control which loading phrases are shown and where they appear.', showInDialog: true, options: [ - { value: 'status', label: 'Status' }, - { value: 'inline', label: 'Inline (after status)' }, - { value: 'ambient', label: 'Ambient (at right)' }, + { value: 'none', label: 'None' }, + { value: 'tips', label: 'Tips Only (at right)' }, + { value: 'wit_status', label: 'Wit Only (in status slot)' }, + { value: 'wit_inline', label: 'Wit Only (after status)' }, + { value: 'wit_ambient', label: 'Wit Only (at right)' }, + { value: 'all_inline', label: 'Tips at right, Wit inline' }, + { value: 'all_ambient', label: 'Tips and Wit at right' }, ], }, showMemoryUsage: { @@ -731,22 +736,6 @@ const SETTINGS_SCHEMA = { description: 'Show the spinner during operations.', showInDialog: true, }, - loadingPhrases: { - type: 'enum', - label: 'Loading Phrases', - category: 'UI', - requiresRestart: false, - default: 'all', - description: - 'What to show while the model is working: tips, witty comments, both, or nothing.', - showInDialog: true, - options: [ - { value: 'tips', label: 'Tips' }, - { value: 'witty', label: 'Witty' }, - { value: 'all', label: 'All' }, - { value: 'off', label: 'Off' }, - ], - }, errorVerbosity: { type: 'enum', label: 'Error Verbosity', diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 637a7a8309..c2cfb669c7 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -2102,7 +2102,7 @@ Logging in with Google... Restarting Gemini CLI to continue. streamingState, shouldShowFocusHint, retryStatus, - loadingPhrasesMode: settings.merged.ui.loadingPhrases, + loadingPhraseLayout: settings.merged.ui.loadingPhraseLayout, customWittyPhrases: settings.merged.ui.customWittyPhrases, errorVerbosity: settings.merged.ui.errorVerbosity, maxLength, diff --git a/packages/cli/src/ui/components/Composer.test.tsx b/packages/cli/src/ui/components/Composer.test.tsx index 999b1531f9..06429e1b85 100644 --- a/packages/cli/src/ui/components/Composer.test.tsx +++ b/packages/cli/src/ui/components/Composer.test.tsx @@ -402,13 +402,13 @@ describe('Composer', () => { expect(output).not.toContain('ShortcutsHint'); }); - it('renders LoadingIndicator with thought when loadingPhrases is off', async () => { + it('renders LoadingIndicator with thought when loadingPhraseLayout is none', async () => { const uiState = createMockUIState({ streamingState: StreamingState.Responding, thought: { subject: 'Hidden', description: 'Should not show' }, }); const settings = createMockSettings({ - merged: { ui: { loadingPhrases: 'off' } }, + merged: { ui: { loadingPhraseLayout: 'none' } }, }); const { lastFrame } = await renderComposer(uiState, settings); diff --git a/packages/cli/src/ui/components/Composer.tsx b/packages/cli/src/ui/components/Composer.tsx index 6ebca992d9..02b3a13b8a 100644 --- a/packages/cli/src/ui/components/Composer.tsx +++ b/packages/cli/src/ui/components/Composer.tsx @@ -59,7 +59,14 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { const isAlternateBuffer = useAlternateBuffer(); const { showApprovalModeIndicator } = uiState; const newLayoutSetting = settings.merged.ui.newFooterLayout; - const wittyPosition = settings.merged.ui.wittyPhrasePosition; + const { loadingPhraseLayout } = settings.merged.ui; + const wittyPosition: 'status' | 'inline' | 'ambient' = + loadingPhraseLayout === 'wit_status' + ? 'status' + : loadingPhraseLayout === 'wit_inline' || + loadingPhraseLayout === 'all_inline' + ? 'inline' + : 'ambient'; const isExperimentalLayout = newLayoutSetting !== 'legacy'; const showUiDetails = uiState.cleanUiDetailsVisible; const suggestionsPosition = isAlternateBuffer ? 'above' : 'below'; @@ -198,8 +205,15 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { const ambientText = isInteractiveShellWaiting ? undefined - : uiState.currentTip || - (wittyPosition === 'ambient' ? uiState.currentWittyPhrase : undefined); + : (loadingPhraseLayout === 'tips' || + loadingPhraseLayout === 'all_inline' || + loadingPhraseLayout === 'all_ambient' + ? uiState.currentTip + : undefined) || + (loadingPhraseLayout === 'wit_ambient' || + loadingPhraseLayout === 'all_ambient' + ? uiState.currentWittyPhrase + : undefined); let estimatedStatusLength = 0; if ( @@ -239,6 +253,7 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { uiState.streamingState !== StreamingState.Idle && !hasPendingActionRequired && ambientText && + loadingPhraseLayout !== 'none' && !willCollideAmbient && !isNarrow; @@ -299,10 +314,9 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { : uiState.thought } currentLoadingPhrase={ - uiState.currentLoadingPhrase?.includes('press tab to focus shell') + uiState.currentLoadingPhrase?.includes('Tab to focus') ? uiState.currentLoadingPhrase - : !isExperimentalLayout && - settings.merged.ui.loadingPhrases !== 'off' + : !isExperimentalLayout && loadingPhraseLayout !== 'none' ? uiState.currentLoadingPhrase : isExperimentalLayout && uiState.streamingState === StreamingState.Responding && @@ -376,7 +390,7 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { : uiState.thought } currentLoadingPhrase={ - settings.merged.ui.loadingPhrases === 'off' + loadingPhraseLayout === 'none' ? undefined : uiState.currentLoadingPhrase } @@ -419,7 +433,7 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { : uiState.thought } currentLoadingPhrase={ - settings.merged.ui.loadingPhrases === 'off' + loadingPhraseLayout === 'none' ? undefined : uiState.currentLoadingPhrase } diff --git a/packages/cli/src/ui/hooks/useLoadingIndicator.test.tsx b/packages/cli/src/ui/hooks/useLoadingIndicator.test.tsx index e0ae9b5f20..63bdf4fa36 100644 --- a/packages/cli/src/ui/hooks/useLoadingIndicator.test.tsx +++ b/packages/cli/src/ui/hooks/useLoadingIndicator.test.tsx @@ -34,7 +34,7 @@ describe('useLoadingIndicator', () => { initialStreamingState: StreamingState, initialShouldShowFocusHint: boolean = false, initialRetryStatus: RetryAttemptPayload | null = null, - loadingPhrasesMode: LoadingPhrasesMode = 'all', + loadingPhraseLayout: LoadingPhrasesMode = 'all_inline', initialErrorVerbosity: 'low' | 'full' = 'full', ) => { let hookResult: ReturnType; @@ -55,7 +55,7 @@ describe('useLoadingIndicator', () => { streamingState, shouldShowFocusHint: !!shouldShowFocusHint, retryStatus: retryStatus || null, - loadingPhrasesMode: mode, + loadingPhraseLayout: mode, errorVerbosity, }); return null; @@ -65,7 +65,7 @@ describe('useLoadingIndicator', () => { streamingState={initialStreamingState} shouldShowFocusHint={initialShouldShowFocusHint} retryStatus={initialRetryStatus} - mode={loadingPhrasesMode} + mode={loadingPhraseLayout} errorVerbosity={initialErrorVerbosity} />, ); @@ -84,7 +84,7 @@ describe('useLoadingIndicator', () => { }) => rerender( , @@ -253,7 +253,7 @@ describe('useLoadingIndicator', () => { StreamingState.Responding, false, retryStatus, - 'all', + 'all_inline', 'low', ); @@ -273,7 +273,7 @@ describe('useLoadingIndicator', () => { StreamingState.Responding, false, retryStatus, - 'all', + 'all_inline', 'low', ); @@ -282,12 +282,12 @@ describe('useLoadingIndicator', () => { ); }); - it('should show no phrases when loadingPhrasesMode is "off"', () => { + it('should show no phrases when loadingPhraseLayout is "none"', () => { const { result } = renderLoadingIndicatorHook( StreamingState.Responding, false, null, - 'off', + 'none', ); expect(result.current.currentLoadingPhrase).toBeUndefined(); diff --git a/packages/cli/src/ui/hooks/useLoadingIndicator.ts b/packages/cli/src/ui/hooks/useLoadingIndicator.ts index 0d9240738f..1ecdf9e71f 100644 --- a/packages/cli/src/ui/hooks/useLoadingIndicator.ts +++ b/packages/cli/src/ui/hooks/useLoadingIndicator.ts @@ -20,7 +20,7 @@ export interface UseLoadingIndicatorProps { streamingState: StreamingState; shouldShowFocusHint: boolean; retryStatus: RetryAttemptPayload | null; - loadingPhrasesMode?: LoadingPhrasesMode; + loadingPhraseLayout?: LoadingPhrasesMode; customWittyPhrases?: string[]; errorVerbosity?: 'low' | 'full'; maxLength?: number; @@ -30,7 +30,7 @@ export const useLoadingIndicator = ({ streamingState, shouldShowFocusHint, retryStatus, - loadingPhrasesMode, + loadingPhraseLayout, customWittyPhrases, errorVerbosity = 'full', maxLength, @@ -46,7 +46,7 @@ export const useLoadingIndicator = ({ isPhraseCyclingActive, isWaiting, shouldShowFocusHint, - loadingPhrasesMode, + loadingPhraseLayout, customWittyPhrases, maxLength, ); diff --git a/packages/cli/src/ui/hooks/usePhraseCycler.test.tsx b/packages/cli/src/ui/hooks/usePhraseCycler.test.tsx index 02517f80ec..16334d1ccc 100644 --- a/packages/cli/src/ui/hooks/usePhraseCycler.test.tsx +++ b/packages/cli/src/ui/hooks/usePhraseCycler.test.tsx @@ -21,20 +21,20 @@ const TestComponent = ({ isActive, isWaiting, isInteractiveShellWaiting = false, - loadingPhrasesMode = 'all', + loadingPhraseLayout = 'all_inline', customPhrases, }: { isActive: boolean; isWaiting: boolean; isInteractiveShellWaiting?: boolean; - loadingPhrasesMode?: LoadingPhrasesMode; + loadingPhraseLayout?: LoadingPhrasesMode; customPhrases?: string[]; }) => { const { currentTip, currentWittyPhrase } = usePhraseCycler( isActive, isWaiting, isInteractiveShellWaiting, - loadingPhrasesMode, + loadingPhraseLayout, customPhrases, ); return {currentTip || currentWittyPhrase}; @@ -293,7 +293,7 @@ describe('usePhraseCycler', () => { ); diff --git a/packages/cli/src/ui/hooks/usePhraseCycler.ts b/packages/cli/src/ui/hooks/usePhraseCycler.ts index 231b2048e2..7c936994fb 100644 --- a/packages/cli/src/ui/hooks/usePhraseCycler.ts +++ b/packages/cli/src/ui/hooks/usePhraseCycler.ts @@ -18,16 +18,16 @@ export const INTERACTIVE_SHELL_WAITING_PHRASE = * @param isActive Whether the phrase cycling should be active. * @param isWaiting Whether to show a specific waiting phrase. * @param shouldShowFocusHint Whether to show the shell focus hint. - * @param loadingPhrasesMode Which phrases to show: tips, witty, all, or off. + * @param loadingPhraseLayout Which phrases to show and where. * @param customPhrases Optional list of custom phrases to use instead of built-in witty phrases. * @param maxLength Optional maximum length for the selected phrase. - * @returns The current loading phrase. + * @returns The current tip and witty phrase. */ export const usePhraseCycler = ( isActive: boolean, isWaiting: boolean, shouldShowFocusHint: boolean, - loadingPhrasesMode: LoadingPhrasesMode = 'tips', + loadingPhraseLayout: LoadingPhrasesMode = 'all_inline', customPhrases?: string[], maxLength?: number, ) => { @@ -58,7 +58,7 @@ export const usePhraseCycler = ( return; } - if (!isActive || loadingPhrasesMode === 'off') { + if (!isActive || loadingPhraseLayout === 'none') { setCurrentTip(undefined); setCurrentWittyPhrase(undefined); return; @@ -70,10 +70,23 @@ export const usePhraseCycler = ( : WITTY_LOADING_PHRASES; const setRandomPhrase = () => { - let currentMode = loadingPhrasesMode; + let currentMode: 'tips' | 'witty' | 'all' = 'all'; - // In 'all' mode, we decide once per phrase cycle what to show - if (loadingPhrasesMode === 'all') { + if (loadingPhraseLayout === 'tips') { + currentMode = 'tips'; + } else if ( + loadingPhraseLayout === 'wit_status' || + loadingPhraseLayout === 'wit_inline' || + loadingPhraseLayout === 'wit_ambient' + ) { + currentMode = 'witty'; + } + + // In 'all' modes, we decide once per phrase cycle what to show + if ( + loadingPhraseLayout === 'all_inline' || + loadingPhraseLayout === 'all_ambient' + ) { if (!hasShownFirstRequestTipRef.current) { currentMode = 'tips'; hasShownFirstRequestTipRef.current = true; @@ -85,13 +98,9 @@ export const usePhraseCycler = ( const phraseList = currentMode === 'witty' ? wittyPhrases : INFORMATIVE_TIPS; - // If we have a maxLength, we need to account for potential prefixes. - // Tips are prefixed with "Tip: " in the Composer UI. - const adjustedMaxLength = maxLength; - const filteredList = - adjustedMaxLength !== undefined - ? phraseList.filter((p) => p.length <= adjustedMaxLength) + maxLength !== undefined + ? phraseList.filter((p) => p.length <= maxLength) : phraseList; if (filteredList.length > 0) { @@ -105,7 +114,7 @@ export const usePhraseCycler = ( setCurrentWittyPhrase(undefined); } } else { - // If no phrases fit, try to fallback to a very short list or nothing + // If no phrases fit, try to fallback setCurrentTip(undefined); setCurrentWittyPhrase(undefined); } @@ -129,7 +138,7 @@ export const usePhraseCycler = ( isActive, isWaiting, shouldShowFocusHint, - loadingPhrasesMode, + loadingPhraseLayout, customPhrases, maxLength, ]);