feat: implement theme switching with system preference support #902
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #902 +/- ##
=======================================
Coverage 79.15% 79.15%
=======================================
Files 56 56
Lines 2226 2226
=======================================
Hits 1762 1762
Misses 360 360
Partials 104 104 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
eternal-flame-AD
left a comment
eternal-flame-AD
left a comment
There was a problem hiding this comment.
Thanks for the PR, see comments please
| }); | ||
| const theme = themeMap[currentTheme]; | ||
| const prefersDark = useMediaQuery('(prefers-color-scheme: dark)'); | ||
| const paletteMode = currentTheme === 'system' ? (prefersDark ? 'dark' : 'light') : currentTheme; |
There was a problem hiding this comment.
Why does this start with one of the manual options instead of "system", it looks like you are checking the system settings?
I think we can either
(1) Don't put "system" as a setting and only use the media query as a fall back: if the user decide to switch the local storage will have a record so the next time they open the app it will respect their choice. If the user doesn't choose there shouldn't be a local storage entry, so you can detect the system theme and use that.
(2) Keep the original behavior just add "system" as a third coice: always start with dark, and only use the system them when the user explicitly sets it to the "system" setting.
wdyt?
2 participants
toggle.
To maintain a consistent user experience, the dark theme is still used as the default.
related issue: #851
Preview: