refactor: clean up recording overlay code, add roundRect fallback

ammar-agent · ammar-agent · commit a549122956cf · 2025-12-03T09:55:56.000-06:00
- Deduplicate mode-based styling with lookup tables
- Extract RecordingHints component for keyboard shortcuts
- Simplify VoiceInputButton color logic
- Add roundRect fallback for older browser compatibility
- Improve code organization and comments
diff --git a/src/browser/components/ChatInput/RecordingOverlay.tsx b/src/browser/components/ChatInput/RecordingOverlay.tsx
@@ -10,16 +10,27 @@ import { formatKeybind, KEYBINDS } from "@/browser/utils/ui/keybinds";
 import type { UIMode } from "@/common/types/mode";
 import type { VoiceInputState } from "@/browser/hooks/useVoiceInput";
 
-// Mode color values for the visualizer (CSS var values from globals.css)
-const MODE_COLORS = {
-  plan: "hsl(210, 70%, 55%)", // Slightly lighter than --color-plan-mode for visibility
-  exec: "hsl(268, 94%, 65%)", // Slightly lighter than --color-exec-mode for visibility
-} as const;
+/** Canvas fill colors for the waveform (slightly lighter than CSS vars for visibility) */
+const MODE_COLORS: Record<UIMode, string> = {
+  plan: "hsl(210, 70%, 55%)",
+  exec: "hsl(268, 94%, 65%)",
+};
+
+/** Tailwind classes for recording state, keyed by mode */
+const RECORDING_CLASSES: Record<UIMode, string> = {
+  plan: "cursor-pointer border-plan-mode bg-plan-mode/10",
+  exec: "cursor-pointer border-exec-mode bg-exec-mode/10",
+};
 
