Skip to content

Task Done #525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mehul-m-prajapati merged 1 commit into from
May 29, 2026
+63 −9
Merged

Task Done #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/App.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
/* Global theme transition styles */
:root {
--transition-duration: 300ms;
}

html.theme-transitioning,
html.theme-transitioning * {
transition: background-color var(--transition-duration) ease-in-out,
color var(--transition-duration) ease-in-out,
border-color var(--transition-duration) ease-in-out,
box-shadow var(--transition-duration) ease-in-out !important;
}

#root {
max-width: 1280px;
margin: 0 auto;
Expand Down
47 changes: 38 additions & 9 deletions src/components/Navbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
import { NavLink, Link } from "react-router-dom";
import { useState, useContext } from "react";
import { useState, useContext, useRef } from "react";
import { ThemeContext } from "../context/ThemeContext";
import { Moon, Sun, Menu, X, Github } from "lucide-react";
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove unused Github import to satisfy lint checks.

Line 4 imports Github but it isn’t used, and ESLint already flags this as an error.

🧹 Proposed fix 
-import { Moon, Sun, Menu, X, Github } from "lucide-react";
+import { Moon, Sun, Menu, X } from "lucide-react";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { Moon, Sun, Menu, X, Github } from "lucide-react";
import { Moon, Sun, Menu, X } from "lucide-react";
🧰 Tools
🪛 ESLint

[error] 4-4: 'Github' is defined but never used.

(@typescript-eslint/no-unused-vars)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Navbar.tsx` at line 4, The import list in Navbar.tsx includes
an unused symbol "Github" which causes ESLint errors; remove "Github" from the
import statement (leaving the other used icons Moon, Sun, Menu, X) so the file
no longer imports an unused identifier and linting passes.


const Navbar: React.FC = () => {
const [isOpen, setIsOpen] = useState(false);
const [isThemeToggling, setIsThemeToggling] = useState(false);
const themeToggleRef = useRef<HTMLButtonElement>(null);

const themeContext = useContext(ThemeContext);

if (!themeContext) return null;

const { toggleTheme, mode } = themeContext;

const handleThemeToggle = () => {
setIsThemeToggling(true);
toggleTheme();

// Reset animation state after transition
setTimeout(() => {
setIsThemeToggling(false);
}, 300);
};
Comment on lines +17 to +25
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify timeout lifecycle handling in Navbar theme toggle
rg -n -C2 'handleThemeToggle|setTimeout|clearTimeout|isThemeToggling|useEffect' src/components/Navbar.tsx

Repository: GitMetricsLab/github_tracker

Length of output: 2086

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/components/Navbar.tsx"

# Show the whole component header area (imports + state/refs + handleThemeToggle)
sed -n '1,120p' "$FILE"

# Check for any other usage of themeToggleRef in the file
echo "---- themeToggleRef usages ----"
rg -n "themeToggleRef" "$FILE" || true

Repository: GitMetricsLab/github_tracker

Length of output: 4204

Stabilize theme-toggle timer lifecycle to avoid animation race conditions.

handleThemeToggle schedules setTimeout on every click without clearing prior timers, so rapid clicks can set isThemeToggling to false too early; a pending timer can also fire after unmount.

⏱️ Proposed fix 
-import { useState, useContext, useRef } from "react";
+import { useState, useContext, useRef, useEffect } from "react";
@@
-  const themeToggleRef = useRef<HTMLButtonElement>(null);
+  const themeToggleTimeoutRef = useRef<number | null>(null);
@@
   const handleThemeToggle = () => {
+    if (themeToggleTimeoutRef.current !== null) {
+      clearTimeout(themeToggleTimeoutRef.current);
+    }
     setIsThemeToggling(true);
     toggleTheme();
-    
-    // Reset animation state after transition
-    setTimeout(() => {
+
+    // Reset animation state after transition
+    themeToggleTimeoutRef.current = window.setTimeout(() => {
       setIsThemeToggling(false);
     }, 300);
   };
+
+  useEffect(() => {
+    return () => {
+      if (themeToggleTimeoutRef.current !== null) {
+        clearTimeout(themeToggleTimeoutRef.current);
+      }
+    };
+  }, []);
@@
-            ref={themeToggleRef}
             onClick={handleThemeToggle}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Navbar.tsx` around lines 17 - 25, handleThemeToggle currently
