Fix terminal content resizing on expansion (#1512)

* auto-claude: subtask-1-1 - Enhance useXterm fit() function to validate container dimensions * auto-claude: subtask-1-2 - Replace fixed timeout in Terminal.tsx expansion handler - Replace fixed TERMINAL_DOM_UPDATE_DELAY_MS timeout with transitionend event listener and RAF-based retry logic for terminal expansion resize handling - Add transitionend listeners on terminal container and parent element to detect when CSS transitions complete - Implement performFit() with requestAnimationFrame and retry logic (max 5 retries, 50ms apart) following the pattern from useXterm.ts performInitialFit - Add 300ms fallback timeout as safety net for edge cases where transitionend doesn't fire - Remove unused TERMINAL_DOM_UPDATE_DELAY_MS import - Properly clean up all event listeners, RAF, and timeouts in useEffect cleanup This ensures terminal content properly resizes after expansion/collapse transitions complete, rather than relying on a fixed timeout that may fire before layout changes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * auto-claude: subtask-1-3 - Update fit() return type in useXterm hook and ensure consistency - Add explicit UseXtermReturn interface documenting all hook return values - Document fit() return type as boolean in the interface with JSDoc - Apply UseXtermReturn return type annotation to useXterm function - Export UseXtermReturn for use by consuming components This ensures type consistency across all usages of the fit() function, making the boolean return value explicit and documented. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix code review findings in Terminal.tsx resize logic - Cancel existing RAF before scheduling new one in performFit() to prevent multiple concurrent fit attempts when rapid transitionend events fire - Add fitSucceeded flag to prevent redundant fallback timeout execution after successful fit via transitionend - Reset fitSucceeded flag in handleTransitionEnd for new transitions These improvements address code review findings while maintaining the existing resize behavior and fixing potential performance issues. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-26 09:46:46 +01:00
parent 63e2847fc5
commit 9f6c0026bc
2 changed files with 155 additions and 9 deletions
@@ -8,7 +8,6 @@ import { useSettingsStore } from '../stores/settings-store';
 import { useToast } from '../hooks/use-toast';
 import type { TerminalProps } from './terminal/types';
 import type { TerminalWorktreeConfig } from '../../shared/types';
-import { TERMINAL_DOM_UPDATE_DELAY_MS } from '../../shared/constants';
 import { TerminalHeader } from './terminal/TerminalHeader';
 import { CreateWorktreeDialog } from './terminal/CreateWorktreeDialog';
 import { useXterm } from './terminal/useXterm';
@@ -210,11 +209,113 @@ export const Terminal = forwardRef<TerminalHandle, TerminalProps>(function Termi
  }, [isActive, focus]);

  // Refit terminal when expansion state changes
+  // Uses transitionend event listener and RAF-based retry logic instead of fixed timeout
+  // for more reliable resizing after CSS transitions complete
  useEffect(() => {
-    const timeoutId = setTimeout(() => {
-      fit();
-    }, TERMINAL_DOM_UPDATE_DELAY_MS);
-    return () => clearTimeout(timeoutId);
+    // RAF fallback for test environments where requestAnimationFrame may not be defined
+    const raf = typeof requestAnimationFrame !== 'undefined'
+      ? requestAnimationFrame
+      : (cb: FrameRequestCallback) => setTimeout(() => cb(Date.now()), 0) as unknown as number;
+
+    const cancelRaf = typeof cancelAnimationFrame !== 'undefined'
+      ? cancelAnimationFrame
+      : (id: number) => clearTimeout(id);
+
+    let rafId: number | null = null;
+    let retryTimeoutId: ReturnType<typeof setTimeout> | null = null;
+    let fallbackTimeoutId: ReturnType<typeof setTimeout> | null = null;
+    let isCleanedUp = false;
+    let fitSucceeded = false;
+    let retryCount = 0;
+    const MAX_RETRIES = 5;
+    const RETRY_DELAY_MS = 50;
+    const FALLBACK_TIMEOUT_MS = 300;
+
+    // Perform fit with RAF and retry logic, following the pattern from useXterm.ts performInitialFit
+    const performFit = () => {
+      if (isCleanedUp) return;
+
+      // Cancel any existing RAF to prevent multiple concurrent fit attempts
+      if (rafId !== null) {
+        cancelRaf(rafId);
+        rafId = null;
+      }
+
+      rafId = raf(() => {
+        if (isCleanedUp) return;
+
+        // fit() returns boolean indicating success (true if container had valid dimensions)
+        const success = fit();
+
+        if (success) {
+          fitSucceeded = true;
+        } else if (retryCount < MAX_RETRIES) {
+          // Container not ready yet, retry after a short delay
+          retryCount++;
+          retryTimeoutId = setTimeout(performFit, RETRY_DELAY_MS);
+        }
+      });
+    };
+
+    // Get terminal container element for transition listening
+    const container = terminalRef.current;
+
+    // Handler for transitionend event - fits terminal after CSS transition completes
+    const handleTransitionEnd = (e: TransitionEvent) => {
+      // Only react to relevant transitions (height, width, flex changes)
+      const relevantProps = ['height', 'width', 'flex', 'max-height', 'max-width'];
+      if (relevantProps.some(prop => e.propertyName.includes(prop))) {
+        // Reset retry count and success flag for new transition
+        retryCount = 0;
+        fitSucceeded = false;
+        performFit();
+      }
+    };
+
+    // Listen for transitionend on the terminal container and its parent
+    // (expansion may trigger transitions on either element)
+    if (container) {
+      container.addEventListener('transitionend', handleTransitionEnd);
+      container.parentElement?.addEventListener('transitionend', handleTransitionEnd);
+    }
+
+    // Start the fit process immediately with RAF-based retry
+    // This handles cases where expansion is instant (no CSS transition)
+    performFit();
+
+    // Fallback timeout to ensure fit happens even if transitionend doesn't fire
+    // This is a safety net for edge cases
+    fallbackTimeoutId = setTimeout(() => {
+      if (!isCleanedUp && !fitSucceeded) {
+        retryCount = 0;
+        performFit();
+      }
+    }, FALLBACK_TIMEOUT_MS);
+
+    return () => {
+      isCleanedUp = true;
+
+      // Clean up RAF
+      if (rafId !== null) {
+        cancelRaf(rafId);
+      }
+
+      // Clean up retry timeout
+      if (retryTimeoutId !== null) {
+        clearTimeout(retryTimeoutId);
+      }
+
+      // Clean up fallback timeout
+      if (fallbackTimeoutId !== null) {
+        clearTimeout(fallbackTimeoutId);
+      }
+
+      // Remove event listeners
+      if (container) {
+        container.removeEventListener('transitionend', handleTransitionEnd);
+        container.parentElement?.removeEventListener('transitionend', handleTransitionEnd);
+      }
+    };
  }, [isExpanded, fit]);

  // Trigger deferred Claude resume when terminal becomes active
