From b4c16d1f56f4e19fffd3d7608b410570f35045f9 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Sat, 24 May 2025 00:44:17 -0700 Subject: Code review comment fixes and some refactors. (#525) No intentional different behavior aside for tweaks suggested from the code review of #506 Refactor: Extract console message logic to custom hook This commit refactors the console message handling from App.tsx into a new custom hook useConsoleMessages. This change improves the testability of the console message logic and declutters the main App component. Created useConsoleMessages.ts to encapsulate console message state and update logic. Updated App.tsx to utilize the new useConsoleMessages hook. Added unit tests for useConsoleMessages.ts to ensure its functionality. I deleted and started over on LoadingIndicator.test.tsx as I spent way too much time trying to fix it before just regenerating the tests as the code was easier to write tests for from scratch and the existing tests were not that good (I added them in the previous pull request). --- packages/cli/src/ui/hooks/useTimer.ts | 65 +++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 packages/cli/src/ui/hooks/useTimer.ts (limited to 'packages/cli/src/ui/hooks/useTimer.ts') diff --git a/packages/cli/src/ui/hooks/useTimer.ts b/packages/cli/src/ui/hooks/useTimer.ts new file mode 100644 index 00000000..53c266dd --- /dev/null +++ b/packages/cli/src/ui/hooks/useTimer.ts @@ -0,0 +1,65 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useState, useEffect, useRef } from 'react'; + +/** + * Custom hook to manage a timer that increments every second. + * @param isActive Whether the timer should be running. + * @param resetKey A key that, when changed, will reset the timer to 0 and restart the interval. + * @returns The elapsed time in seconds. + */ +export const useTimer = (isActive: boolean, resetKey: unknown) => { + const [elapsedTime, setElapsedTime] = useState(0); + const timerRef = useRef(null); + const prevResetKeyRef = useRef(resetKey); + const prevIsActiveRef = useRef(isActive); + + useEffect(() => { + let shouldResetTime = false; + + if (prevResetKeyRef.current !== resetKey) { + shouldResetTime = true; + prevResetKeyRef.current = resetKey; + } + + if (prevIsActiveRef.current === false && isActive) { + // Transitioned from inactive to active + shouldResetTime = true; + } + + if (shouldResetTime) { + setElapsedTime(0); + } + prevIsActiveRef.current = isActive; + + // Manage interval + if (isActive) { + // Clear previous interval unconditionally before starting a new one + // This handles resetKey changes while active, ensuring a fresh interval start. + if (timerRef.current) { + clearInterval(timerRef.current); + } + timerRef.current = setInterval(() => { + setElapsedTime((prev) => prev + 1); + }, 1000); + } else { + if (timerRef.current) { + clearInterval(timerRef.current); + timerRef.current = null; + } + } + + return () => { + if (timerRef.current) { + clearInterval(timerRef.current); + timerRef.current = null; + } + }; + }, [isActive, resetKey]); + + return elapsedTime; +}; -- cgit v1.2.3