-// Sliding window config
-const WINDOW_DURATION_MS = 10000; // 10 seconds of history
-const SAMPLE_INTERVAL_MS = 50; // Sample every 50ms
-const NUM_SAMPLES = Math.floor(WINDOW_DURATION_MS / SAMPLE_INTERVAL_MS); // 200 samples
+const TEXT_CLASSES: Record<UIMode, string> = {
+  plan: "text-plan-mode-light",
+  exec: "text-exec-mode-light",
+};
+
+// Waveform shows last 10 seconds of audio, sampled every 50ms (200 samples)
+const WINDOW_DURATION_MS = 10_000;
+const SAMPLE_INTERVAL_MS = 50;
+const NUM_SAMPLES = WINDOW_DURATION_MS / SAMPLE_INTERVAL_MS;
 
 interface RecordingOverlayProps {
   state: VoiceInputState;
@@ -32,16 +43,9 @@ export const RecordingOverlay: React.FC<RecordingOverlayProps> = (props) => {
   const isRecording = props.state === "recording";
   const isTranscribing = props.state === "transcribing";
 
-  const modeColor = MODE_COLORS[props.mode];
-
-  // Border and background classes based on state
   const containerClasses = cn(
     "mb-1 flex w-full flex-col items-center justify-center gap-1 rounded-md border px-3 py-2 transition-all focus:outline-none",
-    isRecording
-      ? props.mode === "plan"
-        ? "cursor-pointer border-plan-mode bg-plan-mode/10"
-        : "cursor-pointer border-exec-mode bg-exec-mode/10"
-      : "cursor-wait border-amber-500 bg-amber-500/10"
+    isRecording ? RECORDING_CLASSES[props.mode] : "cursor-wait border-amber-500 bg-amber-500/10"
   );
 
   return (
@@ -52,63 +56,66 @@ export const RecordingOverlay: React.FC<RecordingOverlayProps> = (props) => {
       className={containerClasses}
       aria-label={isRecording ? "Stop recording" : "Transcribing..."}
     >
-      {/* Visualizer / Animation Area */}
       <div className="flex h-8 w-full items-center justify-center">
         {isRecording && props.mediaRecorder ? (
-          <SlidingWaveform mediaRecorder={props.mediaRecorder} color={modeColor} height={32} />
+          <SlidingWaveform
+            mediaRecorder={props.mediaRecorder}
+            color={MODE_COLORS[props.mode]}
+            height={32}
+          />
         ) : (
-          <TranscribingAnimation />
+          <Loader2 className="h-5 w-5 animate-spin text-amber-500" />
         )}
       </div>
 
-      {/* Status Text */}
       <span
         className={cn(
           "text-xs font-medium",
-          isRecording
-            ? props.mode === "plan"
-              ? "text-plan-mode-light"
-              : "text-exec-mode-light"
-            : "text-amber-500"
+          isRecording ? TEXT_CLASSES[props.mode] : "text-amber-500"
         )}
       >
-        {isRecording ? (
-          <>
-            <span className="opacity-70">space</span> send ·{" "}
-            <span className="opacity-70">{formatKeybind(KEYBINDS.TOGGLE_VOICE_INPUT)}</span> review
-            · <span className="opacity-70">esc</span> cancel
-          </>
-        ) : (
-          "Transcribing..."
-        )}
+        {isRecording ? <RecordingHints /> : "Transcribing..."}
       </span>
     </button>
   );
 };
 
-/**
- * Sliding window waveform - shows amplitude over the last ~10 seconds.
- * New samples appear on the right and slide left over time.
- */
+/** Keyboard hint display for recording state */
+const RecordingHints: React.FC = () => (
+  <>
+    <span className="opacity-70">space</span> send ·{" "}
+    <span className="opacity-70">{formatKeybind(KEYBINDS.TOGGLE_VOICE_INPUT)}</span> review ·{" "}
+    <span className="opacity-70">esc</span> cancel
+  </>
+);
+
+// =============================================================================
+// SlidingWaveform - Canvas-based amplitude visualization
+// =============================================================================
+
 interface SlidingWaveformProps {
   mediaRecorder: MediaRecorder;
   color: string;
   height: number;
 }
 
+/**
+ * Renders a sliding window of audio amplitude over time.
+ * New samples appear on the right and scroll left as time passes.
+ */
 const SlidingWaveform: React.FC<SlidingWaveformProps> = (props) => {
   const canvasRef = useRef<HTMLCanvasElement>(null);
   const containerRef = useRef<HTMLDivElement>(null);
   const [containerWidth, setContainerWidth] = useState(600);
 
-  // Audio analysis refs (persist across renders)
+  // Audio analysis state (refs to avoid re-renders)
   const audioContextRef = useRef<AudioContext | null>(null);
   const analyserRef = useRef<AnalyserNode | null>(null);
   const samplesRef = useRef<number[]>(new Array<number>(NUM_SAMPLES).fill(0));
   const animationFrameRef = useRef<number>(0);
   const lastSampleTimeRef = useRef<number>(0);
 
-  // Measure container width
+  // Track container width for responsive canvas
   useLayoutEffect(() => {
     const container = containerRef.current;
     if (!container) return;
@@ -118,14 +125,13 @@ const SlidingWaveform: React.FC<SlidingWaveformProps> = (props) => {
         setContainerWidth(entry.contentRect.width);
       }
     });
-
     observer.observe(container);
     setContainerWidth(container.offsetWidth);
 
     return () => observer.disconnect();
   }, []);
 
-  // Set up audio analysis
+  // Initialize Web Audio API analyser
   useEffect(() => {
     const stream = props.mediaRecorder.stream;
     if (!stream) return;
@@ -140,8 +146,6 @@ const SlidingWaveform: React.FC<SlidingWaveformProps> = (props) => {
 
     audioContextRef.current = audioContext;
     analyserRef.current = analyser;
-
-    // Reset samples when starting
     samplesRef.current = new Array<number>(NUM_SAMPLES).fill(0);
     lastSampleTimeRef.current = performance.now();
 
@@ -152,7 +156,7 @@ const SlidingWaveform: React.FC<SlidingWaveformProps> = (props) => {
     };
   }, [props.mediaRecorder]);
 
-  // Animation loop - sample audio and render
+  // Animation loop: sample audio amplitude and render bars
   const draw = useCallback(() => {
     const canvas = canvasRef.current;
     const analyser = analyserRef.current;
@@ -161,73 +165,60 @@ const SlidingWaveform: React.FC<SlidingWaveformProps> = (props) => {
     const ctx = canvas.getContext("2d");
     if (!ctx) return;
 
+    // Sample audio at fixed intervals
     const now = performance.now();
-    const timeSinceLastSample = now - lastSampleTimeRef.current;
-
-    // Take a new sample if enough time has passed
-    if (timeSinceLastSample >= SAMPLE_INTERVAL_MS) {
+    if (now - lastSampleTimeRef.current >= SAMPLE_INTERVAL_MS) {
       const dataArray = new Uint8Array(analyser.frequencyBinCount);
       analyser.getByteTimeDomainData(dataArray);
 
-      // Calculate RMS amplitude (0-1 range)
+      // Calculate RMS (root mean square) amplitude
       let sum = 0;
       for (const sample of dataArray) {
-        const normalized = (sample - 128) / 128; // -1 to 1
+        const normalized = (sample - 128) / 128;
         sum += normalized * normalized;
       }
       const rms = Math.sqrt(sum / dataArray.length);
 
-      // Shift samples left and add new one
       samplesRef.current.shift();
       samplesRef.current.push(rms);
       lastSampleTimeRef.current = now;
     }
 
-    // Clear canvas
+    // Render bars
     ctx.clearRect(0, 0, canvas.width, canvas.height);
 
-    // Draw waveform bars - calculate to fill full width
     const samples = samplesRef.current;
     const numBars = samples.length;
-
-    // Calculate bar and gap sizes to fill exactly the canvas width
-    // We want: numBars * barWidth + (numBars - 1) * gap = canvasWidth
-    // With gap = barWidth * 0.4, we get:
-    // numBars * barWidth + (numBars - 1) * 0.4 * barWidth = canvasWidth
-    // barWidth * (numBars + 0.4 * numBars - 0.4) = canvasWidth
-    // barWidth = canvasWidth / (1.4 * numBars - 0.4)
-    const totalWidth = canvas.width;
-    const barWidth = totalWidth / (1.4 * numBars - 0.4);
+    // Bar sizing: bars fill full width with 40% gap ratio
+    const barWidth = canvas.width / (1.4 * numBars - 0.4);
     const gap = barWidth * 0.4;
     const centerY = canvas.height / 2;
 
     ctx.fillStyle = props.color;
 
     for (let i = 0; i < numBars; i++) {
-      const amplitude = samples[i];
-      // Scale amplitude for visibility (boost quiet sounds)
-      const scaledAmplitude = Math.min(1, amplitude * 3);
+      const scaledAmplitude = Math.min(1, samples[i] * 3); // Boost for visibility
       const barHeight = Math.max(2, scaledAmplitude * canvas.height * 0.9);
-
       const x = i * (barWidth + gap);
       const y = centerY - barHeight / 2;
 
       ctx.beginPath();
-      ctx.roundRect(x, y, barWidth, barHeight, 1);
+      // roundRect fallback for older browsers (though Electron 38+ supports it)
+      if (ctx.roundRect) {
+        ctx.roundRect(x, y, barWidth, barHeight, 1);
+      } else {
+        ctx.rect(x, y, barWidth, barHeight);
+      }
       ctx.fill();
     }
 
     animationFrameRef.current = requestAnimationFrame(draw);
   }, [props.color]);
 
-  // Start/stop animation loop
+  // Run animation loop
   useEffect(() => {
     animationFrameRef.current = requestAnimationFrame(draw);
-    return () => {
-      if (animationFrameRef.current) {
-        cancelAnimationFrame(animationFrameRef.current);
-      }
-    };
+    return () => cancelAnimationFrame(animationFrameRef.current);
   }, [draw]);
 
   return (
@@ -241,12 +232,3 @@ const SlidingWaveform: React.FC<SlidingWaveformProps> = (props) => {
     </div>
   );
 };
-
-/**
- * Simple pulsing animation for transcribing state
- */
-const TranscribingAnimation: React.FC = () => (
-  <div className="flex items-center gap-2 text-amber-500">
-    <Loader2 className="h-5 w-5 animate-spin" />
-  </div>
-);
diff --git a/src/browser/components/ChatInput/VoiceInputButton.tsx b/src/browser/components/ChatInput/VoiceInputButton.tsx
@@ -21,38 +21,40 @@ interface VoiceInputButtonProps {
   mode: UIMode;
 }
 
-function getRecordingColorClass(mode: UIMode): string {
-  return mode === "plan"
-    ? "text-plan-mode-light animate-pulse"
-    : "text-exec-mode-light animate-pulse";
-}
+/** Color classes for each voice input state */
+const STATE_COLORS: Record<VoiceInputState, string> = {
+  idle: "text-muted/50 hover:text-muted",
+  recording: "", // Set dynamically based on mode
+  transcribing: "text-amber-500",
+};
 
-const STATE_CONFIG: Record<VoiceInputState, { label: string; colorClass: string }> = {
-  idle: { label: "Voice input", colorClass: "text-muted/50 hover:text-muted" },
-  recording: { label: "Stop recording", colorClass: "" }, // handled dynamically
-  transcribing: { label: "Transcribing...", colorClass: "text-amber-500" },
+const RECORDING_COLORS: Record<UIMode, string> = {
+  plan: "text-plan-mode-light animate-pulse",
+  exec: "text-exec-mode-light animate-pulse",
 };
 
+function getColorClass(state: VoiceInputState, mode: UIMode): string {
+  return state === "recording" ? RECORDING_COLORS[mode] : STATE_COLORS[state];
+}
+
 export const VoiceInputButton: React.FC<VoiceInputButtonProps> = (props) => {
   if (!props.shouldShowUI) return null;
 
   const needsHttps = props.requiresSecureContext;
   const needsApiKey = !needsHttps && !props.isApiKeySet;
-  const isDisabledReason = needsHttps || needsApiKey;
+  const isDisabled = needsHttps || needsApiKey;
+
+  const label = isDisabled
+    ? needsHttps
+      ? "Voice input (requires HTTPS)"
+      : "Voice input (requires OpenAI API key)"
+    : props.state === "recording"
+      ? "Stop recording"
+      : props.state === "transcribing"
+        ? "Transcribing..."
+        : "Voice input";
 
-  const stateConfig = STATE_CONFIG[props.state];
-  const { label, colorClass } = isDisabledReason
-    ? {
-        label: needsHttps
-          ? "Voice input (requires HTTPS)"
-          : "Voice input (requires OpenAI API key)",
-        colorClass: "text-muted/50",
-      }
-    : {
-        label: stateConfig.label,
-        colorClass:
-          props.state === "recording" ? getRecordingColorClass(props.mode) : stateConfig.colorClass,
-      };
+  const colorClass = isDisabled ? "text-muted/50" : getColorClass(props.state, props.mode);
 
   const Icon = props.state === "transcribing" ? Loader2 : Mic;
   const isTranscribing = props.state === "transcribing";
@@ -62,7 +64,7 @@ export const VoiceInputButton: React.FC<VoiceInputButtonProps> = (props) => {
       <button
         type="button"
         onClick={props.onToggle}
-        disabled={(props.disabled ?? false) || isTranscribing || isDisabledReason}
+        disabled={(props.disabled ?? false) || isTranscribing || isDisabled}
         aria-label={label}
         aria-pressed={props.state === "recording"}
         className={cn(
