Improve tracking of animated components. (#12618)

packages/cli/src/ui/components/CliSpinner.test.tsx

@@ -0,0 +1,24 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { render } from '../../test-utils/render.js';
+import { CliSpinner } from './CliSpinner.js';
+import { debugState } from '../debug.js';
+import { describe, it, expect, beforeEach } from 'vitest';
+
+describe('<CliSpinner />', () => {
+  beforeEach(() => {
+    debugState.debugNumAnimatedComponents = 0;
+  });
+
+  it('should increment debugNumAnimatedComponents on mount and decrement on unmount', () => {
+    expect(debugState.debugNumAnimatedComponents).toBe(0);
+    const { unmount } = render(<CliSpinner />);
+    expect(debugState.debugNumAnimatedComponents).toBe(1);
+    unmount();
+    expect(debugState.debugNumAnimatedComponents).toBe(0);
+  });
+});

packages/cli/src/ui/components/CliSpinner.tsx

@@ -6,17 +6,15 @@
 
 import Spinner from 'ink-spinner';
 import { type ComponentProps, useEffect } from 'react';
-
+import { debugState } from '../debug.js';
-// A top-level field to track the total number of active spinners.
-export let debugNumSpinners = 0;
 
 export type SpinnerProps = ComponentProps<typeof Spinner>;
 
 export const CliSpinner = (props: SpinnerProps) => {
   useEffect(() => {
-    debugNumSpinners++;
+    debugState.debugNumAnimatedComponents++;
     return () => {
-      debugNumSpinners--;
+      debugState.debugNumAnimatedComponents--;
     };
   }, []);
 

packages/cli/src/ui/components/DebugProfiler.test.tsx

@@ -12,6 +12,7 @@ import {
   FRAME_TIMESTAMP_CAPACITY,
 } from './DebugProfiler.js';
 import { FixedDeque } from 'mnemonist';
+import { debugState } from '../debug.js';
 
 describe('DebugProfiler', () => {
   beforeEach(() => {
@@ -29,12 +30,14 @@ describe('DebugProfiler', () => {
       Array,
       ACTION_TIMESTAMP_CAPACITY,
     );
+    debugState.debugNumAnimatedComponents = 0;
   });
 
   afterEach(() => {
     vi.restoreAllMocks();
     profiler.actionTimestamps.clear();
     profiler.possiblyIdleFrameTimestamps.clear();
+    debugState.debugNumAnimatedComponents = 0;
   });
 
   it('should not exceed action timestamp capacity', () => {
@@ -193,4 +196,20 @@ describe('DebugProfiler', () => {
 
     expect(profiler.totalIdleFrames).toBe(0);
   });
+
+  it('should not report frames as idle if debugNumAnimatedComponents > 0', async () => {
+    const startTime = Date.now();
+    vi.setSystemTime(startTime);
+    debugState.debugNumAnimatedComponents = 1;
+
+    for (let i = 0; i < 5; i++) {
+      profiler.reportFrameRendered();
+      vi.advanceTimersByTime(20);
+    }
+
+    vi.advanceTimersByTime(1000);
+    profiler.checkForIdleFrames();
+
+    expect(profiler.totalIdleFrames).toBe(0);
+  });
 });

packages/cli/src/ui/components/DebugProfiler.tsx

@@ -9,7 +9,7 @@ import { useEffect, useState } from 'react';
 import { FixedDeque } from 'mnemonist';
 import { theme } from '../semantic-colors.js';
 import { useUIState } from '../contexts/UIStateContext.js';
-import { debugNumSpinners } from './CliSpinner.js';
+import { debugState } from '../debug.js';
 import { appEvents, AppEvent } from '../../utils/events.js';
 
 // Frames that render at least this far before or after an action are considered
@@ -52,7 +52,7 @@ export const profiler = {
     if (now - this.lastFrameStartTime > 16) {
       this.lastFrameStartTime = now;
       this.numFrames++;
-      if (debugNumSpinners === 0) {
+      if (debugState.debugNumAnimatedComponents === 0) {
         if (this.possiblyIdleFrameTimestamps.size >= FRAME_TIMESTAMP_CAPACITY) {
           this.possiblyIdleFrameTimestamps.shift();
         }

packages/cli/src/ui/debug.ts

@@ -0,0 +1,11 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+// A top-level field to track the total number of active animated components.
+// This is used for testing to ensure we wait for animations to finish.
+export const debugState = {
+  debugNumAnimatedComponents: 0,
+};

packages/cli/src/ui/hooks/useAnimatedScrollbar.test.tsx

@@ -0,0 +1,73 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { act } from 'react';
+import { render } from '../../test-utils/render.js';
+import { useAnimatedScrollbar } from './useAnimatedScrollbar.js';
+import { debugState } from '../debug.js';
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+
+const TestComponent = ({ isFocused = false }: { isFocused?: boolean }) => {
+  useAnimatedScrollbar(isFocused, () => {});
+  return null;
+};
+
+describe('useAnimatedScrollbar', () => {
+  beforeEach(() => {
+    debugState.debugNumAnimatedComponents = 0;
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  it('should not increment debugNumAnimatedComponents when not focused', () => {
+    render(<TestComponent isFocused={false} />);
+    expect(debugState.debugNumAnimatedComponents).toBe(0);
+  });
+
+  it('should not increment debugNumAnimatedComponents on initial mount even if focused', () => {
+    render(<TestComponent isFocused={true} />);
+    expect(debugState.debugNumAnimatedComponents).toBe(0);
+  });
+
+  it('should increment debugNumAnimatedComponents when becoming focused', () => {
+    const { rerender } = render(<TestComponent isFocused={false} />);
+    expect(debugState.debugNumAnimatedComponents).toBe(0);
+    rerender(<TestComponent isFocused={true} />);
+    expect(debugState.debugNumAnimatedComponents).toBe(1);
+  });
+
+  it('should decrement debugNumAnimatedComponents when becoming unfocused', () => {
+    const { rerender } = render(<TestComponent isFocused={false} />);
+    rerender(<TestComponent isFocused={true} />);
+    expect(debugState.debugNumAnimatedComponents).toBe(1);
+    rerender(<TestComponent isFocused={false} />);
+    expect(debugState.debugNumAnimatedComponents).toBe(0);
+  });
+
+  it('should decrement debugNumAnimatedComponents on unmount', () => {
+    const { rerender, unmount } = render(<TestComponent isFocused={false} />);
+    rerender(<TestComponent isFocused={true} />);
+    expect(debugState.debugNumAnimatedComponents).toBe(1);
+    unmount();
+    expect(debugState.debugNumAnimatedComponents).toBe(0);
+  });
+
+  it('should decrement debugNumAnimatedComponents after animation finishes', async () => {
+    const { rerender } = render(<TestComponent isFocused={false} />);
+    rerender(<TestComponent isFocused={true} />);
+    expect(debugState.debugNumAnimatedComponents).toBe(1);
+
+    // Advance timers by enough time for animation to complete (200 + 1000 + 300 + buffer)
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(2000);
+    });
+
+    expect(debugState.debugNumAnimatedComponents).toBe(0);
+  });
+});

packages/cli/src/ui/hooks/useAnimatedScrollbar.ts

@@ -7,6 +7,7 @@
 import { useState, useEffect, useRef, useCallback } from 'react';
 import { theme } from '../semantic-colors.js';
 import { interpolateColor } from '../themes/color-utils.js';
+import { debugState } from '../debug.js';
 
 export function useAnimatedScrollbar(
   isFocused: boolean,
@@ -18,8 +19,13 @@ export function useAnimatedScrollbar(
 
   const animationFrame = useRef<NodeJS.Timeout | null>(null);
   const timeout = useRef<NodeJS.Timeout | null>(null);
+  const isAnimatingRef = useRef(false);
 
   const cleanup = useCallback(() => {
+    if (isAnimatingRef.current) {
+      debugState.debugNumAnimatedComponents--;
+      isAnimatingRef.current = false;
+    }
     if (animationFrame.current) {
       clearInterval(animationFrame.current);
       animationFrame.current = null;
@@ -32,6 +38,8 @@ export function useAnimatedScrollbar(
 
   const flashScrollbar = useCallback(() => {
     cleanup();
+    debugState.debugNumAnimatedComponents++;
+    isAnimatingRef.current = true;
 
     const fadeInDuration = 200;
     const visibleDuration = 1000;
@@ -67,10 +75,7 @@ export function useAnimatedScrollbar(
             );
 
             if (progress === 1) {
-              if (animationFrame.current) {
+              cleanup();
-                clearInterval(animationFrame.current);
-                animationFrame.current = null;
-              }
             }
           };