Task Done#525
Merged
Merged
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThis PR adds smooth fade-in/fade-out animations during light/dark theme switches. A new 300ms transition duration is established in the context layer, global CSS rules trigger animations on theme-relevant properties, the ThemeContext temporarily applies a ChangesTheme Transition Animation
Sequence DiagramsequenceDiagram
participant User
participant Navbar
participant ThemeContext
participant DOM as document.documentElement
participant CSS as CSS Transitions
User->>Navbar: Click theme toggle button
Navbar->>Navbar: Set isThemeToggling=true
Navbar->>ThemeContext: Call toggleTheme()
ThemeContext->>DOM: Add theme-transitioning class
ThemeContext->>DOM: Update dark class
ThemeContext->>DOM: Persist to localStorage
CSS->>CSS: Transition background, color, border, shadow
Navbar->>Navbar: Icon rotates and scales during transition
ThemeContext->>DOM: Remove theme-transitioning class after 300ms
Navbar->>Navbar: Set isThemeToggling=false after 300ms
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/App.css (1)
1-13: ⚡ Quick winRespect reduced-motion preferences for global theme animation.
Line 6 applies transitions globally, but there’s no reduced-motion fallback. Add a
prefers-reduced-motion: reduceoverride to disable/shorten these transitions.♿ Proposed fix
: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; } + +@media (prefers-reduced-motion: reduce) { + html.theme-transitioning, + html.theme-transitioning * { + transition: none !important; + } +}🤖 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/App.css` around lines 1 - 13, The global theme-transition CSS applies transitions via :root and html.theme-transitioning but does not respect users' reduced-motion preference; add a media query for prefers-reduced-motion: reduce that overrides --transition-duration to 0ms (or removes transitions) and/or disables transitions on html.theme-transitioning and html.theme-transitioning * so animations are suppressed for users who prefer reduced motion.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/components/Navbar.tsx`:
- 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.
- Around line 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.
---
Nitpick comments:
In `@src/App.css`:
- Around line 1-13: The global theme-transition CSS applies transitions via
:root and html.theme-transitioning but does not respect users' reduced-motion
preference; add a media query for prefers-reduced-motion: reduce that overrides
--transition-duration to 0ms (or removes transitions) and/or disables
transitions on html.theme-transitioning and html.theme-transitioning * so
animations are suppressed for users who prefer reduced motion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58a8bac2-7c45-4bd8-9c0e-abc2932f0b0f
📒 Files selected for processing (3)
src/App.csssrc/components/Navbar.tsxsrc/context/ThemeContext.tsx
| 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"; |
Contributor
There was a problem hiding this comment.
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.
Comment on lines
+17
to
+25
| const handleThemeToggle = () => { | ||
| setIsThemeToggling(true); | ||
| toggleTheme(); | ||
|
|
||
| // Reset animation state after transition | ||
| setTimeout(() => { | ||
| setIsThemeToggling(false); | ||
| }, 300); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 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.tsxRepository: 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" || trueRepository: 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.
|
🎉🎉 Thank you for your contribution! Your PR #525 has been merged! 🎉🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Description
Implemented smooth theme transition animations across the UI to improve visual polish and enhance the user experience when toggling between light and dark modes.
Changes Made
Animation Details
Transition duration set to:
The animation provides a smooth yet snappy feel when switching themes
User Experience Improvements
How Has This Been Tested?
Theme Toggle Test
UI Consistency Test
Screenshots (if applicable)
Added screen recordings demonstrating:
theme_toggle.mp4
Type of Change
Summary by CodeRabbit
Release Notes