feat: add theme toggle to all settings pages ($200 bounty) - #301
feat: add theme toggle to all settings pages ($200 bounty)#301foxyManTou wants to merge 1 commit into
Conversation
Ship dark mode toggle for settings pages (bounty.$200) - Added ThemeToggle component to org settings layout - Added ThemeToggle component to personal settings layout - Toggle available on all settings sub-pages: account, general, billing, payments, members - Uses next-themes (already wired in providers.tsx) - Compact pill-button UI: Light / Dark / System - Consistent design language using existing design tokens (primary, border-subtle, surface-hover)
|
Someone is attempting to deploy a commit to the Bounty Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds theme toggle UI controls across two settings layouts ( ChangesTheme Toggle Implementation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/web/src/app/`[slug]/settings/layout.tsx:
- Around line 4-44: The ThemeToggle logic is duplicated across multiple layouts
and overlaps with ThemeSelector, so extract a single shared theme-picking
component under components/settings and reuse it from the settings layouts. Keep
the theme state handling and button rendering in one place, then update this
ThemeToggle implementation and the matching layout/component to import and
configure that shared component instead of copy-pasting the same UI and logic.
- Around line 4-44: Guard the ThemeToggle render in layout.tsx until the
component is mounted, since useTheme().theme is undefined on the server/first
client pass and currently changes the active class and aria-pressed in
ThemeToggle. Add a mounted state check inside ThemeToggle and return null or
defer the toggle UI until after mount, while keeping the existing theme list,
setTheme handler, and isActive logic unchanged once mounted.
In `@apps/web/src/components/settings/theme-selector.tsx`:
- Around line 14-33: The ThemeSelector currently reads useTheme() immediately
and falls back to theme ?? 'system', which can render the wrong active state
during SSR and then flicker on hydration. Update ThemeSelector to delay any
theme-dependent rendering until after the component mounts by using a
mounted/ready state or a placeholder, and keep the currentTheme logic and themes
map rendering gated behind that check.
🪄 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: d17f69b0-732a-4d4b-a37b-64f89587003d
📒 Files selected for processing (3)
apps/web/src/app/[slug]/settings/layout.tsxapps/web/src/app/settings/layout.tsxapps/web/src/components/settings/theme-selector.tsx
| import { Sun, Moon, Monitor } from 'lucide-react'; | ||
| import { cn } from '@bounty/ui/lib/utils'; | ||
| import { useTheme } from 'next-themes'; | ||
|
|
||
| function ThemeToggle() { | ||
| const { theme, setTheme } = useTheme(); | ||
|
|
||
| const themes = [ | ||
| { name: 'light', label: 'Light', icon: Sun }, | ||
| { name: 'dark', label: 'Dark', icon: Moon }, | ||
| { name: 'system', label: 'System', icon: Monitor }, | ||
| ] as const; | ||
|
|
||
| return ( | ||
| <div className="flex flex-col gap-2 mb-4"> | ||
| <label className="text-xs font-medium text-text-secondary">Theme</label> | ||
| <div className="flex gap-1.5"> | ||
| {themes.map(({ name, label, icon: Icon }) => { | ||
| const isActive = theme === name; | ||
| return ( | ||
| <button | ||
| key={name} | ||
| type="button" | ||
| onClick={() => setTheme(name)} | ||
| className={cn( | ||
| 'inline-flex items-center gap-1.5 px-2.5 py-1.5 rounded-md text-xs font-medium border transition-colors', | ||
| isActive | ||
| ? 'bg-primary/10 border-primary/40 text-foreground' | ||
| : 'border-border-subtle bg-background text-text-secondary hover:bg-surface-hover hover:text-foreground' | ||
| )} | ||
| aria-pressed={isActive} | ||
| > | ||
| <Icon className="h-3 w-3" /> | ||
| {label} | ||
| </button> | ||
| ); | ||
| })} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate ThemeToggle implementation across layouts.
This component is copy-pasted verbatim in apps/web/src/app/settings/layout.tsx, and a third, visually different ThemeSelector exists in apps/web/src/components/settings/theme-selector.tsx implementing the same "pick a theme" concept. Consider extracting one shared, parameterizable theme-toggle component under components/settings/ and importing it in both layouts instead of duplicating the logic and styling.
Also applies to: 53-55
🤖 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 `@apps/web/src/app/`[slug]/settings/layout.tsx around lines 4 - 44, The
ThemeToggle logic is duplicated across multiple layouts and overlaps with
ThemeSelector, so extract a single shared theme-picking component under
components/settings and reuse it from the settings layouts. Keep the theme state
handling and button rendering in one place, then update this ThemeToggle
implementation and the matching layout/component to import and configure that
shared component instead of copy-pasting the same UI and logic.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== layout.tsx ==\n'
sed -n '1,220p' apps/web/src/app/'[slug]'/settings/layout.tsx
printf '\n== search next-themes usage ==\n'
rg -n "useTheme\(|mounted|setMounted|theme === name|aria-pressed" apps/web/src/app apps/web/src/components apps/web/src -g '!**/node_modules/**'
printf '\n== package.json refs ==\n'
rg -n '"next-themes"|next-themes' package.json apps/web/package.json . -g 'package.json'Repository: bountydotnew/bounty.new
Length of output: 4842
Guard theme-dependent markup until mount.
theme drives the active button class and aria-pressed on the first render. Add a mounted check, or defer rendering this toggle, so server and client markup stay consistent.
🩹 Proposed fix: guard render until mounted
function ThemeToggle() {
const { theme, setTheme } = useTheme();
+ const [mounted, setMounted] = React.useState(false);
+
+ React.useEffect(() => {
+ setMounted(true);
+ }, []);
+
+ if (!mounted) return null;
const themes = [🤖 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 `@apps/web/src/app/`[slug]/settings/layout.tsx around lines 4 - 44, Guard the
ThemeToggle render in layout.tsx until the component is mounted, since
useTheme().theme is undefined on the server/first client pass and currently
changes the active class and aria-pressed in ThemeToggle. Add a mounted state
check inside ThemeToggle and return null or defer the toggle UI until after
mount, while keeping the existing theme list, setTheme handler, and isActive
logic unchanged once mounted.
| export function ThemeSelector() { | ||
| const { theme, setTheme } = useTheme(); | ||
| const currentTheme = theme ?? 'system'; | ||
|
|
||
| return ( | ||
| <Card className="p-5"> | ||
| <div className="flex items-center justify-between mb-4"> | ||
| <div> | ||
| <h2 className="text-sm font-semibold text-foreground"> | ||
| Appearance | ||
| </h2> | ||
| <p className="text-xs text-text-muted mt-0.5"> | ||
| Choose your preferred theme for the dashboard | ||
| </p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="flex flex-wrap gap-2"> | ||
| {themes.map(({ name, label, icon: Icon }) => { | ||
| const isActive = currentTheme === name; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## theme-selector\n'
sed -n '1,220p' apps/web/src/components/settings/theme-selector.tsx
printf '\n## search useTheme and theme provider usage\n'
rg -n "useTheme\\(|ThemeProvider|mounted|system" apps/web/src -g '!**/node_modules/**'Repository: bountydotnew/bounty.new
Length of output: 7370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## theme-provider\n'
sed -n '1,220p' apps/web/src/components/theme-provider.tsx
printf '\n## app providers\n'
sed -n '1,220p' apps/web/src/components/providers.tsx
printf '\n## settings layouts using useTheme\n'
sed -n '1,180p' apps/web/src/app/settings/layout.tsx
printf '\n---\n'
sed -n '1,180p' apps/web/src/app/[slug]/settings/layout.tsx
printf '\n## find mounted/suppressHydrationWarning in web app\n'
rg -n "suppressHydrationWarning|mounted|resolvedTheme|ThemeProvider" apps/web/src -g '!**/node_modules/**'Repository: bountydotnew/bounty.new
Length of output: 8656
🌐 Web query:
next-themes hydration mismatch mounted theme-dependent UI documentation
💡 Result:
Hydration mismatches occur when using next-themes because theme information—stored in browser APIs like localStorage—is unavailable during server-side rendering (SSR) [1][2]. Consequently, the theme values returned by the useTheme hook are undefined on the server but resolve to a specific theme (e.g., 'light' or 'dark') once the component mounts on the client [1][2]. React detects this inconsistency between the initial server-rendered HTML and the client's expected state, triggering a hydration error [1][3]. To address this, there are two primary approaches: 1. The Mounted State Pattern (Classic Solution) To prevent the error, you must delay rendering any theme-dependent UI until the component has mounted on the client [2]. You can implement this by creating a custom hook or using a local state within your component: - Initialize a mounted state to false [1][3]. - Use useEffect to set it to true once the component mounts [1][3]. - Conditionally render your theme-dependent components only after the state is true (or render a placeholder/skeleton while false) [3][2]. Example: const [mounted, setMounted] = useState(false); useEffect( => setMounted(true), []); if (!mounted) return null; // Or return a loading skeleton return
Citations:
- 1: https://deepwiki.com/pacocoursey/next-themes/4.5-avoiding-hydration-mismatches
- 2: https://github.com/pacocoursey/next-themes/blob/master/next-themes/README.md
- 3: https://dev.to/edgardo_mota/solving-a-hydration-error-in-nextjs-with-next-themes-bce
- 4: docs: add Tailwind v4 data-theme styling guidance pacocoursey/next-themes#395
- 5: [Readme] Clear up hydration issue explanation pacocoursey/next-themes#364
- 6: [Bug]: Hydration failed pacocoursey/next-themes#317
- 7: https://stackoverflow.com/questions/77026759/using-next-themes-for-dark-mode-generates-hydration-failed-error
- 8: I am using nextjs 13, typescript and tailwind and get this error of hydration pacocoursey/next-themes#169
Delay rendering this selector until it mounts. theme ?? 'system' still renders “System” as active on the server, then flips to the persisted theme on the client and causes a hydration mismatch/flicker. Gate the theme-dependent UI behind a mounted check or placeholder before reading theme.
🤖 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 `@apps/web/src/components/settings/theme-selector.tsx` around lines 14 - 33,
The ThemeSelector currently reads useTheme() immediately and falls back to theme
?? 'system', which can render the wrong active state during SSR and then flicker
on hydration. Update ThemeSelector to delay any theme-dependent rendering until
after the component mounts by using a mounted/ready state or a placeholder, and
keep the currentTheme logic and themes map rendering gated behind that check.
|
🦊 Friendly ping! This PR implements the theme toggle for all settings pages as per the $200 bounty. Ready for review and merge. Payment to 0x93bd7eb8d98cb6a6ce0a72010d4536fbe5391241 (Base) via Stripe. |
1 participant
Summary
Ships dark mode / theme switching to all settings pages as requested in the bounty.
Changes
New
ThemeTogglecomponent in settings layouts (org-scoped and personal)next-themesuseTheme()— already wired inProvidersborder-subtle,surface-hover,primary)org settings layout (
/[slug]/settings/layout.tsx) — theme toggle at top of scroll areapersonal settings layout (
/settings/layout.tsx) — theme toggle at top of scroll areaCoverage
The toggle now appears on every settings sub-page:
Design
Compact, accessible, uses existing
lucideicons, no external deps, no API calls needed (theme is fully client-side).PR Links
foxy/dark-mode-settingsCloses the dark mode bounty.
Summary by CodeRabbit