feat: Add dark mode support and fix GitHub repository links#41
feat: Add dark mode support and fix GitHub repository links#41rishabh3562 merged 8 commits intorishabh3562:mainfrom
Conversation
Contributor License Agreement (CLA)Thank you for your contribution to ToolBox. By submitting this pull request, you automatically agree to our Contributor License Agreement (CLA). Key Terms
Full AgreementPlease review the complete CLA for all terms and conditions. QuestionsIf you have questions about the CLA, please comment on this pull request or open a discussion. By continuing with this pull request, you confirm your acceptance of the CLA. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughWraps the app with a next-themes provider, adds a ThemeToggle client component and header integration, standardizes some CSS token uses to HSL and removes explicit light/dark custom presets, updates GitHub links to the specific repo, removes two Upstash deps, and adds an "Install dependencies" CI step. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Providers as ThemeProvider
participant ThemeToggle
participant Storage as localStorage
rect rgb(235,245,255)
User->>Browser: Visit app
Browser->>ThemeProvider: Initialize (attribute="class", default=system)
ThemeProvider->>Storage: read "devtools-theme" or detect OS
Storage-->>ThemeProvider: theme value
ThemeProvider->>Browser: apply theme class on <html>
end
rect rgb(255,250,235)
User->>ThemeToggle: Click toggle
ThemeToggle->>ThemeProvider: setTheme("dark"/"light")
ThemeProvider->>Storage: persist "devtools-theme"
ThemeProvider->>Browser: update <html> class -> UI reflects theme
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (1)
Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
components/theme-toggle.tsx (1)
7-29: Consider handling "system" theme in toggle logic.The toggle currently switches between "dark" and "light", but users could be on "system" theme (as configured in
layout.tsx). When a user on "system" theme clicks the toggle, consider which explicit theme to switch to based on the current resolved theme.You can use
resolvedThemefromuseTheme()to determine the actual theme being displayed:export function ThemeToggle() { const [mounted, setMounted] = React.useState(false); - const { theme, setTheme } = useTheme(); + const { resolvedTheme, setTheme } = useTheme(); React.useEffect(() => { setMounted(true); }, []); if (!mounted) { return null; } return ( <button - onClick={() => setTheme(theme === "dark" ? "light" : "dark")} + onClick={() => setTheme(resolvedTheme === "dark" ? "light" : "dark")} className="relative inline-flex h-10 w-10 items-center justify-center rounded-md border border-input bg-background hover:bg-accent hover:text-accent-foreground transition-colors" aria-label="Toggle theme" > <Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all dark:-rotate-90 dark:scale-0" /> <Moon className="absolute h-[1.2rem] w-[1.2rem] rotate-90 scale-0 transition-all dark:rotate-0 dark:scale-100" /> <span className="sr-only">Toggle theme</span> </button> ); }app/globals.css (1)
33-33: Optional: Remove trailing whitespace.There's a trailing whitespace on line 33 that could be removed for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
app/globals.css(3 hunks)app/layout.tsx(2 hunks)app/page.tsx(1 hunks)components/site-footer.tsx(2 hunks)components/site-header.tsx(2 hunks)components/theme-toggle.tsx(1 hunks)package.json(3 hunks)tsconfig.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/layout.tsx (1)
components/site-header.tsx (1)
SiteHeader(15-54)
components/site-header.tsx (1)
components/theme-toggle.tsx (1)
ThemeToggle(7-30)
🔇 Additional comments (7)
app/page.tsx (1)
78-78: LGTM! GitHub link correctly updated.The GitHub link now points to the specific repository, consistent with the changes throughout the PR.
components/site-footer.tsx (1)
16-16: LGTM! GitHub links properly updated.Both GitHub references now point to the specific repository, maintaining consistency across the footer.
Also applies to: 26-26
package.json (1)
51-52: Verify whether Upstash dependencies should be removed.The AI summary indicates that
@upstash/ratelimitand@upstash/rediswere removed, but these dependencies are still present in the package.json. Please clarify whether these should be removed or if the summary is incorrect.app/globals.css (1)
98-98: LGTM! Color format corrected to HSL.The changes from
rgb()tohsl()properly align with the HSL-based CSS custom properties defined in the:rootand.darkselectors.Also applies to: 110-110, 115-115
components/site-header.tsx (2)
8-8: LGTM! ThemeToggle properly integrated.The ThemeToggle component is correctly imported and rendered in the header's action area, providing easy access to theme switching.
Also applies to: 44-44
46-46: LGTM! GitHub link updated consistently.The GitHub link now points to the specific repository, matching the updates in other components.
app/layout.tsx (1)
6-6: LGTM! ThemeProvider integration is well-configured.The ThemeProvider setup follows best practices:
suppressHydrationWarningprevents hydration mismatches- Configuration properly enables system theme detection
- Storage key matches the PR objectives
disableTransitionOnChangeprevents visual glitches during theme switchesAlso applies to: 22-36
|
@rishabh3562 the CI/CD failures are due to the workflow using Node.js 18.20.8, but my PR updates Next.js to v16.0.0 which requires Node.js ≥20.9.0. Next.js 16 has many benefits, to convince you:
Fix:I'm updating the workflow in this PR (line 31 in .github/workflows/ci-cd-pipeline.yml): Code:
This will resolve all failing checks. The workflow changes will apply immediately when I push the commit. |
Reverted package versions to match main branch: - next: 16.0.0 → 13.5.1 - postcss: ^8.5.6 → 8.4.30 - react-syntax-highlighter: ^16.0.0 → ^15.5.0 - tsconfig.json: jsx setting updated to "preserve" for Next.js compatibility The workflow's Node.js version upgrade to 20.x is retained. This fixes the CI/CD pipeline error where `next lint` was failing due to version incompatibilities introduced by the Next.js 16 upgrade. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add "docs" to tsconfig.json exclude list to prevent TypeScript errors from Docusaurus files that don't have their dependencies installed - Sync tsconfig.json formatting with main branch - This fixes the CI/CD TypeScript check failure 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
I’ve pushed a fix to this PR to keep it consistent with the main branch: Reverted Next.js to 13.5.1 (was upgraded to 16) Reverted postcss and react-syntax-highlighter to match main Updated tsconfig.json (jsx: preserve) for Next.js 13 Kept the Node 20.x workflow upgrade While I agree that Next.js 16 is the way forward, 16.0.0 is not recommended yet, especially since several forks are still actively working on 13.5.1. This ensures stability and compatibility for everyone currently working on the project. The PR now contains only the dark mode and GitHub link updates, and the CI/CD pipeline should pass. |
- Add "npm ci" before building in deploy-preview job - This fixes "next: not found" error when vercel build falls back to npm run build - Dependencies are now installed before attempting to build the project The error occurred because: 1. Vercel build failed (VERCEL_TOKEN not configured) 2. Fallback to npm run build was triggered 3. But dependencies were never installed, causing "next: not found" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@CodeRabbit in deployment of this i can see dark mode but the toggle is not working , please check this pr and tell me if this is a logic issue or what is happening |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1294 🏁 Script executed: Length of output: 1123 🏁 Script executed: Length of output: 125 🏁 Script executed: Length of output: 2526 🏁 Script executed: Length of output: 3484 🏁 Script executed: Length of output: 42 @rishabh3562 I found the issue! There's a logic bug in the theme toggle component. Here's what's happening: The ProblemIn onClick={() => setTheme(theme === "dark" ? "light" : "dark")}When
The FixUpdate "use client";
import * as React from "react";
import { Moon, Sun } from "lucide-react";
import { useTheme } from "next-themes";
export function ThemeToggle() {
const [mounted, setMounted] = React.useState(false);
const { resolvedTheme, setTheme } = useTheme(); // Changed: use resolvedTheme
React.useEffect(() => {
setMounted(true);
}, []);
if (!mounted) {
return null;
}
return (
<button
onClick={() => setTheme(resolvedTheme === "dark" ? "light" : "dark")} // Changed: use resolvedTheme
className="relative inline-flex h-10 w-10 items-center justify-center rounded-md border border-input bg-background hover:bg-accent hover:text-accent-foreground transition-colors"
aria-label="Toggle theme"
>
<Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all dark:-rotate-90 dark:scale-0" />
<Moon className="absolute h-[1.2rem] w-[1.2rem] rotate-90 scale-0 transition-all dark:rotate-0 dark:scale-100" />
<span className="sr-only">Toggle theme</span>
</button>
);
}What changed:
Additional Deployment ChecksIf the fix doesn't work immediately in deployment:
Let me know once you apply this fix and I can verify the solution! 🐰 |
@aryadharmadhikari please fix this , i am requesting changes |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/theme-toggle.tsx (1)
7-30: Consider adding a three-way toggle cycle (optional enhancement).The current implementation toggles between "dark" and "light" only. A common UX pattern is to cycle through three states: light → dark → system, allowing users to return to system preference after a manual override.
Example implementation:
return ( <button - onClick={() => setTheme(resolvedTheme === "dark" ? "light" : "dark")} + onClick={() => { + const { theme } = useTheme(); + if (theme === "light") setTheme("dark"); + else if (theme === "dark") setTheme("system"); + else setTheme("light"); + }} className="relative inline-flex h-10 w-10 items-center justify-center rounded-md border border-input bg-background hover:bg-accent hover:text-accent-foreground transition-colors" aria-label="Toggle theme" >Note: You would need to destructure
themefromuseTheme()in addition toresolvedThemeto check the current theme setting (which can be "system"), and update the icon display logic to show a third icon (e.g., a Monitor/Laptop icon) whentheme === "system".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci-cd-pipeline.yml(1 hunks)components/theme-toggle.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-cd-pipeline.yml
🔇 Additional comments (1)
components/theme-toggle.tsx (1)
9-9: LGTM! The resolvedTheme fix correctly addresses the toggle bug.Using
resolvedThemeinstead ofthemeensures the toggle works correctly when the theme is set to "system". TheresolvedThemeproperty always returns the actual applied theme ("dark" or "light"), whilethemecan be "system", which would have caused the toggle logic to fail.Also applies to: 21-21
aryadharmadhikari
had a problem deploying
to
preview
GitHub Actions
Failure
|
@CodeRabbit what is the issue , the toggle is still not working, pin point the error so that i can fix it , secondly everything else looks good to me , whats your take (yes or no) |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3631 @rishabh3562 Good news! The code is actually correct now - the fix has been applied properly. The issue is deployment/caching, not the code itself. 🎯 Pinpointed Issue: Deployment CacheThe theme-toggle.tsx correctly uses The deployed build is using cached/old JavaScript files. 🔧 How to Fix (Choose One):Option 1: Force Vercel Rebuild (Recommended)# In your local terminal
git commit --allow-empty -m "trigger rebuild"
git pushOption 2: Clear Vercel Build Cache
Option 3: Manual Deployment Command# If you have Vercel CLI
vercel --prod --forceAfter redeploying, users should hard refresh their browsers ( ✅ My Take: YESEverything else in the PR is excellent: ✅ Dark mode implementation: Properly configured with next-themes, class-based, system preference support Recommendation: Approve and merge once the deployment cache is cleared. The code is production-ready. The toggle will work perfectly after a fresh build is deployed. 🐰 |
|
@rishabh3562 is attempting to deploy a commit to the rishabh's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rishabh3562
force-pushed
the
feat/dark-theme
branch
from
45ce0f0 to
c7cc4b2
Compare
…mpatibility - Extract ThemeProvider into a separate client component (components/providers.tsx) - Fix theme toggle functionality by ensuring ThemeProvider runs on client-side - Resolves hydration mismatch and theme state persistence issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Thanks for the PR and the detailed implementation! I noticed that the dark mode setup is causing hydration/rendering issues because ThemeProvider from next-themes is used directly in app/layout.tsx, which is a server component. To fix this, wrap ThemeProvider in a client component (with "use client";) and use that wrapper in layout.tsx. This ensures the theme logic only runs in the browser, preventing hydration mismatches and making the theme toggle functional. Once this is addressed, CI/CD and local testing should work correctly without SSR issues. |
@rishabh3562 I just checked the CI/CD pipeline PR opened and merged by you. It has been mentioned over there that the deployments will be skipped until the GitHub secrets are configured, and similar errors of it being not configured are visible during the Additionally the API routes(templates and health) are currently static export, this will further cause an issue. |
|
@aryadharmadhikari The repo secrets are already configured. GitHub doesn’t pass them to workflows triggered from forks, which is why the deploy preview initially failed. I’ve run the workflow manually, and the deployment works fine now. I’ll be merging this soon. Thanks for your patience! If you’d like to discuss further, feel free to continue the conversation on this PR or in the discussions channel.
|
rishabh3562
added
the
hacktoberfest-accepted
|
@rishabh3562 Thank you so much for your collaboration and for the time you invested in reviewing and implementing improvements to this PR! This being one of my early open source contributions, I have gained invaluable experience navigating real-world CI/CD issues, understanding Next.js architecture decisions, and learning how production projects are structured and maintained. Your guidance and patience throughout this process have been incredibly helpful. I really appreciate the opportunity to contribute to ToolBox, and I would like to continue contributing beyond Hacktoberfest whenever I can. Looking forward to collaborating on future improvements! |
|
Glad to hear this experience helped you understand how things work in real-world workflows; that’s exactly what open source should deliver. You’re welcome to continue contributing to ToolBox beyond Hacktoberfest |
2 participants
User description
Description
This PR implements comprehensive dark mode functionality for the ToolBox application with a theme toggle button in the navigation header. Additionally, it fixes GitHub repository links throughout the application to point to the official repository instead of the generic GitHub homepage.
Type of Change
Related Issue
Changes Made
devtools-themeThemeProviderwrapper inlayout.tsxfor global theme managementtheme-toggle.tsxcomponent with hydration-safe rendering@media (prefers-color-scheme: dark)queries inglobals.csshttps://github.comtohttps://github.com/rishabh3562/ToolBoxnext-themesfor theme management andlucide-reactfor iconsUpdated to address feedback:
Screenshots (if applicable)
Testing
Checklist
Additional Context
Technical Implementation
darkMode: ["class"]) with Tailwind CSSdark:variantsGitHub Links Fixed
Suggestions for Future Improvements
While implementing this PR, I noticed some UI/UX enhancements that could improve the navigation:
I would be happy to implement these improvements in a follow-up PR if you and the other maintainers agree with these suggestions.
PR Type
Enhancement, Bug fix
Description
Implement dark mode support with theme toggle button
ThemeProviderwrapper usingnext-themeslibraryThemeTogglecomponent with sun/moon iconsdevtools-themeFix GitHub repository links throughout application
https://github.com/rishabh3562/ToolBoxClean up CSS and improve dark mode compatibility
@media (prefers-color-scheme: dark)queriesUpdate dependencies and configuration
next-themesandlucide-reactpackagesDiagram Walkthrough
File Walkthrough
globals.css
Fix CSS color format and remove conflicting media queriesapp/globals.css
@media (prefers-color-scheme: dark)CSS rulesrgb(var(...))tohsl(var(...))in prose stylescompatibility
layout.tsx
Add ThemeProvider wrapper for dark mode supportapp/layout.tsx
ThemeProviderfromnext-themessuppressHydrationWarningto html elementlocalStorage persistence
devtools-themesite-header.tsx
Add theme toggle and fix GitHub link in headercomponents/site-header.tsx
ThemeTogglecomponent import and integrationhttps://github.comtohttps://github.com/rishabh3562/ToolBoxtheme-toggle.tsx
Create ThemeToggle component with icon animationscomponents/theme-toggle.tsx
page.tsx
Fix GitHub repository link in home pageapp/page.tsx
https://github.comtohttps://github.com/rishabh3562/ToolBoxsite-footer.tsx
Fix GitHub repository links in footercomponents/site-footer.tsx
https://github.comtohttps://github.com/rishabh3562/ToolBoxpackage.json
Update dependencies and add Upstash packagespackage.json
@upstash/ratelimitand@upstash/redisdependenciestsconfig.json
Update TypeScript configuration for Next.js 16tsconfig.json
.next/dev/types/**/*.tsto include pathsSummary by CodeRabbit
New Features
Bug Fixes
Style
Chores