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
This commit is contained in:
Keith Guerin
2026-02-28 23:37:40 -08:00
parent 3bd36ce4f0
commit e1e863dba2
12 changed files with 126 additions and 98 deletions

View File

@@ -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<string, unknown>;
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<string, unknown>;
if (uiValue['loadingPhrases'] !== undefined) {
expect(uiValue['loadingPhrases']).toBe('witty');
if (uiValue['loadingPhraseLayout'] !== undefined) {
expect(uiValue['loadingPhraseLayout']).toBe('wit_inline');
}
}
});

View File

@@ -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;

View File

@@ -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',
]);
});

View File

@@ -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',

View File

@@ -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,

View File

@@ -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);

View File

@@ -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
}

View File

@@ -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<typeof useLoadingIndicator>;
@@ -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(
<TestComponent
mode={loadingPhrasesMode}
mode={loadingPhraseLayout}
errorVerbosity={initialErrorVerbosity}
{...newProps}
/>,
@@ -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();

View File

@@ -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,
);

View File

@@ -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 <Text>{currentTip || currentWittyPhrase}</Text>;
@@ -293,7 +293,7 @@ describe('usePhraseCycler', () => {
<TestComponent
isActive={config.isActive}
isWaiting={false}
loadingPhrasesMode="witty"
loadingPhraseLayout="wit_inline"
customPhrases={config.customPhrases}
/>
);

View File

@@ -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,
]);