creates a new setTimeout on every click which can race and fire after unmount;
fix it by tracking the timeout id in a ref (e.g., themeToggleTimeoutRef), clear
any existing timeout with clearTimeout before creating a new one, assign the new
id to the ref, and also clear the timeout in a useEffect cleanup to prevent it
firing after unmount; keep using setIsThemeToggling and toggleTheme but ensure
the timeout lifecycle is managed via the ref and cleanup.


const navLinkStyles = ({ isActive }: { isActive: boolean }) =>
`px-4 py-2 rounded-xl text-sm lg:text-base font-semibold transition-all duration-300 ${
isActive
Expand Down Expand Up @@ -59,14 +71,23 @@ const Navbar: React.FC = () => {

{/* Theme Toggle */}
<button
onClick={toggleTheme}
className="ml-2 p-2 rounded-xl border border-gray-300 dark:border-gray-700 hover:bg-gray-100 dark:hover:bg-gray-800 transition-colors"
ref={themeToggleRef}
onClick={handleThemeToggle}
className="ml-2 p-2 rounded-xl border border-gray-300 dark:border-gray-700 hover:bg-gray-100 dark:hover:bg-gray-800 transition-all duration-300"
aria-label="Toggle Theme"
>
{mode === "dark" ? (
<Sun className="h-5 w-5 text-yellow-400" />
<Sun
className={`h-5 w-5 text-yellow-400 transition-all duration-300 ${
isThemeToggling ? 'rotate-180 scale-0' : 'rotate-0 scale-100'
}`}
/>
) : (
<Moon className="h-5 w-5 text-slate-700" />
<Moon
className={`h-5 w-5 text-slate-700 transition-all duration-300 ${
isThemeToggling ? 'rotate-180 scale-0' : 'rotate-0 scale-100'
}`}
/>
)}
</button>
</div>
Expand All @@ -76,14 +97,22 @@ const Navbar: React.FC = () => {

{/* Theme Toggle */}
<button
onClick={toggleTheme}
className="p-2 rounded-lg hover:bg-gray-100 dark:hover:bg-gray-800 transition-colors"
onClick={handleThemeToggle}
className="p-2 rounded-lg hover:bg-gray-100 dark:hover:bg-gray-800 transition-all duration-300"
aria-label="Toggle Theme"
>
{mode === "dark" ? (
<Sun className="h-5 w-5 text-yellow-400" />
<Sun
className={`h-5 w-5 text-yellow-400 transition-all duration-300 ${
isThemeToggling ? 'rotate-180 scale-0' : 'rotate-0 scale-100'
}`}
/>
) : (
<Moon className="h-5 w-5 text-white" />
<Moon
className={`h-5 w-5 text-white transition-all duration-300 ${
isThemeToggling ? 'rotate-180 scale-0' : 'rotate-0 scale-100'
}`}
/>
)}
</button>

Expand Down
12 changes: 12 additions & 0 deletions src/context/ThemeContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ interface ThemeContextType {
export const ThemeContext = createContext<ThemeContextType | null>(null);

const THEME_STORAGE_KEY = 'theme';
const TRANSITION_DURATION = 300; // milliseconds

const ThemeWrapper = ({ children }: { children: ReactNode }) => {
const [mode, setMode] = useState<'light' | 'dark'>(() => {
Expand All @@ -19,12 +20,23 @@ const ThemeWrapper = ({ children }: { children: ReactNode }) => {

// Sync mode with <html> class and localStorage
useEffect(() => {
// Add transition class
document.documentElement.classList.add('theme-transitioning');

// Apply theme change
if (mode === 'dark') {
document.documentElement.classList.add('dark');
} else {
document.documentElement.classList.remove('dark');
}
localStorage.setItem(THEME_STORAGE_KEY, mode);

// Remove transition class after animation completes
const timer = setTimeout(() => {
document.documentElement.classList.remove('theme-transitioning');
}, TRANSITION_DURATION);

return () => clearTimeout(timer);
}, [mode]);

const toggleTheme = () => {
Expand Down
Loading