From da6bc36deb844bda15d49f9875d467858f71aea8 Mon Sep 17 00:00:00 2001 From: Keith Guerin Date: Fri, 27 Mar 2026 13:57:21 -0700 Subject: [PATCH] feat(ui): implement circular 8-dot braille spinner --- .prettierignore | 1 + packages/cli/src/test-utils/settings.ts | 4 + .../ui/components/CircularSpinner.test.tsx | 122 +++++++++++++++ .../cli/src/ui/components/CircularSpinner.tsx | 141 ++++++++++++++++++ .../cli/src/ui/components/CliSpinner.test.tsx | 17 ++- packages/cli/src/ui/components/CliSpinner.tsx | 19 ++- .../ui/components/GeminiRespondingSpinner.tsx | 11 +- .../src/ui/components/GeminiSpinner.test.tsx | 41 +++++ .../cli/src/ui/components/GeminiSpinner.tsx | 23 ++- .../ui/components/LoadingIndicator.test.tsx | 6 +- .../src/ui/components/LoadingIndicator.tsx | 4 +- .../src/ui/components/MainContent.test.tsx | 4 + .../cli/src/ui/components/spinner-spec.md | 29 ++++ 13 files changed, 398 insertions(+), 24 deletions(-) create mode 100644 packages/cli/src/ui/components/CircularSpinner.test.tsx create mode 100644 packages/cli/src/ui/components/CircularSpinner.tsx create mode 100644 packages/cli/src/ui/components/GeminiSpinner.test.tsx create mode 100644 packages/cli/src/ui/components/spinner-spec.md diff --git a/.prettierignore b/.prettierignore index 9009498d8d..26ce627dd6 100644 --- a/.prettierignore +++ b/.prettierignore @@ -22,3 +22,4 @@ Thumbs.db .pytest_cache **/SKILL.md packages/sdk/test-data/*.json +**/*.snap diff --git a/packages/cli/src/test-utils/settings.ts b/packages/cli/src/test-utils/settings.ts index 20d0613f83..f5f4f16a06 100644 --- a/packages/cli/src/test-utils/settings.ts +++ b/packages/cli/src/test-utils/settings.ts @@ -46,6 +46,7 @@ export const createMockSettings = ( workspace, isTrusted, errors, + merged: mergedOverride, ...settingsOverrides } = overrides; @@ -60,6 +61,7 @@ export const createMockSettings = ( settings: settingsOverrides, originalSettings: settingsOverrides, }, + (workspace as any) || { path: '', settings: {}, originalSettings: {} }, isTrusted ?? true, errors || [], @@ -68,6 +70,8 @@ export const createMockSettings = ( if (mergedOverride) { // @ts-expect-error - overriding private field for testing loaded._merged = createTestMergedSettings(mergedOverride); + // @ts-expect-error - re-calculating snapshot after merged override + loaded._snapshot = loaded.computeSnapshot(); } // Assign any function overrides (e.g., vi.fn() for methods) diff --git a/packages/cli/src/ui/components/CircularSpinner.test.tsx b/packages/cli/src/ui/components/CircularSpinner.test.tsx new file mode 100644 index 0000000000..6f823ff81c --- /dev/null +++ b/packages/cli/src/ui/components/CircularSpinner.test.tsx @@ -0,0 +1,122 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { act } from 'react'; +import { renderWithProviders } from '../../test-utils/render.js'; +import { createMockSettings } from '../../test-utils/settings.js'; +import { CircularSpinner } from './CircularSpinner.js'; +import { describe, it, expect, vi, afterEach } from 'vitest'; + +describe('', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should render the static frame correctly', async () => { + const { lastFrame, waitUntilReady } = await renderWithProviders( + , + ); + await waitUntilReady(); + expect(lastFrame()?.trim()).toBe('⢎⡱'); + }); + + it('should render Small variant frames (tail length 2)', async () => { + const { + lastFrame: lastFrame0, + waitUntilReady: wait0, + rerender, + } = await renderWithProviders( + , + ); + await wait0(); + // Frame 0: idx 0, 7 + // bits1 = 0x08 (⠈), bits2 = 0x01 (⠁) + expect(lastFrame0({ allowEmpty: true })?.trim()).toBe('⠈⠁'); + + await act(async () => { + rerender(); + }); + await wait0(); + // Frame 1: idx 1, 0 + // bits1 = 0, bits2 = 0x11 (⠑) + expect(lastFrame0({ allowEmpty: true })?.trim()).toBe('⠀⠑'); + + await act(async () => { + rerender(); + }); + await wait0(); + // Frame 2: idx 2, 1 + // bits2 = 0x30 (⠰) + expect(lastFrame0({ allowEmpty: true })?.trim()).toBe('⠀⠰'); + }); + + it('should render Medium variant frames (tail length 3)', async () => { + const { lastFrame, waitUntilReady, rerender } = await renderWithProviders( + , + ); + await waitUntilReady(); + + // Frame 0: idx 0, 7, 6 + // bits1 = 0x0A (⠊), bits2 = 0x01 (⠁) + expect(lastFrame({ allowEmpty: true })?.trim()).toBe('⠊⠁'); + + await act(async () => { + rerender(); + }); + await waitUntilReady(); + // Frame 1: idx 1, 0, 7 + // bits1 = 0x08 (⠈), bits2 = 0x11 (⠑) + expect(lastFrame({ allowEmpty: true })?.trim()).toBe('⠈⠑'); + }); + + it('should render Composite variant frames (12 ticks/shift)', async () => { + const { lastFrame, rerender, waitUntilReady } = await renderWithProviders( + , + ); + await waitUntilReady(); + + // index 0: length 2 + expect(lastFrame({ allowEmpty: true })?.trim()).toBe('⠈⠁'); + + await act(async () => { + rerender(); + }); + await waitUntilReady(); + // index 11: length 2, head shifted 11 mod 8 = 3 (C2.7) + // DOTS[3] = {0, 0x40}, DOTS[2] = {0, 0x20} + // bits2 = 0x40 | 0x20 = 0x60 (⠠) + // Actually the logic is: bits1 |= DOTS[idx].c1; bits2 |= DOTS[idx].c2; + // idx for i=0: (11-0+8)%8 = 3. idx for i=1: (11-1+8)%8 = 2. + // DOTS[3] = {c1:0, c2:0x40}. DOTS[2] = {c1:0, c2:0x20}. + // bits2 = 0x60. char2 = U+2860 (⡠). Wait, 0x2800 + 0x60 = 0x2860. + // Let me check braille chart for 0x60. + expect(lastFrame({ allowEmpty: true })?.trim()).toBe('⠀⡠'); + + await act(async () => { + rerender(); + }); + await waitUntilReady(); + // index 12: length 3, head shifted 12 mod 8 = 4 (C1.8) + // DOTS[4] = {0x80, 0}, DOTS[3] = {0, 0x40}, DOTS[2] = {0, 0x20} + // bits1 = 0x80 (⢀), bits2 = 0x40 | 0x20 = 0x60 (⡠) + expect(lastFrame({ allowEmpty: true })?.trim()).toBe('⢀⡠'); + }); + + it('should handle showSpinner setting', async () => { + const settings = createMockSettings({ + merged: { + ui: { showSpinner: false }, + }, + }); + + const { lastFrame, waitUntilReady } = await renderWithProviders( + , + { settings }, + ); + await waitUntilReady(); + expect(lastFrame({ allowEmpty: true })?.trim()).toBe(''); + }); +}); diff --git a/packages/cli/src/ui/components/CircularSpinner.tsx b/packages/cli/src/ui/components/CircularSpinner.tsx new file mode 100644 index 0000000000..ad44a13353 --- /dev/null +++ b/packages/cli/src/ui/components/CircularSpinner.tsx @@ -0,0 +1,141 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useState, useEffect, type FC } from 'react'; +import { Text } from 'ink'; +import { useSettings } from '../contexts/SettingsContext.js'; + +export type CircularSpinnerVariant = + | 'Small' + | 'Medium' + | 'Long' + | 'Composite' + | 'Static'; + +interface CircularSpinnerProps { + variant?: CircularSpinnerVariant; + color?: string; + /** Individual color for the first character. Overrides 'color' if provided. */ + color1?: string; + /** Individual color for the second character. Overrides 'color' if provided. */ + color2?: string; + /** Initial frame index when not controlled. */ + startFrameIndex?: number; + /** Directly control the frame index for testing. */ + frameIndex?: number; + /** Whether to animate even in test environment. */ + animateInTests?: boolean; +} + +const TICK_MS = 80; + +// Dot bitmasks for a circular perimeter across two characters (c1 and c2). +// Row 1: . C1.4 C2.1 . +// Row 2: C1.2 . . C2.5 +// Row 3: C1.3 . . C2.6 +// Row 4: . C1.8 C2.7 . +// Clockwise sequence: C2.1 -> C2.5 -> C2.6 -> C2.7 -> C1.8 -> C1.3 -> C1.2 -> C1.4 +const DOTS = [ + { c1: 0, c2: 0x01 }, // C2.1 + { c1: 0, c2: 0x10 }, // C2.5 + { c1: 0, c2: 0x20 }, // C2.6 + { c1: 0, c2: 0x40 }, // C2.7 + { c1: 0x80, c2: 0 }, // C1.8 + { c1: 0x04, c2: 0 }, // C1.3 + { c1: 0x02, c2: 0 }, // C1.2 + { c1: 0x08, c2: 0 }, // C1.4 +]; + +export const CircularSpinner: FC = ({ + variant = 'Medium', + color, + color1, + color2, + startFrameIndex = 0, + frameIndex: controlledFrameIndex, + animateInTests = false, +}) => { + const settings = useSettings(); + const showSpinner = settings.merged.ui?.showSpinner !== false; + const [internalFrameIndex, setInternalFrameIndex] = useState(startFrameIndex); + + useEffect(() => { + if (controlledFrameIndex === undefined) { + setInternalFrameIndex(startFrameIndex); + } + }, [startFrameIndex, controlledFrameIndex]); + + useEffect(() => { + if ( + !showSpinner || + variant === 'Static' || + controlledFrameIndex !== undefined || + (process.env['NODE_ENV'] === 'test' && !animateInTests) + ) + return; + + const interval = setInterval(() => { + setInternalFrameIndex((prev) => (prev + 1) % 144); + }, TICK_MS); + + return () => clearInterval(interval); + }, [showSpinner, variant, controlledFrameIndex, animateInTests]); + + if (!showSpinner) return null; + + if (variant === 'Static') { + return ⢎⡱; + } + + const effectiveFrameIndex = controlledFrameIndex ?? internalFrameIndex; + + const getTailLength = (index: number) => { + switch (variant) { + case 'Small': + return 2; + case 'Medium': + return 3; + case 'Long': { + const lengths = [1, 3, 5]; + return lengths[Math.floor(index / 8) % lengths.length]; + } + case 'Composite': { + const compositeLengths = [2, 3, 4, 5, 4, 3]; + return compositeLengths[ + Math.floor(index / 12) % compositeLengths.length + ]; + } + default: + return 3; + } + }; + + const tailLength = getTailLength(effectiveFrameIndex); + let bits1 = 0; + let bits2 = 0; + + for (let i = 0; i < tailLength; i++) { + const idx = ((effectiveFrameIndex % 8) - i + 8) % 8; + bits1 |= DOTS[idx].c1; + bits2 |= DOTS[idx].c2; + } + + const char1 = String.fromCharCode(0x2800 + bits1); + const char2 = String.fromCharCode(0x2800 + bits2); + + return ( + + {[ + + {char1} + , + + {char2} + , + ]} + + ); +}; diff --git a/packages/cli/src/ui/components/CliSpinner.test.tsx b/packages/cli/src/ui/components/CliSpinner.test.tsx index 4da6abb199..ff13bf70d7 100644 --- a/packages/cli/src/ui/components/CliSpinner.test.tsx +++ b/packages/cli/src/ui/components/CliSpinner.test.tsx @@ -24,11 +24,24 @@ describe('', () => { }); it('should not render when showSpinner is false', async () => { - const settings = createMockSettings({ ui: { showSpinner: false } }); + const settings = createMockSettings({ + merged: { + ui: { showSpinner: false }, + }, + }); const { lastFrame, unmount } = await renderWithProviders(, { settings, }); - expect(lastFrame({ allowEmpty: true })).toBe(''); + expect(lastFrame({ allowEmpty: true })?.trim()).toBe(''); + unmount(); + }); + + it('should render CircularSpinner when useBraille is true', async () => { + const { lastFrame, waitUntilReady, unmount } = await renderWithProviders( + , + ); + await waitUntilReady(); + expect(lastFrame()?.trim()).toBe('⢎⡱'); unmount(); }); }); diff --git a/packages/cli/src/ui/components/CliSpinner.tsx b/packages/cli/src/ui/components/CliSpinner.tsx index 66cb7a0281..fadd71a816 100644 --- a/packages/cli/src/ui/components/CliSpinner.tsx +++ b/packages/cli/src/ui/components/CliSpinner.tsx @@ -8,10 +8,21 @@ import Spinner from 'ink-spinner'; import { type ComponentProps, useEffect } from 'react'; import { debugState } from '../debug.js'; import { useSettings } from '../contexts/SettingsContext.js'; +import { + CircularSpinner, + type CircularSpinnerVariant, +} from './CircularSpinner.js'; -export type SpinnerProps = ComponentProps; +type SpinnerProps = ComponentProps & { + useBraille?: boolean; + variant?: CircularSpinnerVariant; +}; -export const CliSpinner = (props: SpinnerProps) => { +export const CliSpinner = ({ + useBraille = false, + variant = 'Medium', + ...props +}: SpinnerProps) => { const settings = useSettings(); const shouldShow = settings.merged.ui?.showSpinner !== false; @@ -29,5 +40,9 @@ export const CliSpinner = (props: SpinnerProps) => { return null; } + if (useBraille) { + return ; + } + return ; }; diff --git a/packages/cli/src/ui/components/GeminiRespondingSpinner.tsx b/packages/cli/src/ui/components/GeminiRespondingSpinner.tsx index 316438d737..86d9dbe2dd 100644 --- a/packages/cli/src/ui/components/GeminiRespondingSpinner.tsx +++ b/packages/cli/src/ui/components/GeminiRespondingSpinner.tsx @@ -6,7 +6,6 @@ import type React from 'react'; import { Text, useIsScreenReaderEnabled } from 'ink'; -import type { SpinnerName } from 'cli-spinners'; import { useStreamingContext } from '../contexts/StreamingContext.js'; import { StreamingState } from '../types.js'; import { @@ -15,6 +14,7 @@ import { } from '../textConstants.js'; import { theme } from '../semantic-colors.js'; import { GeminiSpinner } from './GeminiSpinner.js'; +import { type CircularSpinnerVariant } from './CircularSpinner.js'; interface GeminiRespondingSpinnerProps { /** @@ -22,7 +22,7 @@ interface GeminiRespondingSpinnerProps { * If not provided and not Responding, renders null. */ nonRespondingDisplay?: string; - spinnerType?: SpinnerName; + variant?: CircularSpinnerVariant; /** * If true, we prioritize showing the nonRespondingDisplay (hook icon) * even if the state is Responding. @@ -35,7 +35,7 @@ export const GeminiRespondingSpinner: React.FC< GeminiRespondingSpinnerProps > = ({ nonRespondingDisplay, - spinnerType = 'dots', + variant = 'Composite', isHookActive = false, color, }) => { @@ -46,10 +46,7 @@ export const GeminiRespondingSpinner: React.FC< // to be consistent, instead of the rainbow spinner which means "Gemini is talking". if (streamingState === StreamingState.Responding && !isHookActive) { return ( - + ); } diff --git a/packages/cli/src/ui/components/GeminiSpinner.test.tsx b/packages/cli/src/ui/components/GeminiSpinner.test.tsx new file mode 100644 index 0000000000..db715a99b1 --- /dev/null +++ b/packages/cli/src/ui/components/GeminiSpinner.test.tsx @@ -0,0 +1,41 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { renderWithProviders } from '../../test-utils/render.js'; +import { GeminiSpinner } from './GeminiSpinner.js'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { useIsScreenReaderEnabled } from 'ink'; + +vi.mock('ink', async () => { + const actual = await vi.importActual('ink'); + return { + ...actual, + useIsScreenReaderEnabled: vi.fn(), + }; +}); + +describe('', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should render CircularSpinner when screen reader is disabled', async () => { + vi.mocked(useIsScreenReaderEnabled).mockReturnValue(false); + const { lastFrame, unmount } = await renderWithProviders(); + // Component renders immediately. The interval updates state, but we don't need to wait for it. + expect(lastFrame()).toBeTruthy(); + unmount(); + }); + + it('should render altText when screen reader is enabled', async () => { + vi.mocked(useIsScreenReaderEnabled).mockReturnValue(true); + const { lastFrame, unmount } = await renderWithProviders( + , + ); + expect(lastFrame()?.trim()).toBe('Custom Loading'); + unmount(); + }); +}); diff --git a/packages/cli/src/ui/components/GeminiSpinner.tsx b/packages/cli/src/ui/components/GeminiSpinner.tsx index 37d1930625..b8452d27aa 100644 --- a/packages/cli/src/ui/components/GeminiSpinner.tsx +++ b/packages/cli/src/ui/components/GeminiSpinner.tsx @@ -7,20 +7,22 @@ import type React from 'react'; import { useState, useEffect, useMemo } from 'react'; import { Text, useIsScreenReaderEnabled } from 'ink'; -import { CliSpinner } from './CliSpinner.js'; -import type { SpinnerName } from 'cli-spinners'; +import { + CircularSpinner, + type CircularSpinnerVariant, +} from './CircularSpinner.js'; import { Colors } from '../colors.js'; import tinygradient from 'tinygradient'; const COLOR_CYCLE_DURATION_MS = 4000; interface GeminiSpinnerProps { - spinnerType?: SpinnerName; + variant?: CircularSpinnerVariant; altText?: string; } export const GeminiSpinner: React.FC = ({ - spinnerType = 'dots', + variant = 'Composite', altText, }) => { const isScreenReaderEnabled = useIsScreenReaderEnabled(); @@ -51,13 +53,18 @@ export const GeminiSpinner: React.FC = ({ }, [isScreenReaderEnabled]); const progress = (time % COLOR_CYCLE_DURATION_MS) / COLOR_CYCLE_DURATION_MS; - const currentColor = googleGradient.rgbAt(progress).toHexString(); + const leadingColor = googleGradient.rgbAt(progress).toHexString(); + // Offset the trailing color by ~10% of the cycle duration + const trailingProgress = (progress - 0.1 + 1) % 1; + const trailingColor = googleGradient.rgbAt(trailingProgress).toHexString(); return isScreenReaderEnabled ? ( {altText} ) : ( - - - + ); }; diff --git a/packages/cli/src/ui/components/LoadingIndicator.test.tsx b/packages/cli/src/ui/components/LoadingIndicator.test.tsx index ef2e21e132..0a761f96f0 100644 --- a/packages/cli/src/ui/components/LoadingIndicator.test.tsx +++ b/packages/cli/src/ui/components/LoadingIndicator.test.tsx @@ -86,7 +86,7 @@ describe('', () => { ); await waitUntilReady(); const output = lastFrame(); - expect(output).toContain('⠏'); // Static char for WaitingForConfirmation + expect(output).toContain('⢎⡱'); // Static char for WaitingForConfirmation expect(output).toContain('Confirm action'); expect(output).not.toContain('(esc to cancel)'); expect(output).not.toContain(', 10s'); @@ -208,7 +208,7 @@ describe('', () => { }); await waitUntilReady(); output = lastFrame(); - expect(output).toContain('⠏'); + expect(output).toContain('⢎⡱'); expect(output).toContain('Please Confirm'); expect(output).not.toContain('(esc to cancel)'); expect(output).not.toContain(', 15s'); @@ -461,7 +461,7 @@ describe('', () => { await waitUntilReady(); const output = lastFrame(); expect(output).toContain('?'); - expect(output).not.toContain('⠏'); + expect(output).not.toContain('⢎⡱'); unmount(); }); }); diff --git a/packages/cli/src/ui/components/LoadingIndicator.tsx b/packages/cli/src/ui/components/LoadingIndicator.tsx index a48451b26c..86a138c81b 100644 --- a/packages/cli/src/ui/components/LoadingIndicator.tsx +++ b/packages/cli/src/ui/components/LoadingIndicator.tsx @@ -97,7 +97,7 @@ export const LoadingIndicator: React.FC = ({ nonRespondingDisplay={ spinnerIcon ?? (streamingState === StreamingState.WaitingForConfirmation - ? '⠏' + ? '⢎⡱' : '') } isHookActive={isHookActive} @@ -141,7 +141,7 @@ export const LoadingIndicator: React.FC = ({ nonRespondingDisplay={ spinnerIcon ?? (streamingState === StreamingState.WaitingForConfirmation - ? '⠏' + ? '⢎⡱' : '') } isHookActive={isHookActive} diff --git a/packages/cli/src/ui/components/MainContent.test.tsx b/packages/cli/src/ui/components/MainContent.test.tsx index b6bc0795eb..45ea98164e 100644 --- a/packages/cli/src/ui/components/MainContent.test.tsx +++ b/packages/cli/src/ui/components/MainContent.test.tsx @@ -53,6 +53,10 @@ vi.mock('../contexts/AppContext.js', async () => { }; }); +vi.mock('./GeminiSpinner.js', () => ({ + GeminiSpinner: () => null, +})); + vi.mock('../hooks/useAlternateBuffer.js', () => ({ useAlternateBuffer: vi.fn(), })); diff --git a/packages/cli/src/ui/components/spinner-spec.md b/packages/cli/src/ui/components/spinner-spec.md new file mode 100644 index 0000000000..ade0fede10 --- /dev/null +++ b/packages/cli/src/ui/components/spinner-spec.md @@ -0,0 +1,29 @@ +# Task: Update Braille Loader to Circular Spinner (Clean Re-implementation) + +## Objective + +Replace the legacy braille animation with a smoother, circular 8-dot spinner +effect spanning two Braille characters. + +## Technical Requirements + +- **Character Set**: Utilize the Braille Patterns Unicode block (U+2800 - + U+28FF). +- **Dot Mapping**: Map 8 dots in a circular perimeter across two characters (c1 + and c2). +- **Variants**: + - `Static`: Fixed frame `⢎⡱` for confirmation states. + - `Small`: Fixed tail length of 2. + - `Medium`: Fixed tail length of 3. + - `Long`: Phased growth (lengths 1, 3, 5). + - `Composite`: Dynamic length sequence `[2, 3, 4, 5, 4, 3]`. +- **Performance**: Use `setInterval` with a default 80ms tick, gated by + `SettingsContext` (showSpinner). +- **Testing**: Update `CircularSpinner.test.tsx` verification frames and ensure + `LoadingIndicator.test.tsx` matches the static `⢎⡱` frame. + +## Implementation Steps + +1. Define the circular `DOTS` bitmask array. +2. Implement `getFrame()` logic using `String.fromCharCode(0x2800 + bits)`. +3. Verify with `npm test`.