Skip to content

feat: add design-system theme provider with light/dark modes (Closes #30) - #43

Merged
meshackyaro merged 3 commits into
workman-labs:devfrom
DammyAji:feat/30-design-system-theme
Aug 19, 2026
Merged

feat: add design-system theme provider with light/dark modes (Closes #30)#43
meshackyaro merged 3 commits into
workman-labs:devfrom
DammyAji:feat/30-design-system-theme

Conversation

@DammyAji

@DammyAji DammyAji commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Closes #30

Design-System Theme Provider (navy/gold/terracotta) with Dark Mode

This PR implements the Design-System Theme Provider for the GuildWorkman web app, codifying the navy/gold/terracotta identity system into a single, accessible theme provider with light and dark modes persisted per user.

Description

The core objective of issue #30 is to:

Codify the navy/gold/terracotta identity system into a theme provider with accessible light and dark modes persisted per user.

Before this PR, the navy/gold/terracotta colour tokens and their dark-mode overrides already lived in src/app/globals.css (Tailwind v4 @theme inline + data-theme overrides), but the logic around them was scattered:

  • A standalone ThemeToggle component read/wrote localStorage and stamped data-theme on <html> directly, with no central state.
  • The no-FOUC (flash-of-wrong-theme) inline script was hard-coded in the root layout, duplicated from the toggle.
  • There was no single provider codifying the theme system, and no test coverage.

This PR introduces a dedicated theme layer that follows the project's existing conventions (mirroring the NotificationProvider Context pattern used for the Transaction Notification Center), centralising every decision about the theme system in one place.

What changed

New files

  • src/lib/theme.ts — the single source of truth for the theme system:

    • Theme type ("light" | "dark") and THEME_STORAGE_KEY
    • isTheme type guard for untrusted values (localStorage, inline script)
    • getStoredTheme / setStoredTheme — guarded, never-throwing per-user persistence
    • getSystemTheme — OS preference via prefers-color-scheme (safe fallback)
    • applyTheme — stamps <html data-theme> + sets color-scheme so native controls (scrollbars, selects, date pickers) match the theme
    • listenForSystemTheme — subscribes to OS preference changes, returns an unsubscribe
    • resolveTheme — stored choice wins, otherwise the OS preference
    • themeScript — the shared no-FOUC paint-time script used by the root layout

  • src/components/theme/ThemeProvider.tsx — React Context provider exposing theme, resolvedTheme, setTheme, and toggleTheme via the useTheme() hook. A stored per-user choice wins; otherwise the OS preference is followed live until the user makes an explicit choice.

  • src/components/theme/index.ts — barrel export (matches the notifications/index.ts convention).

  • src/lib/test/theme.test.ts — 14 unit tests covering storage (including corrupted-storage recovery), theme resolution, DOM application, system-preference subscription/unsubscription, and the no-FOUC script.

Edited files

  • src/components/ThemeToggle.tsx — refactored to consume useTheme() (visuals unchanged), with an added aria-pressed for accessibility.
  • src/app/layout.tsx — wraps the app in <ThemeProvider> and imports the no-FOUC script from @/lib/theme (single source of truth instead of a duplicated inline string).
  • README.md — design-system section updated to document the new provider architecture.
  • TODO.md — implementation checklist and architectural decisions documented, per the issue's requirement to document new dependencies/decisions.

Tasks (from issue #30)

  • Review Next.js App Router structure — studied layout.tsx, the Context provider pattern, and component conventions.
  • Implement the UI components for: Design-System Theme Provider (navy/gold/terracotta) with Dark Mode — ThemeProvider + useTheme + refactored ThemeToggle.
  • Ensure responsive design using Tailwind CSS or standard modules — the toggle renders in both the desktop and mobile navbars.
  • Add caching for npm dependencies in CI — already present: .github/workflows/ci.yml uses actions/setup-node with cache: npm.
  • Run npm run lint and npm run typecheck — both pass (0 errors).
  • Run npm run build to verify production bundling — production build succeeds (18 routes).

Acceptance Criteria

  • Feature accurately implements the objective — the navy/gold/terracotta identity system is codified into a theme provider with accessible light and dark modes persisted per user.
  • No TypeScript errors — tsc --noEmit is clean (CI runs it and blocks on failure).
  • CI pipeline runs efficiently — npm dependency caching is configured in the CI workflow.
  • Code is properly reviewed and approved by codeowners — ready for review in the maintainer review flow.

Verification

Check Command Result
Type-check npx tsc --noEmit ✅ clean
Lint npx eslint . ✅ exit 0, 0 errors
Tests npm test ✅ 49/49 pass (14 new theme tests)
Build npm run build ✅ production build succeeds, 18 routes
Merge git merge-tree --write-tree upstream/dev HEAD ✅ clean, no conflicts

Architectural Decisions

  • No new dependencies. Uses React Context (existing NotificationProvider pattern), react-icons (already a dependency), and the existing Tailwind v4 token setup.
  • Tokens stay in CSS. The navy/gold/terracotta tokens and dark-mode overrides live in src/app/globals.css; src/lib/theme.ts owns the logic around them.
  • Persistence is per-user via localStorage (key theme) — the same mechanism the previous toggle used, so existing stored choices keep working. No auth/session exists yet, so browser-local is the right scope; a per-account key can be layered on later.
  • Dark mode is a first-class, accessible mode: color-scheme is set so native controls adapt, focus rings use outline-gold, and the toggle exposes aria-label + aria-pressed.
  • No flash of wrong theme (FOUC): the root layout's inline themeScript (now imported from @/lib/theme) paints the correct theme before React hydrates; the provider only keeps React state in sync.
  • OS-follow by default: until the user makes an explicit choice, the provider follows prefers-color-scheme live; choosing persists and stops following.

Links

Closes #30

@DammyAji @codebuff-team
feat: add design-system theme provider with light/dark modes (workman…
eb5142b 
…-labs#30)

Codify the navy/gold/terracotta identity system into a ThemeProvider
context with accessible light and dark modes persisted per user.
Theme logic lives in src/lib/theme.ts (storage, resolution, system
listening, no-FOUC script); ThemeToggle now consumes useTheme.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@vercel

vercel Bot commented Aug 18, 2026

Copy link
Copy Markdown

@DammyAji is attempting to deploy a commit to the Meshack Yaro's projects Team on Vercel.

A member of the Team first needs to authorize it.

@meshackyaro

meshackyaro commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #30

Design-System Theme Provider (navy/gold/terracotta) with Dark Mode

This PR implements the Design-System Theme Provider for the GuildWorkman web app, codifying the navy/gold/terracotta identity system into a single, accessible theme provider with light and dark modes persisted per user.

Description

The core objective of issue #30 is to:

Codify the navy/gold/terracotta identity system into a theme provider with accessible light and dark modes persisted per user.

Before this PR, the navy/gold/terracotta colour tokens and their dark-mode overrides already lived in src/app/globals.css (Tailwind v4 @theme inline + data-theme overrides), but the logic around them was scattered:

  • A standalone ThemeToggle component read/wrote localStorage and stamped data-theme on <html> directly, with no central state.
  • The no-FOUC (flash-of-wrong-theme) inline script was hard-coded in the root layout, duplicated from the toggle.
  • There was no single provider codifying the theme system, and no test coverage.

This PR introduces a dedicated theme layer that follows the project's existing conventions (mirroring the NotificationProvider Context pattern used for the Transaction Notification Center), centralising every decision about the theme system in one place.

What changed

New files

  • src/lib/theme.ts — the single source of truth for the theme system:

    • Theme type ("light" | "dark") and THEME_STORAGE_KEY
    • isTheme type guard for untrusted values (localStorage, inline script)
    • getStoredTheme / setStoredTheme — guarded, never-throwing per-user persistence
    • getSystemTheme — OS preference via prefers-color-scheme (safe fallback)
    • applyTheme — stamps <html data-theme> + sets color-scheme so native controls (scrollbars, selects, date pickers) match the theme
    • listenForSystemTheme — subscribes to OS preference changes, returns an unsubscribe
    • resolveTheme — stored choice wins, otherwise the OS preference
    • themeScript — the shared no-FOUC paint-time script used by the root layout

  • src/components/theme/ThemeProvider.tsx — React Context provider exposing theme, resolvedTheme, setTheme, and toggleTheme via the useTheme() hook. A stored per-user choice wins; otherwise the OS preference is followed live until the user makes an explicit choice.

  • src/components/theme/index.ts — barrel export (matches the notifications/index.ts convention).

  • src/lib/test/theme.test.ts — 14 unit tests covering storage (including corrupted-storage recovery), theme resolution, DOM application, system-preference subscription/unsubscription, and the no-FOUC script.

Edited files

  • src/components/ThemeToggle.tsx — refactored to consume useTheme() (visuals unchanged), with an added aria-pressed for accessibility.
  • src/app/layout.tsx — wraps the app in <ThemeProvider> and imports the no-FOUC script from @/lib/theme (single source of truth instead of a duplicated inline string).
  • README.md — design-system section updated to document the new provider architecture.
  • TODO.md — implementation checklist and architectural decisions documented, per the issue's requirement to document new dependencies/decisions.

Tasks (from issue #30)

  • Review Next.js App Router structure — studied layout.tsx, the Context provider pattern, and component conventions.
  • Implement the UI components for: Design-System Theme Provider (navy/gold/terracotta) with Dark Mode — ThemeProvider + useTheme + refactored ThemeToggle.
  • Ensure responsive design using Tailwind CSS or standard modules — the toggle renders in both the desktop and mobile navbars.
  • Add caching for npm dependencies in CI — already present: .github/workflows/ci.yml uses actions/setup-node with cache: npm.
  • Run npm run lint and npm run typecheck — both pass (0 errors).
  • Run npm run build to verify production bundling — production build succeeds (18 routes).

Acceptance Criteria

  • Feature accurately implements the objective — the navy/gold/terracotta identity system is codified into a theme provider with accessible light and dark modes persisted per user.
  • No TypeScript errors — tsc --noEmit is clean (CI runs it and blocks on failure).
  • CI pipeline runs efficiently — npm dependency caching is configured in the CI workflow.
  • Code is properly reviewed and approved by codeowners — ready for review in the maintainer review flow.

Verification

Check Command Result
Type-check npx tsc --noEmit ✅ clean
Lint npx eslint . ✅ exit 0, 0 errors
Tests npm test ✅ 49/49 pass (14 new theme tests)
Build npm run build ✅ production build succeeds, 18 routes
Merge git merge-tree --write-tree upstream/dev HEAD ✅ clean, no conflicts

Architectural Decisions

  • No new dependencies. Uses React Context (existing NotificationProvider pattern), react-icons (already a dependency), and the existing Tailwind v4 token setup.
  • Tokens stay in CSS. The navy/gold/terracotta tokens and dark-mode overrides live in src/app/globals.css; src/lib/theme.ts owns the logic around them.
  • Persistence is per-user via localStorage (key theme) — the same mechanism the previous toggle used, so existing stored choices keep working. No auth/session exists yet, so browser-local is the right scope; a per-account key can be layered on later.
  • Dark mode is a first-class, accessible mode: color-scheme is set so native controls adapt, focus rings use outline-gold, and the toggle exposes aria-label + aria-pressed.
  • No flash of wrong theme (FOUC): the root layout's inline themeScript (now imported from @/lib/theme) paints the correct theme before React hydrates; the provider only keeps React state in sync.
  • OS-follow by default: until the user makes an explicit choice, the provider follows prefers-color-scheme live; choosing persists and stops following.

Links

Closes #30

This is a strong, well-scoped PR that centralizes theme logic, adds a ThemeProvider + hook, refactors the toggle, and adds solid unit tests. Overall I consider it near-ready for merge after addressing a few targeted, actionable items (mainly around runtime-safety, small accessibility details, and one clarity/test gap). Risk: low-to-medium — mostly UI plumbing with good test coverage; main checks are runtime/SSR-safety and accessibility.

Critical, diff-anchored issues to fix before merge

src/lib/theme.ts — getStoredTheme: guard against server-side execution and absent localStorage (window undefined) and ensure function never throws; please ensure top-level calls are safe during SSR (e.g., wrap localStorage access in typeof window !== 'undefined' checks).
src/app/layout.tsx — insertion of themeScript: confirm the script is inserted with React's safe API (e.g., <script dangerouslySetInnerHTML={{ __html: themeScript }} />) and that the exported string cannot be mutated. Also ensure the string is minified/escaped to avoid introducing an accidental XSS vector during render.
src/components/theme/ThemeProvider.tsx — system-preference listener lifecycle: when a user explicitly sets a theme via setTheme, ensure the provider stops following system theme (or documents the intended behavior) and that the listener is cleaned up correctly (unsubscribe on unmount and when switching from "auto" to explicit). If your implementation leaves the listener active after an explicit choice, please either stop listening or clearly document and test the behavior.
Why these are critical: they affect runtime safety (SSR and XSS surface) and the promised UX ("stored per-user choice wins; otherwise OS preference followed live until user chooses"). Each should be verified in code and tests.

Possible improvements (actionable suggestions)

src/components/ThemeToggle.tsx — accessibility: add a clear accessible label (aria-label, or visually-hidden text) describing the control (e.g., "Toggle color theme") in addition to aria-pressed so screen-reader users understand the current state and action. Also ensure the role and keyboard behavior (Enter/Space) are correct.
src/lib/theme.ts — matchMedia compatibility: prefer using matchMedia.addEventListener('change', …) when available, and fall back to addListener/removeListener for older browsers. Add a small comment and a unit test asserting the fallback path is used when addEventListener is absent.
README.md — usage snippet: include an explicit example showing how/where to insert the exported no-FOUC script in Next.js app router (layout.tsx snippet) and note that the provider must wrap the app to ensure theme-aware components read resolvedTheme on first render. This helps future maintainers avoid regressions when changing layout.

Small nits / style suggestions (non-blocking)

src/lib/theme.ts — export naming: ensure exported constants (THEME_STORAGE_KEY) are documented or re-exported in a central place if other modules will reference them.
src/components/theme/index.ts — good to follow the notifications index pattern; ensure barrel export doesn't create circular imports.
README/TODO — consider adding a short note about where to update brand color tokens if those ever change (link to globals.css).

Get this done and it'll be approved and ready to merge.

@DammyAji

DammyAji commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@meshackyaro Please check now and review. Thank you.

@meshackyaro

meshackyaro commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

@meshackyaro Please check now and review. Thank you.

Check my last review and fix the critical, diff-anchored issues before I can approve and merge

@DammyAji @codebuff-team
fix(theme): harden provider for SSR, XSS safety, and listener lifecycle
23925c7 
Address review feedback: guard localStorage/matchMedia against
server-side execution, add a legacy matchMedia addListener fallback,
stop the OS-preference listener once the user makes an explicit choice,
and document the no-FOUC script wiring. Adds unit tests for SSR safety
and the legacy-listener path.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@DammyAji

DammyAji commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@meshackyaro Please check now and review. Thank you.

Check my last review and fix the critical, diff-anchored issues before I can approve and merge

@meshackyaro I actually worked on it but did not push. I just realized, and i have done that so please kindly review.

@vercel

vercel Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
guildworkman-web Ready Ready Preview Aug 19, 2026 10:40pm

meshackyaro
meshackyaro approved these changes Aug 19, 2026
View reviewed changes

@meshackyaro meshackyaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — this looks great and is ready to merge.

Nice work — the ThemeProvider cleanly centralizes theme logic, removes the duplicated no-FOUC script, persists user choice correctly, and the refactor of ThemeToggle is non-invasive and more accessible. Tests cover the core behaviors (storage, resolution, DOM application, and subscription lifecycle), README/TODO updates document the design, and the implementation follows the repo’s existing Context/provider conventions. Low risk, well-scoped, and improves maintainability.

I’m happy to merge this as-is — go ahead when CI is green.

@meshackyaro
meshackyaro merged commit 71bb89f into workman-labs:dev Aug 19, 2026
3 checks passed
@grantfox-oss grantfox-oss Bot mentioned this pull request Aug 19, 2026
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@meshackyaro meshackyaro meshackyaro approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Design-System Theme Provider (navy/gold/terracotta) with Dark Mode

2 participants

@DammyAji @meshackyaro