Fetch dark mode settings from system settings #3016
Conversation
| } | ||
| }; | ||
|
|
||
| if (localStorage['parcaDarkModeSystemSettings']) dispatch(setDarkMode(mediaQuery.matches)); |
There was a problem hiding this comment.
Instead of manually manipulating the state in localStorage, should we move this to the ui redux state so that it gets persisted automatically?
There was a problem hiding this comment.
hmm so I am manipulating the localStorage because we need to know on the next visit to the Parca app, if a user had previously set that they would like to use the system settings to determine the theme mode.
If I were to use the redux state, doesn't that mean, it wouldn't get remembered on the next visit?
There was a problem hiding this comment.
Oh I see what you mean, Redux's persistence already utilizes localStorage, and I should just use that instead of the localStorage API right?
There was a problem hiding this comment.
No, I meant not to interact with localStorage at all.
From this code:
If I'm not mistaken, we've configured Redux to persist the ui slice. So internally, Redux should handle the localStorage saving and restoring logic.
And if we move parcaDarkModeSystemSettings field to the ui slice then, we don't have to interact with the localStorage no? Just save it to redux state and it takes care of the persistence.
Yes, that's right, missing the second part of our reply.
There was a problem hiding this comment.
yeah that makes total sense. Thanks for catching that. I'll refactor that bit.
|
@manojVivek this is ready for another review. |
| setSystemSettingsDarkMode(false); | ||
| } | ||
| }; | ||
| }); |
There was a problem hiding this comment.
I suppose the dependency array [] is missed here? Or thats intentional?
| }; | ||
|
|
||
| if (isSystemSettingsTheme) dispatch(setDarkMode(mediaQuery.matches)); | ||
| }); |
Description
🤖 Generated by Copilot at 72f5b0c
This pull request improves the theme mode switching functionality in the UI. It adds a new
ThemeTogglecomponent that replaces theDarkModeTogglecomponent and supports more theme options. It also updates theThemeProviderand theDropdowncomponents to work with the new theme logic and enhance the UI consistency.Changes made
🤖 Generated by Copilot at 72f5b0c
DarkModeTogglecomponent toThemeToggleand add functionality to switch between light, dark, or system theme modes (link, link, link)ThemeProvidercomponent to use redux store and local storage for theme mode and listen to media query for system theme changes (link)Dropdowncomponent to allow rendering button with only an element and no text, and to fit the content and position of theThemeTogglecomponent (link, link, link)DarkModeToggle.tsxfile (link)Fixes #2895