feat: add system theme detection for automatic dark/light mode#14
Closed
madhavansingh wants to merge 2 commits intoAOSSIE-Org:mainfrom
Closed
feat: add system theme detection for automatic dark/light mode#14madhavansingh wants to merge 2 commits intoAOSSIE-Org:mainfrom
madhavansingh wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
github-actions
Bot
added
enhancement
WalkthroughThis PR implements automatic system theme detection for the SupportUsButton component by adding a "system" theme option to the Theme type, introducing resolvedTheme logic to detect user preferences, and refactoring theme-based styling throughout the component. Sponsor sorting by tier is also integrated. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User System
participant Component as SupportUsButton
participant DOM as Rendered Output
User->>Component: Provide theme prop<br/>(light|dark|system|...)
alt theme === "system"
User->>Component: System prefers-color-scheme<br/>(dark or light)
Component->>Component: Resolve to dark or light theme
else theme !== "system"
Component->>Component: Use provided theme directly
end
Component->>Component: Compute classAccordingToTheme<br/>(resolvedTheme)
Component->>DOM: Apply theme-aware styles<br/>(sponsors, hero, CTA, etc.)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Author
|
Hi maintainers, I have implemented system theme detection for automatic dark/light mode support. Please let me know if any improvements or modifications are needed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/SupportUsButton.tsx (2)
1-5:⚠️ Potential issue | 🟠 MajorMissing
"use client"directive for client-side features.This component uses
window.matchMedia(line 86), which is a browser-only API. Per coding guidelines for NextJS, components using client-side features must include the"use client"directive at the top of the file.🛠️ Proposed fix
+"use client"; + import React from "react"; import type { supportUsButtonProps } from "../types/index";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SupportUsButton.tsx` around lines 1 - 5, This file is missing the "use client" directive required because the SupportUsButton component calls the browser API window.matchMedia; add the exact string "use client" as the very first line of the file so NextJS treats SupportUsButton (and any hooks or window usage inside it) as a client component, ensuring window.matchMedia calls inside the component and any related functions run in the browser.
277-289: 🧹 Nitpick | 🔵 TrivialConsider externalizing user-visible strings for i18n.
Hardcoded strings like
"Our Sponsors"(line 283) and"ABOUT PROJECT:"(line 231) should be externalized to resource files per coding guidelines for internationalization support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SupportUsButton.tsx` around lines 277 - 289, The component SupportUsButton contains hardcoded user-visible strings (e.g., the h2 text "Our Sponsors" in the JSX and the "ABOUT PROJECT:" string elsewhere) which must be externalized for i18n; update SupportUsButton to import and use the project's localization helper (e.g., useTranslation or t) and replace those literal strings with translation keys (e.g., t('support.ourSponsors') and t('support.aboutProject')), then add corresponding entries to the translation resource files for each supported locale and ensure keys follow the existing naming convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/SupportUsButton.tsx`:
- Around line 152-161: Extract the repeated theme-based class patterns into a
small helper (e.g., a themeStyles object or getThemeClasses(resolvedTheme)
function) and replace the inline repeated templates in SupportUsButton.tsx that
reference resolvedTheme with references to that helper; specifically consolidate
the shadow mappings (AOSSIE, light, dark, minimal, corporate) and outline
mappings and use themeStyles.shadows[resolvedTheme] and
themeStyles.outlines[resolvedTheme] (or return both from getThemeClasses) in
place of the multiple `${resolvedTheme === "X" && "..."}` occurrences and in the
classAccordingToTheme usage to remove duplication and centralize theme-to-class
logic.
- Line 91: In SupportUsButton.tsx update the loose equality to strict equality:
in the JSX className expression that currently checks resolvedTheme == "light"
|| resolvedTheme == "dark", change to resolvedTheme === "light" || resolvedTheme
=== "dark" so it matches the rest of the file; this affects the conditional that
chooses between classAccordingToTheme(resolvedTheme) and the fallback "bg-black
text-white" alongside classNames.container.
- Line 86: The inline access to window.matchMedia causes SSR hydration
mismatches and doesn't update when system theme changes; replace the inline
resolution with a state + effect inside the SupportUsButton component: create a
useState for resolvedTheme with an initial value of (Theme === "system" ?
"light" : Theme), then add a useEffect that runs when Theme changes — if Theme
!== "system" set resolvedTheme to Theme, otherwise create a MediaQueryList with
window.matchMedia("(prefers-color-scheme: dark)"), set resolvedTheme based on
m.matches, attach a change listener (use addEventListener('change') if available
or addEventListener fallback to addListener) to update resolvedTheme on
preference changes, and remove the listener in the cleanup; reference the
symbols resolvedTheme, Theme and the SupportUsButton component when making these
edits.
---
Outside diff comments:
In `@src/components/SupportUsButton.tsx`:
- Around line 1-5: This file is missing the "use client" directive required
because the SupportUsButton component calls the browser API window.matchMedia;
add the exact string "use client" as the very first line of the file so NextJS
treats SupportUsButton (and any hooks or window usage inside it) as a client
component, ensuring window.matchMedia calls inside the component and any related
functions run in the browser.
- Around line 277-289: The component SupportUsButton contains hardcoded
user-visible strings (e.g., the h2 text "Our Sponsors" in the JSX and the "ABOUT
PROJECT:" string elsewhere) which must be externalized for i18n; update
SupportUsButton to import and use the project's localization helper (e.g.,
useTranslation or t) and replace those literal strings with translation keys
(e.g., t('support.ourSponsors') and t('support.aboutProject')), then add
corresponding entries to the translation resource files for each supported
locale and ensure keys follow the existing naming convention.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e0ef26d7-2afc-4196-b568-c0d524fd3b29
📒 Files selected for processing (2)
src/components/SupportUsButton.tsxsrc/types/index.ts
| return tierPriority[aTier] - tierPriority[bTier]; | ||
| }) : []; | ||
|
|
||
| const resolvedTheme = Theme === "system" ? (typeof window !== "undefined" && window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" : "light") : Theme; |
There was a problem hiding this comment.
SSR hydration mismatch and missing reactive updates for system theme.
Accessing window.matchMedia during render causes two issues:
- Hydration mismatch: Server renders with
"light"(window undefined), but client may resolve to"dark", causing React hydration errors. - No reactive updates: If the user changes their system preference, the component won't re-render.
🐛 Proposed fix using useState and useEffect
+import React, { useState, useEffect } from "react";
-import React from "react";Then replace the inline resolution with a hook pattern:
+ const [resolvedTheme, setResolvedTheme] = useState<Exclude<Theme, "system">>(() => {
+ if (Theme !== "system") return Theme;
+ if (typeof window === "undefined") return "light";
+ return window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" : "light";
+ });
+
+ useEffect(() => {
+ if (Theme !== "system") {
+ setResolvedTheme(Theme);
+ return;
+ }
+ const mediaQuery = window.matchMedia("(prefers-color-scheme: dark)");
+ const handleChange = (e: MediaQueryListEvent) => {
+ setResolvedTheme(e.matches ? "dark" : "light");
+ };
+ setResolvedTheme(mediaQuery.matches ? "dark" : "light");
+ mediaQuery.addEventListener("change", handleChange);
+ return () => mediaQuery.removeEventListener("change", handleChange);
+ }, [Theme]);
- const resolvedTheme = Theme === "system" ? (typeof window !== "undefined" && window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" : "light") : Theme;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SupportUsButton.tsx` at line 86, The inline access to
window.matchMedia causes SSR hydration mismatches and doesn't update when system
theme changes; replace the inline resolution with a state + effect inside the
SupportUsButton component: create a useState for resolvedTheme with an initial
value of (Theme === "system" ? "light" : Theme), then add a useEffect that runs
when Theme changes — if Theme !== "system" set resolvedTheme to Theme, otherwise
create a MediaQueryList with window.matchMedia("(prefers-color-scheme: dark)"),
set resolvedTheme based on m.matches, attach a change listener (use
addEventListener('change') if available or addEventListener fallback to
addListener) to update resolvedTheme on preference changes, and remove the
listener in the cleanup; reference the symbols resolvedTheme, Theme and the
SupportUsButton component when making these edits.
Author
There was a problem hiding this comment.
Thank you for the review suggestions. I have updated the implementation to address the SSR hydration issue and applied the recommended improvements.
There was a problem hiding this comment.
🧩 Analysis chain
Let me verify the latest state of the file to confirm the fix.
🏁 Script executed:
#!/bin/bash
# Fetch the latest content of SupportUsButton.tsx from the PR branch
gh pr view 14 --repo AOSSIE-Org/SupportUsButton --json headRefName,headRepositoryOwner,headRepository
Length of output: 228
🏁 Script executed:
#!/bin/bash
# Fetch the latest SupportUsButton.tsx from the PR branch
gh api repos/AOSSIE-Org/SupportUsButton/contents/src/components/SupportUsButton.tsx?ref=feat/system-theme-detection --jq '.content' | base64 -d
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Check the latest commits on the PR
gh pr view 14 --repo AOSSIE-Org/SupportUsButton --json commits --jq '.commits[-3:]'
Length of output: 685
🏁 Script executed:
#!/bin/bash
# Fetch file content using the latest commit OID
gh api repos/AOSSIE-Org/SupportUsButton/contents/src/components/SupportUsButton.tsx?ref=d01aa31da4ceda426abdeca7aebd809e2eda72ef --jq '.content' | base64 -d | grep -n "resolvedTheme\|useState\|useEffect\|matchMedia\|import React"
Length of output: 3704
@madhavansingh, thanks for the response! However, after checking the latest commit (d01aa31) on the PR branch, the original inline implementation is still present at Line 86 — useState/useEffect are not yet imported or used:
// Still present in the latest commit — unfixed
const resolvedTheme = Theme === "system" ? (typeof window !== "undefined" && window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" : "light") : Theme;It looks like the fix may not have been pushed yet. Could you double-check and push the updated commit? The proposed fix from the review should look like this:
import React, { useState, useEffect } from "react";const [resolvedTheme, setResolvedTheme] = useState<Exclude<Theme, "system">>(() => {
if (Theme !== "system") return Theme;
if (typeof window === "undefined") return "light";
return window.matchMedia("(prefers-color-scheme: dark)").matches ? "dark" : "light";
});
useEffect(() => {
if (Theme !== "system") {
setResolvedTheme(Theme);
return;
}
const mediaQuery = window.matchMedia("(prefers-color-scheme: dark)");
const handleChange = (e: MediaQueryListEvent) => {
setResolvedTheme(e.matches ? "dark" : "light");
};
setResolvedTheme(mediaQuery.matches ? "dark" : "light");
mediaQuery.addEventListener("change", handleChange);
return () => mediaQuery.removeEventListener("change", handleChange);
}, [Theme]);Once the updated commit is pushed, I'll be happy to re-verify! 🐇
🧠 Learnings used
Learnt from: rahul-vyas-dev
Repo: AOSSIE-Org/SupportUsButton PR: 5
File: src/types/index.ts:89-113
Timestamp: 2026-03-07T09:55:47.961Z
Learning: In the `SupportUsButton` project (`src/types/index.ts`), the `sponsors` type (`export type sponsors = sponsor[]`) is intentionally used to represent both current and previous sponsors in a single array. No separate `previousSponsors` type is needed or expected.
| // Container for the support us button, with dynamic classes based on the selected theme and custom class names | ||
| <div | ||
| className={`w-full font-sans justify-center items-center text-center ${Theme == "light" || Theme == "dark" ? classAccordingToTheme(Theme) : "bg-black text-white"} ${classNames.container}`} | ||
| className={`w-full font-sans justify-center items-center text-center ${resolvedTheme == "light" || resolvedTheme == "dark" ? classAccordingToTheme(resolvedTheme) : "bg-black text-white"} ${classNames.container}`} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use strict equality (===) for consistency.
Line 91 uses loose equality (==) while other comparisons in the file use strict equality (===). Prefer strict equality throughout.
♻️ Proposed fix
- className={`w-full font-sans justify-center items-center text-center ${resolvedTheme == "light" || resolvedTheme == "dark" ? classAccordingToTheme(resolvedTheme) : "bg-black text-white"} ${classNames.container}`}
+ className={`w-full font-sans justify-center items-center text-center ${resolvedTheme === "light" || resolvedTheme === "dark" ? classAccordingToTheme(resolvedTheme) : "bg-black text-white"} ${classNames.container}`}📝 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
| className={`w-full font-sans justify-center items-center text-center ${resolvedTheme == "light" || resolvedTheme == "dark" ? classAccordingToTheme(resolvedTheme) : "bg-black text-white"} ${classNames.container}`} | |
| className={`w-full font-sans justify-center items-center text-center ${resolvedTheme === "light" || resolvedTheme === "dark" ? classAccordingToTheme(resolvedTheme) : "bg-black text-white"} ${classNames.container}`} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SupportUsButton.tsx` at line 91, In SupportUsButton.tsx update
the loose equality to strict equality: in the JSX className expression that
currently checks resolvedTheme == "light" || resolvedTheme == "dark", change to
resolvedTheme === "light" || resolvedTheme === "dark" so it matches the rest of
the file; this affects the conditional that chooses between
classAccordingToTheme(resolvedTheme) and the fallback "bg-black text-white"
alongside classNames.container.
Comment on lines
+152
to
+161
| ${resolvedTheme === "AOSSIE" && "shadow-xl shadow-primary/20"} | ||
| ${resolvedTheme === "light" && "shadow-xl shadow-gray-300/30"} | ||
| ${resolvedTheme === "dark" && "shadow-xl shadow-gray-700/30"} | ||
| ${resolvedTheme === "minimal" && "shadow-xl shadow-gray-800/30"} | ||
| ${resolvedTheme === "corporate" && "shadow-xl shadow-blue-600/30"} | ||
|
|
||
| // Outline for light and dark themes | ||
| ${Theme === "light" && "outline-1 outline-gray-300"} | ||
| ${Theme === "dark" && "outline-1 outline-gray-700"} | ||
| ${classAccordingToTheme(Theme)}`} | ||
| ${resolvedTheme === "light" && "outline-1 outline-gray-300"} | ||
| ${resolvedTheme === "dark" && "outline-1 outline-gray-700"} | ||
| ${classAccordingToTheme(resolvedTheme)}`} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting repeated theme-based class patterns into a helper.
The pattern ${resolvedTheme === "X" && "shadow-class"} is repeated throughout the file (lines 152-156, 214-218, 252-256, 305-309). A helper object or function could reduce duplication and improve maintainability.
♻️ Example helper approach
const themeStyles = {
shadows: {
AOSSIE: "shadow-xl shadow-primary/20",
light: "shadow-xl shadow-gray-300/30",
dark: "shadow-xl shadow-gray-700/30",
minimal: "shadow-xl shadow-gray-800/30",
corporate: "shadow-xl shadow-blue-600/30",
},
// ... other categories
} as const;
// Usage:
className={`... ${themeStyles.shadows[resolvedTheme]} ...`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SupportUsButton.tsx` around lines 152 - 161, Extract the
repeated theme-based class patterns into a small helper (e.g., a themeStyles
object or getThemeClasses(resolvedTheme) function) and replace the inline
repeated templates in SupportUsButton.tsx that reference resolvedTheme with
references to that helper; specifically consolidate the shadow mappings (AOSSIE,
light, dark, minimal, corporate) and outline mappings and use
themeStyles.shadows[resolvedTheme] and themeStyles.outlines[resolvedTheme] (or
return both from getThemeClasses) in place of the multiple `${resolvedTheme ===
"X" && "..."}` occurrences and in the classAccordingToTheme usage to remove
duplication and centralize theme-to-class logic.
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
Contributor
|
@madhavansingh could you please send any visual guide for this? And also resolve coderabbit review |
Contributor
|
The conflict didn’t get resolved. |
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.
Addressed Issues:
Fixes #13
Screenshots/Recordings:
N/A – This change introduces automatic theme detection logic based on system preferences.
Additional Notes:
This PR introduces a new
Theme="system"option that allows the SupportUsButton component to automatically detect and apply the user's system theme preference.If the system prefers dark mode → the dark theme is applied.
If the system prefers light mode → the light theme is applied.
This improves developer experience and ensures the component automatically adapts to modern UI environments.
Summary by CodeRabbit