feat: add dark mode and consolidate theming onto design tokens - #155
Merged
zeemscript merged 5 commits intoJul 29, 2026
Conversation
Wire next-themes with a ThemeProvider and a hydration-safe theme toggle, make the three appearance controls in Account > Settings functional, and migrate hardcoded colors in the authenticated app to semantic tokens. Split the overloaded accent token: --color-accent stays a fixed dark surface for bg-accent, and a new theme-aware --brand-text takes over the text role, which measured 2.38:1 on dark backgrounds. The text token uses two-level indirection so the cascade resolves it per theme and switching themes never repaints from JS. Fix --primary and --sidebar-primary, both of which failed WCAG AA in light and dark, and drop the dead HSL token blocks that were shadowed by the unlayered oklch definitions. Verified by auditing effective foreground/background contrast across every text element on the seven target screens in both themes. Closes Deen-Bridge#126
This reverts commit a4ea71d.
This reverts commit 0a178c8.
Wire next-themes with a ThemeProvider and a hydration-safe theme toggle, make the three appearance controls in Account > Settings functional, and migrate hardcoded colors in the authenticated app to semantic tokens. Split the overloaded accent token: --color-accent stays a fixed dark surface for bg-accent, and a new theme-aware --brand-text takes over the text role, which measured 2.38:1 on dark backgrounds. The text token uses two-level indirection so the cascade resolves it per theme and switching themes never repaints from JS. Fix --primary and --sidebar-primary, both of which failed WCAG AA in light and dark, and drop the dead HSL token blocks that were shadowed by the unlayered oklch definitions. Verified by auditing effective foreground/background contrast across every text element on the seven target screens in both themes. Closes Deen-Bridge#126
…erti/dnb-frontend into feat/126-dark-mode-theming
|
@cLamberti is attempting to deploy a commit to the Deen Bridge Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
WalkthroughAdds configurable light/dark themes, persisted accent and font-size preferences, theme controls, updated CSS variables, and theme-aware styling across account, dashboard, card, modal, widget, and reader interfaces. ChangesTheme and appearance system
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
app/dashboard/earnings/page.jsx (1)
149-153: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass the new color variables directly to the chart.
The theme tokens now contain complete
oklch(...)colors, sohsl(var(--primary))andhsl(var(--muted))produce invalid CSS. Revenue bars or tooltip cursors can lose their colors after the theme migration. Usevar(--primary)andvar(--muted)directly, or define separate channel variables.Also applies to: 301-305
🤖 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 `@app/dashboard/earnings/page.jsx` around lines 149 - 153, Update the chartConfig color values for revenue and the corresponding muted tooltip cursor configuration to use the complete theme tokens directly via var(--primary) and var(--muted), removing the hsl() wrappers. Preserve the existing chart labels and configuration behavior.app/dashboard/sadaqah/page.jsx (1)
77-93: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle rejected fund-stat requests.
If
getDonationStats()rejects,setStatsLoading(false)is never reached and the page remains stuck in its skeleton state. Wrap the request intry/catch/finally, setstatsError, and preserve the retry state.As per path instructions,
app/**data-fetching screens must expose loading and error states.🤖 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 `@app/dashboard/sadaqah/page.jsx` around lines 77 - 93, Update fetchStats to handle rejected getDonationStats requests with try/catch/finally. Set statsError from the caught error while preserving the existing success and unconfigured handling, and ensure setStatsLoading(false) runs in finally so loading always ends and retry behavior remains available.Source: Path instructions
app/dashboard/search/[searchparam]/page.jsx (2)
61-67: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDistinguish failed searches from empty results.
catch(() => setResults([]))makes API/network failures render the “No results found” state, leaving users without an error or retry path. Track anerrorstate and render it separately.As per path instructions,
app/**data-fetching screens must expose loading and error states.Also applies to: 125-127
🤖 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 `@app/dashboard/search/`[searchparam]/page.jsx around lines 61 - 67, Update the search page’s searchQuery effect to track failures in a dedicated error state instead of converting them to empty results. Clear the error when starting a new search, set it in the catch handler, and render a distinct error state with an appropriate retry path separate from the “No results found” state while preserving loading behavior.Source: Path instructions
56-57: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRead the route param with
useParams()oruse(params).
This page is a client component, soparams?.searchparamis the old sync contract and can leave the search empty in Next 15. Unwrapparamswith Reactuse()or read the segment viauseParams()instead.🤖 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 `@app/dashboard/search/`[searchparam]/page.jsx around lines 56 - 57, Update the Page component’s route-parameter handling to use useParams() or React use(params) instead of synchronously reading params?.searchparam, ensuring the Next 15 route value populates the search term while preserving the existing empty-string fallback.Source: Path instructions
app/dashboard/library/read/[bookid]/BookReaderClient.jsx (1)
263-269: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTheme-aware foregrounds are still placed on fixed light surfaces.
The dark-theme
text-brand-textvalue is light green, but several affected surfaces remain white or light-only. Convert the surfaces—not just the foreground token—to semantic theme tokens.
app/dashboard/library/read/[bookid]/BookReaderClient.jsx#L263-L269: replace the reader header and related white surfaces withbg-card/bg-backgroundvariants; the same applies to the changed loaders and controls.app/dashboard/sadaqah/page.jsx#L167-L170: replace the hero and statisticvia-whitegradients with theme-aware surfaces; this also covers the changed statistic values at Lines 228-247.components/organisms/dashboard/JaasMeetingClientSection.jsx#L348-L355: make the outlined disabled Button surface theme-aware instead of forcingbg-white.🤖 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 `@app/dashboard/library/read/`[bookid]/BookReaderClient.jsx around lines 263 - 269, Replace fixed light surfaces with semantic theme-aware tokens across all affected sites: in BookReaderClient.jsx, update the reader header, loaders, and controls around the shown Link to use bg-card/bg-background variants; in app/dashboard/sadaqah/page.jsx lines 167-170, replace hero and statistic via-white gradients, including the changed statistic values at lines 228-247, with theme-aware surfaces; in JaasMeetingClientSection.jsx lines 348-355, remove forced bg-white from the outlined disabled Button and use a theme-aware surface.app/dashboard/spaces/[spacesid]/page.jsx (1)
10-11: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAwait
paramsbefore destructuring. Next.js 15 App Router treatsparamsas async; useconst { spacesid } = await params;to stay on the supported path.🤖 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 `@app/dashboard/spaces/`[spacesid]/page.jsx around lines 10 - 11, Update the Page function’s parameter handling to await params before destructuring, assigning spacesid from the resolved params object while preserving the existing page behavior.Source: Path instructions
🤖 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 `@app/dashboard/saved/page.jsx`:
- Line 97: Update the Bookmark icon styling to keep its fill consistent with the
brand stroke: replace fill-accent with fill-brand-text in the Bookmark element,
or remove the fill class if the icon should remain outlined.
---
Outside diff comments:
In `@app/dashboard/earnings/page.jsx`:
- Around line 149-153: Update the chartConfig color values for revenue and the
corresponding muted tooltip cursor configuration to use the complete theme
tokens directly via var(--primary) and var(--muted), removing the hsl()
wrappers. Preserve the existing chart labels and configuration behavior.
In `@app/dashboard/library/read/`[bookid]/BookReaderClient.jsx:
- Around line 263-269: Replace fixed light surfaces with semantic theme-aware
tokens across all affected sites: in BookReaderClient.jsx, update the reader
header, loaders, and controls around the shown Link to use bg-card/bg-background
variants; in app/dashboard/sadaqah/page.jsx lines 167-170, replace hero and
statistic via-white gradients, including the changed statistic values at lines
228-247, with theme-aware surfaces; in JaasMeetingClientSection.jsx lines
348-355, remove forced bg-white from the outlined disabled Button and use a
theme-aware surface.
In `@app/dashboard/sadaqah/page.jsx`:
- Around line 77-93: Update fetchStats to handle rejected getDonationStats
requests with try/catch/finally. Set statsError from the caught error while
preserving the existing success and unconfigured handling, and ensure
setStatsLoading(false) runs in finally so loading always ends and retry behavior
remains available.
In `@app/dashboard/search/`[searchparam]/page.jsx:
- Around line 61-67: Update the search page’s searchQuery effect to track
failures in a dedicated error state instead of converting them to empty results.
Clear the error when starting a new search, set it in the catch handler, and
render a distinct error state with an appropriate retry path separate from the
“No results found” state while preserving loading behavior.
- Around line 56-57: Update the Page component’s route-parameter handling to use
useParams() or React use(params) instead of synchronously reading
params?.searchparam, ensuring the Next 15 route value populates the search term
while preserving the existing empty-string fallback.
In `@app/dashboard/spaces/`[spacesid]/page.jsx:
- Around line 10-11: Update the Page function’s parameter handling to await
params before destructuring, assigning spacesid from the resolved params object
while preserving the existing page behavior.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 0d0f6161-c7cc-4629-a105-8c5a5e5ac846
📒 Files selected for processing (46)
CONTRIBUTING.mdapp/account/notifications/page.jsxapp/account/settings/page.jsxapp/dashboard/ai/page.jsxapp/dashboard/courses/[courseId]/CourseDetailPageClient.jsxapp/dashboard/courses/page.jsxapp/dashboard/earnings/page.jsxapp/dashboard/library/[bookid]/BookDetailPageClient.jsxapp/dashboard/library/page.jsxapp/dashboard/library/read/[bookid]/BookReaderClient.jsxapp/dashboard/purchases/page.jsxapp/dashboard/sadaqah/page.jsxapp/dashboard/saved/page.jsxapp/dashboard/search/[searchparam]/page.jsxapp/dashboard/spaces/[spacesid]/page.jsxapp/dashboard/spaces/page.jsxapp/layout.jscomponents/atoms/dashboard/DashTabs.jsxcomponents/atoms/dashboard/Notybell.jsxcomponents/atoms/dashboard/Searchbox.jsxcomponents/atoms/form/StarRate.jsxcomponents/molecules/Modal.jscomponents/molecules/dashboard/cards/courseCard.jsxcomponents/molecules/dashboard/cards/libraryCard.jsxcomponents/molecules/dashboard/cards/spaceCard.jsxcomponents/molecules/dashboard/nav-header.jsxcomponents/molecules/dashboard/wizard-step-indicator.jsxcomponents/organisms/account/profile/ProfileTabs.jsxcomponents/organisms/account/profile/ProfileUserInfo.jsxcomponents/organisms/dashboard/JaasMeetingClientSection.jsxcomponents/organisms/dashboard/OngoingSessions.jsxcomponents/organisms/dashboard/PrayerTimesWidget.jsxcomponents/organisms/dashboard/R-BooksCard.jsxcomponents/organisms/dashboard/R-CourseCard.jsxcomponents/organisms/dashboard/RecentChats.jsxcomponents/organisms/dashboard/ReviewsSection.jsxcomponents/organisms/dashboard/StatsOverview.jsxcomponents/organisms/dashboard/StreamingAIChat.jsxcomponents/organisms/dashboard/Supports.jsxcomponents/organisms/dashboard/UpcomingSessions.jsxcomponents/providers/AppearanceProvider.jsxcomponents/providers/ThemeProvider.jsxcomponents/ui/theme-toggle.jsxlib/config/appearance.config.jspublic/sw.jsstyles/globals.css
| <div> | ||
| <h1 className="text-2xl font-bold flex items-center gap-2"> | ||
| <Bookmark className="h-6 w-6 text-accent fill-accent" /> | ||
| <Bookmark className="h-6 w-6 text-brand-text fill-accent" /> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the bookmark fill on the same theme token.
text-brand-text themes the stroke, but fill-accent remains a surface token. In dark mode the filled portion becomes dark gray while the stroke is bright green. Use fill-brand-text for a filled brand icon, or remove the fill class if an outline icon is intended.
🤖 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 `@app/dashboard/saved/page.jsx` at line 97, Update the Bookmark icon styling to
keep its fill consistent with the brand stroke: replace fill-accent with
fill-brand-text in the Bookmark element, or remove the fill class if the icon
should remain outlined.
Contributor
Author
|
CodeRabbit its mentioning some issues; do you want me to resolve them? |
6 tasks
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.
Hi @zeemscript, thanks for the opportunity to contribute and the patience, here's what I did:
Closes #126
What
Wires
next-themesend-to-end, adds an accessible theme toggle, makes the threeappearance controls in Account → Settings functional, and migrates hardcoded
colors in the authenticated app to semantic tokens.
The repo already had a full light/dark token palette and
next-themesinstalled,but nothing rendered a
ThemeProvider, so every dark token was dead code.New
components/providers/ThemeProvider.jsx—next-themeswrappercomponents/providers/AppearanceProvider.jsx— accent colour + font sizecomponents/ui/theme-toggle.jsx— light / dark / system, hydration-safelib/config/appearance.config.js— palettes and the boot scriptChanged — 41 files, +203 −215.
styles/globals.cssloses 74 lines of dead code.Why these decisions
The accent token was doing two contradictory jobs.
--color-accentis a fixed#265902serving both as a dark surface (bg-accent+text-white, 89 files) andas a text colour (
text-accent, 154 occurrences). In dark mode the text rolemeasured 2.38:1 — unreadable.
Making
--color-accenttheme-aware was not an option:--accentin light is anear-white grey, and 73 files put
text-whiteon top of it. That change would haveturned ~11:1 into ~1.1:1 and broken light mode, which works today.
So the token was split by role.
--color-accentstays a fixed surface — the 89files using
bg-accentare untouched — and a new theme-aware--brand-texttakesover the text role.
--brand-textuses two-level indirection, and this is deliberate:The appearance provider writes only the two bottom values, once. The cascade picks
which one applies. The alternative — a single property rewritten by JS on theme
change — runs after
next-themesadds.dark, which is a visible flash of thebrand colour on every toggle.
localStoragestores resolved hex, not the palette name. That keeps thesynchronous boot script trivial: it reads and applies, with no need for the palette
table duplicated inside a serialised string.
Contrast — WCAG AA
Every changed token was measured (oklch → sRGB → WCAG). Both
primarypairs failedin light and dark, not only dark as the issue assumed.
primary-foreground/primaryprimary-foreground/primarysidebar-primary-foreground/sidebar-primarysidebar-primary-foreground/sidebar-primarybrand-textlight on backgroundbrand-textdark on background.dark --primarywasoklch(0.23 0.77 120)— chroma far outside sRGB gamut,clamping to
#005400against a#0d542bforeground.All five accent palettes pass: white on surface 8.36:1–12.54:1, text-light
6.47:1–8.72:1, text-dark 10.48:1–11.93:1.
Testing
Verified against a local mock API with each screen loaded fresh in each theme,
auditing the effective foreground/background contrast of every text element
(compositing real backgrounds up the ancestor chain).
/dashboard/dashboard/courses/dashboard/courses/[id]/dashboard/library/dashboard/library/[id]/dashboard/search/[q]/account/settingsThe audit caught four defects that review alone would have missed, including one
introduced by this branch (a badge on
bg-accentinheriting the new near-blackprimary-foreground, 2.38:1).Appearance controls were exercised with real clicks: selecting purple + large wrote
the three CSS variables and
font-size: 18px, persisted across navigation andreload, and applied before React on boot. With purple active, switching to dark
resolves
--brand-textto#d8b4fepurely through the cascade.npm run lintclean.npm run buildsucceeds.Not verified
/account/walletthrows a client-side exception in this environment. Notcaused by this branch — the file is untouched and the page depends on
friendbot.stellar.organdstellar.expert, which were unreachable. Needs apass with network access.
Notes for the maintainer
The issue lists
BookStats&Info.jsxas an offender; it is not. It usesbg-accent(#265902) with white on top — 8.36:1, and unaffected by themebecause the token is fixed. Its surfaces were left alone.
components/molecules/Modal.jswas changed although it is outside the files theissue names. It had a fixed
bg-whitewhile its text inheritstext-foreground,which turns near-white in dark — white on white, reachable from the nav on every
screen. One-line fix.
Font size has a known limitation, and it is stated in the UI. The control scales
the root font size, but the repo has 47 arbitrary
pxvalues that do not scale,four of them
text-[11px]. Converting those toremis follow-up.Follow-up, measured: 29 files / 68 occurrences of
text-accentremain outsidethe seven target screens (landing, about, blog, contact), at 2.38:1 in dark. Also
text-highlight(15),text-basic(1),text-secondary(8), and theghost/outlineButtonvariants, whosehover:bg-accent hover:text-accent-foregroundyields ~1.6:1 in light. All are instances of the same structural limit: a fixed
colour cannot be readable text on both a light and a dark background.
Open question. Is the brand green identity or user preference? The accent
selector was already in the UI promising the latter, and this PR makes it work on
that premise. If the intent was the former, the control should be removed rather
than implemented — that is a product call, not a technical one.
**Screenshots:
Summary by CodeRabbit
New Features
Style
Documentation