Added Fully Functional Dark Mode Toggle & Universal Dark Theme Support#37
Added Fully Functional Dark Mode Toggle & Universal Dark Theme Support#37Naman1313 wants to merge 2 commits intomonu808:mainfrom
Conversation
|
@Naman1313 is attempting to deploy a commit to the Narendra Singh's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR implements dark theme support by updating the Supabase dependency, adding Tailwind CSS with comprehensive light/dark CSS variables to globals.css, creating a ThemeToggle component that persists user preference to localStorage, injecting a theme-detection script in the layout, and integrating the toggle into Header and TouristHeader components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
monu808
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/app/globals.css`:
- Around line 410-413: The global rule selector html.dark svg { color: `#f1f5f9`
!important; stroke: `#f1f5f9` !important; } force-overrides component-level
Tailwind color utilities and multi-color icons; remove the !important and
restrict the rule to only unstyled icons (e.g., html.dark svg.icon-uncolored, or
html.dark svg:not([class*="text-"]):not([fill]):not([stroke])) so icons that
declare colors via classes or attributes keep their colors, or alternatively
drop the rule entirely and apply .icon-uncolored on components that need the
dark-mode tint.
- Around line 289-299: The OS-level dark-mode media query is currently
unconditional and can override the user's explicit toggle; change the media
query that sets --background/--foreground (the `@media` (prefers-color-scheme:
dark) block) so it only applies when the page does not have the .dark class
(e.g., scope it to html:not(.dark) or similar) so that html.dark and your toggle
always take precedence over system defaults for the --background/--foreground
variables.
In `@src/app/layout.tsx`:
- Around line 46-62: Replace the inline script using dangerouslySetInnerHTML in
layout.tsx with a Next.js Script component that runs before first paint (use
strategy "beforeInteractive"); move the theme initialization into that Script
and implement a safe fallback to system preference with
window.matchMedia('(prefers-color-scheme: dark)') if no saved localStorage
'theme' exists, and avoid swallowing errors—catch and log or silently ignore but
do not use dangerouslySetInnerHTML. Ensure you reference the Script import from
'next/script' and place the Script component before rendering {children} so the
document.documentElement.classList is set (add/remove 'dark') before first
paint.
In `@src/components/ThemeToggle.tsx`:
- Around line 1-29: ThemeToggle currently lacks proper button semantics and
doesn't keep its UI state in sync with the active theme; update the ThemeToggle
component to (1) add type="button" and an appropriate aria-label on the button,
(2) track theme state with useState (e.g., isDark) and initialize it in
useEffect from localStorage/document.documentElement.classList, (3) set
aria-pressed based on that state and render different icons/text based on
isDark, and (4) update both document.documentElement.classList and localStorage
inside the existing toggle function while also updating the component state so
the UI stays in sync.
In `@src/components/TouristHeader.tsx`:
- Line 8: The ThemeToggle button lacks an accessible name; update the
ThemeToggle component (the button element inside ThemeToggle) to include an
aria-label such as "Toggle dark mode" or "Toggle theme" while keeping the
existing onClick handler (toggle) and classes intact so screen readers announce
the button's purpose.
🧹 Nitpick comments (2)
postcss.config.mjs (1)
6-15: Drop the commented-out PostCSS config (or replace with a short explanatory comment).
Having an entire alternate config block commented out tends to rot and mislead during future CSS/build debugging.src/app/globals.css (1)
7-14: Dark-mode source of truth is split (--background/...vs--bg/...) andbodyis defined multiple times.Right now
bodyis styled withvar(--background)(Line 46-47) but later overridden byvar(--bg)(Line 270-273), plus a secondbodytransition block (Line 281-283). This will make theme bugs hard to reason about and can cause partial overrides. Consider consolidating to one variable set + onebodydefinition.Proposed consolidation (keep existing `--background/--foreground`, map the newer vars to them)
/* Light mode variables (default) */ :root { - --bg: `#ffffff`; - --text: `#111111`; - --card: `#f4f4f4`; - --border: `#e2e2e2`; + --bg: var(--background); + --text: var(--foreground); + /* optionally introduce these into the existing system instead of parallel vars */ + --card: var(--surface-elevated); + --border: `#e2e8f0`; /* or a named existing token */ } /* Dark mode variables */ .dark { - --bg: `#0d0d0d`; - --text: `#f5f5f5`; - --card: `#1a1a1a`; - --border: `#333333`; + --bg: var(--background); + --text: var(--foreground); + --card: var(--surface-elevated); + --border: `#334155`; } /* Global application */ body { - background-color: var(--bg); - color: var(--text); - transition: background-color 0.3s ease, color 0.3s ease; + background: var(--background); + color: var(--foreground); + transition: background-color 0.3s ease, color 0.3s ease, border-color 0.3s ease; } - -html.dark { - background-color: `#0f172a`; /* slate-900 */ - color: `#f1f5f9`; /* slate-100 */ -} - -body { - transition: background 0.3s, color 0.3s; -}Also applies to: 253-283
📜 Review details
Configuration used: Organization 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 (7)
package.jsonpostcss.config.mjssrc/app/globals.csssrc/app/layout.tsxsrc/components/Header.tsxsrc/components/ThemeToggle.tsxsrc/components/TouristHeader.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Next.js with TypeScript, Tailwind CSS, ESLint, App Router, and src directory structure for project scaffolding
Files:
src/app/layout.tsxsrc/components/ThemeToggle.tsxpackage.jsonsrc/components/Header.tsxsrc/components/TouristHeader.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure all TypeScript errors are resolved and the project compiles successfully with Next.js build
Files:
src/app/layout.tsxsrc/components/ThemeToggle.tsxsrc/components/Header.tsxsrc/components/TouristHeader.tsx
🧬 Code graph analysis (2)
src/components/Header.tsx (1)
src/components/ThemeToggle.tsx (1)
ThemeToggle(3-29)
src/components/TouristHeader.tsx (1)
src/components/ThemeToggle.tsx (1)
ThemeToggle(3-29)
🪛 ast-grep (0.40.5)
src/app/layout.tsx
[warning] 46-46: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/app/layout.tsx
[error] 47-47: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (2)
src/components/Header.tsx (1)
8-8: Theme toggle wiring looks fine; verify Header visuals actually respond to dark mode.
Right now the Header container usesbg-white border-gray-200 ..., so unlesssrc/app/globals.cssis explicitly overriding those, the toggle may flip the app but leave the header “light”.Optional direction (CSS variable-based colors)
- <header className="bg-white border-b border-gray-200 h-16 fixed top-0 right-0 left-64 z-30 shadow-sm"> + <header className="h-16 fixed top-0 right-0 left-64 z-30 shadow-sm border-b" + style={{ background: "var(--background)", borderColor: "var(--border)" }} + >Also applies to: 47-51
package.json (1)
20-20: Lockfile is properly updated with the@supabase/supabase-jsbump to^2.90.1.
All helper and adapter packages (@supabase/auth-helpers-nextjs,@supabase/ssr,@next-auth/supabase-adapter, etc.) are specified in dependencies with compatible versions. The transitive dependency versions in the lockfile reflect npm's normal resolution across different package requirements and do not indicate incompatibility. If auth functionality works as expected, no further action needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /* Base */ | ||
| html.dark { | ||
| background-color: #0f172a; /* slate-900 */ | ||
| color-scheme: dark; | ||
| } | ||
|
|
||
| html.dark body { | ||
| background-color: #0f172a; | ||
| color: #f1f5f9; /* slate-100 */ | ||
| } | ||
|
|
There was a problem hiding this comment.
prefers-color-scheme can override user-selected light mode unless scoped.
If the user picks “light” in the toggle, the @media (prefers-color-scheme: dark) { :root { ... } } block (Line 28-35) can still flip --background/--foreground on systems set to dark, because it’s independent of html.dark. Consider scoping the media query to only apply when the user hasn’t explicitly enabled .dark.
One way to scope OS-dark defaults only when `.dark` isn’t set
-@media (prefers-color-scheme: dark) {
- :root {
+@media (prefers-color-scheme: dark) {
+ html:not(.dark) {
--background: `#0f172a`;
--foreground: `#f1f5f9`;
--surface: `#1e293b`;
--surface-elevated: `#334155`;
}
}🤖 Prompt for AI Agents
In `@src/app/globals.css` around lines 289 - 299, The OS-level dark-mode media
query is currently unconditional and can override the user's explicit toggle;
change the media query that sets --background/--foreground (the `@media`
(prefers-color-scheme: dark) block) so it only applies when the page does not
have the .dark class (e.g., scope it to html:not(.dark) or similar) so that
html.dark and your toggle always take precedence over system defaults for the
--background/--foreground variables.
| html.dark svg { | ||
| color: #f1f5f9 !important; | ||
| stroke: #f1f5f9 !important; | ||
| } |
There was a problem hiding this comment.
Global html.dark svg { ... !important } will break intentional icon colors.
This will override Tailwind color utilities (e.g., text-green-600) and multi-color icons across the app, including icons used for branding/semantic meaning. Prefer letting component-level classes win, or scope this rule to only icons that don’t declare a color.
Safer alternative: drop `!important` and scope to unstyled icons
-html.dark svg {
- color: `#f1f5f9` !important;
- stroke: `#f1f5f9` !important;
-}
+html.dark svg:not([class*="text-"]) {
+ color: `#f1f5f9`;
+ stroke: currentColor;
+}📝 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.
| html.dark svg { | |
| color: #f1f5f9 !important; | |
| stroke: #f1f5f9 !important; | |
| } | |
| html.dark svg:not([class*="text-"]) { | |
| color: `#f1f5f9`; | |
| stroke: currentColor; | |
| } |
🤖 Prompt for AI Agents
In `@src/app/globals.css` around lines 410 - 413, The global rule selector
html.dark svg { color: `#f1f5f9` !important; stroke: `#f1f5f9` !important; }
force-overrides component-level Tailwind color utilities and multi-color icons;
remove the !important and restrict the rule to only unstyled icons (e.g.,
html.dark svg.icon-uncolored, or html.dark
svg:not([class*="text-"]):not([fill]):not([stroke])) so icons that declare
colors via classes or attributes keep their colors, or alternatively drop the
rule entirely and apply .icon-uncolored on components that need the dark-mode
tint.
| <script | ||
| dangerouslySetInnerHTML={{ | ||
| __html: ` | ||
| try { | ||
| const saved = localStorage.getItem('theme'); | ||
| const html = document.documentElement; | ||
|
|
||
| if (saved === 'dark') { | ||
| html.classList.add('dark'); | ||
| } else if (saved === 'light') { | ||
| html.classList.remove('dark'); | ||
| } | ||
| } catch (e) {} | ||
| `, | ||
| }} | ||
| /> | ||
|
|
There was a problem hiding this comment.
Avoid dangerouslySetInnerHTML here (CI/security lint) and run theme init before first paint.
Biome is flagging this as a security error; also placing the script after {children} risks a visible “flash” before the dark class is applied.
Proposed patch (use next/script + early init + system fallback)
import type { Metadata } from "next";
import { Geist, Geist_Mono, Playfair_Display, Inter } from "next/font/google";
import "./globals.css";
import { AuthProvider } from "@/contexts/AuthContext";
+import Script from "next/script";
@@
export default function RootLayout({
children,
}: Readonly<{
children: React.ReactNode;
}>) {
return (
- <html lang="en">
+ <html lang="en" suppressHydrationWarning>
<body
className={`${geistSans.variable} ${geistMono.variable} ${playfairDisplay.variable} ${inter.variable} antialiased`}
>
+ <Script id="theme-init" strategy="beforeInteractive">
+ {`
+ try {
+ const saved = localStorage.getItem('theme');
+ const html = document.documentElement;
+ const prefersDark =
+ window.matchMedia &&
+ window.matchMedia('(prefers-color-scheme: dark)').matches;
+
+ const useDark = saved === 'dark' || (!saved && prefersDark);
+ html.classList.toggle('dark', useDark);
+ html.style.colorScheme = useDark ? 'dark' : 'light';
+ } catch (e) {}
+ `}
+ </Script>
<AuthProvider>
{children}
</AuthProvider>
- <script
- dangerouslySetInnerHTML={{
- __html: `
- try {
- const saved = localStorage.getItem('theme');
- const html = document.documentElement;
-
- if (saved === 'dark') {
- html.classList.add('dark');
- } else if (saved === 'light') {
- html.classList.remove('dark');
- }
- } catch (e) {}
- `,
- }}
- />
-
</body>
</html>
);
}🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 46-46: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
[error] 47-47: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
In `@src/app/layout.tsx` around lines 46 - 62, Replace the inline script using
dangerouslySetInnerHTML in layout.tsx with a Next.js Script component that runs
before first paint (use strategy "beforeInteractive"); move the theme
initialization into that Script and implement a safe fallback to system
preference with window.matchMedia('(prefers-color-scheme: dark)') if no saved
localStorage 'theme' exists, and avoid swallowing errors—catch and log or
silently ignore but do not use dangerouslySetInnerHTML. Ensure you reference the
Script import from 'next/script' and place the Script component before rendering
{children} so the document.documentElement.classList is set (add/remove 'dark')
before first paint.
| "use client"; | ||
|
|
||
| export default function ThemeToggle() { | ||
| const toggle = () => { | ||
| const html = document.documentElement; | ||
|
|
||
| if (html.classList.contains("dark")) { | ||
| html.classList.remove("dark"); | ||
| localStorage.setItem("theme", "light"); | ||
| } else { | ||
| html.classList.add("dark"); | ||
| localStorage.setItem("theme", "dark"); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <button | ||
| onClick={toggle} | ||
| className="px-3 py-2 rounded-md border" | ||
| style={{ | ||
| background: "var(--card)", | ||
| color: "var(--text)", | ||
| borderColor: "var(--border)", | ||
| }} | ||
| > | ||
| 🌙 / ☀️ | ||
| </button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add button semantics + a11y state + keep UI in sync with the active theme.
At minimum: type="button", aria-label, and aria-pressed (and ideally render an icon/state based on current theme).
Proposed patch
"use client";
+import { useEffect, useState } from "react";
+
export default function ThemeToggle() {
+ const [isDark, setIsDark] = useState(false);
+
+ useEffect(() => {
+ setIsDark(document.documentElement.classList.contains("dark"));
+ }, []);
+
const toggle = () => {
const html = document.documentElement;
-
- if (html.classList.contains("dark")) {
- html.classList.remove("dark");
- localStorage.setItem("theme", "light");
- } else {
- html.classList.add("dark");
- localStorage.setItem("theme", "dark");
- }
+ const nextIsDark = !html.classList.contains("dark");
+ html.classList.toggle("dark", nextIsDark);
+ localStorage.setItem("theme", nextIsDark ? "dark" : "light");
+ setIsDark(nextIsDark);
};
return (
<button
+ type="button"
onClick={toggle}
+ aria-label="Toggle dark mode"
+ aria-pressed={isDark}
className="px-3 py-2 rounded-md border"
style={{
background: "var(--card)",
color: "var(--text)",
borderColor: "var(--border)",
}}
>
- 🌙 / ☀️
+ {isDark ? "☀️" : "🌙"}
</button>
);
}🤖 Prompt for AI Agents
In `@src/components/ThemeToggle.tsx` around lines 1 - 29, ThemeToggle currently
lacks proper button semantics and doesn't keep its UI state in sync with the
active theme; update the ThemeToggle component to (1) add type="button" and an
appropriate aria-label on the button, (2) track theme state with useState (e.g.,
isDark) and initialize it in useEffect from
localStorage/document.documentElement.classList, (3) set aria-pressed based on
that state and render different icons/text based on isDark, and (4) update both
document.documentElement.classList and localStorage inside the existing toggle
function while also updating the component state so the UI stays in sync.
| import { useAuth } from '@/contexts/AuthContext'; | ||
| import Link from 'next/link'; | ||
| import { useRouter } from 'next/navigation'; | ||
| import ThemeToggle from "./ThemeToggle"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking ThemeToggle.tsx existence and content ==="
if [ -f "src/components/ThemeToggle.tsx" ]; then
echo "File found. Reading content:"
cat -n src/components/ThemeToggle.tsx
else
echo "File not found at src/components/ThemeToggle.tsx"
find . -name "ThemeToggle.tsx" -type f 2>/dev/null || echo "ThemeToggle.tsx not found anywhere"
fi
echo ""
echo "=== Checking TouristHeader.tsx around lines 8 and 71 ==="
if [ -f "src/components/TouristHeader.tsx" ]; then
echo "TouristHeader.tsx content:"
cat -n src/components/TouristHeader.tsx
else
echo "File not found at src/components/TouristHeader.tsx"
fiRepository: monu808/GreenPass
Length of output: 11926
Add an accessible label to the ThemeToggle button.
The ThemeToggle component button uses only emoji text (🌙 / ☀️) without an accessible name. Add an aria-label attribute to ensure screen readers announce the button's purpose:
<button
onClick={toggle}
aria-label="Toggle dark mode"
className="px-3 py-2 rounded-md border"
// ...
>
🌙 / ☀️
</button>The 'use client' directive is already properly set.
🤖 Prompt for AI Agents
In `@src/components/TouristHeader.tsx` at line 8, The ThemeToggle button lacks an
accessible name; update the ThemeToggle component (the button element inside
ThemeToggle) to include an aria-label such as "Toggle dark mode" or "Toggle
theme" while keeping the existing onClick handler (toggle) and classes intact so
screen readers announce the button's purpose.
@Naman1313 can you address the following issues:
As you can see on the screenshots no dark mode appears prperly on dashboard and the homepage and tourist side dashboard has no option to switch it to dark mode. |
|
ok @monu808 let me check again |
2 participants
This PR implements a complete Dark Mode Toggle along with global dark theme support across the project.
✅ What’s Added / Fixed:-
🧩 Linked Issue:-
Closes #17
🧪 Testing:-
Request:-