@@ -24,6 +24,38 @@ interface UseXtermOptions {
  onDimensionsReady?: (cols: number, rows: number) => void;
 }

+/**
+ * Return type for the useXterm hook.
+ * Provides terminal control methods and state.
+ */
+export interface UseXtermReturn {
+  /** Ref to attach to the terminal container div */
+  terminalRef: React.RefObject<HTMLDivElement | null>;
+  /** Ref to the xterm.js Terminal instance */
+  xtermRef: React.MutableRefObject<XTerm | null>;
+  /** Ref to the FitAddon instance */
+  fitAddonRef: React.MutableRefObject<FitAddon | null>;
+  /**
+   * Fit the terminal content to the container dimensions.
+   * @returns boolean indicating whether fit was successful (had valid dimensions)
+   */
+  fit: () => boolean;
+  /** Write data to the terminal */
+  write: (data: string) => void;
+  /** Write a line to the terminal */
+  writeln: (data: string) => void;
+  /** Focus the terminal */
+  focus: () => void;
+  /** Dispose of the terminal and clean up resources */
+  dispose: () => void;
+  /** Current number of columns */
+  cols: number;
+  /** Current number of rows */
+  rows: number;
+  /** Whether dimensions have been measured and are ready */
+  dimensionsReady: boolean;
+}
+
 // Debounce helper function
 function debounce<T extends (...args: unknown[]) => void>(fn: T, ms: number): T {
  let timeoutId: ReturnType<typeof setTimeout> | null = null;
@@ -33,7 +65,7 @@ function debounce<T extends (...args: unknown[]) => void>(fn: T, ms: number): T
  }) as T;
 }

-export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsReady }: UseXtermOptions) {
+export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsReady }: UseXtermOptions): UseXtermReturn {
  const terminalRef = useRef<HTMLDivElement>(null);
  const xtermRef = useRef<XTerm | null>(null);
  const fitAddonRef = useRef<FitAddon | null>(null);
@@ -379,10 +411,23 @@ export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsRea
    return () => window.removeEventListener('terminal-refit-all', handleRefitAll);
  }, []);

-  const fit = useCallback(() => {
-    if (fitAddonRef.current && xtermRef.current) {
-      fitAddonRef.current.fit();
+  /**
+   * Fit the terminal content to the container dimensions.
+   * @returns boolean indicating whether fit was successful (had valid dimensions)
+   */
+  const fit = useCallback((): boolean => {
+    if (fitAddonRef.current && xtermRef.current && terminalRef.current) {
+      // Validate container has valid dimensions before fitting
+      const rect = terminalRef.current.getBoundingClientRect();
+      if (rect.width > 0 && rect.height > 0) {
+        fitAddonRef.current.fit();
+        const cols = xtermRef.current.cols;
+        const rows = xtermRef.current.rows;
+        setDimensions({ cols, rows });
+        return true;
+      }
    }
+    return false;
  }, []);

  const write = useCallback((data: string) => {