FEAT use ThemeProvider instead of boolean for theme#1991
Open
hannahwestra25 wants to merge 3 commits into
Open
FEAT use ThemeProvider instead of boolean for theme#1991hannahwestra25 wants to merge 3 commits into
hannahwestra25 wants to merge 3 commits into
Conversation
…rast Replace prop-drilled isDarkMode state with a ThemeProvider/useTheme context hook in src/hooks/ (matching the useConnectionHealth pattern and the style guide's cross-cutting concern rule). Behavior changes: - Three-mode picker (System/Light/Dark) in the sidebar via a Fluent Menu/MenuItemRadio. - 'System' follows prefers-color-scheme live. - Selection persists to localStorage. - forced-colors media query auto-applies Fluent's high-contrast theme regardless of mode, so OS HC users get a usable UI without a manual toggle. - documentElement gets data-theme and color-scheme attributes for any CSS overrides. Refactors: - App.tsx, MainLayout.tsx, and Navigation.tsx no longer pass theme state around. - main.tsx wraps App in <ThemeProvider> outside AuthProvider so theme applies during auth flows. Tests: - New useTheme.test.tsx covers resolver, persistence, live media query updates, document attribute writes, and localStorage failure modes. - Navigation.test.tsx rewritten around the new menu picker. - App.test.tsx and MainLayout.test.tsx updated to drop dropped theme props/tests and wrap with ThemeProvider where needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
hannahwestra25
changed the title
We don't actually QA the app in forced-colors mode and Fluent's HC theme only covers Fluent components — claiming HC support set an expectation we hadn't validated. HC users still get the browser's default forced-colors fallback; if we want real HC support later it should be a dedicated effort. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
So this would allow us to add more custom ones as well? Like a raccoon theme, or pirate theme, etc.? |
Contributor
Author
omg love the idea of a raccoon / pirate theme but yes! Adding another theme is basically:
Two caveats worth flagging:
|
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.
What
Replaces the prop-drilled
isDarkModeboolean with a properThemeProvider+useThemehook. The sidebar now has a 3-way picker (System / Light / Dark) instead of a single toggle, and the choice persists across reloads.Why
The old setup had a few issues:
src/hooks/What's new
prefers-color-schemelive (flip your OS theme, the app follows)data-theme+color-schemewritten to<html>for any CSS that wants to reactTry it
Sidebar → sun/moon icon → pick a mode.
Easiest way to test System mode without changing OS settings: DevTools → Cmd/Ctrl+Shift+P → "Show Rendering" → toggle "Emulate CSS media feature prefers-color-scheme" between light/dark. With the picker on System, the app should flip live.