-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Adds ability to persist theming to local storage. #4367
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might I suggest using the prefers-color-scheme
selector to determine the default value? The logic would then essentially be to use the stored value only if it exists, and otherwise fall back to this logic we have Keycloak (but with thepf-v6-theme-dark
class).
Thanks for the suggestion will look to see how to use it. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of notes.
- It would be better to use the
storage
event instead ofvisibilitychange
, this allows the storage to be synchronized when it changes. - Don't store the preference unless it is done so by user action (e.g. clicking the toggle), instead used the result of the media query. This prevents synchronization issues when the user switches between system preferences.
- Use the
change
event of the media query to dynamically change the dark mode based on system preferences in real time.
You'd basically end up with something like:
const DARK_MODE_CLASS = "pf-v6-theme-dark";
const DARK_MODE_STORAGE_KEY = "dark-mode";
const mediaQuery = window.matchMedia("(prefers-color-scheme: dark)");
// Ensure that the dark mode is correctly set when the page is loaded.
updateDarkMode();
// Update the dark mode when the the user changes the system preference.
mediaQuery.addEventListener("change", () => updateDarkMode());
// Update the dark mode when the user changes the preference in another tab.
window.addEventListener("storage", (event) => {
if (event.key === DARK_MODE_STORAGE_KEY) {
updateDarkMode();
}
});
/**
* Updates the dark mode based on the stored value (if any) or the system preference.
*/
function updateDarkMode() {
const storedValue = localStorage.getItem(DARK_MODE_STORAGE_KEY);
const isEnabled = storedValue === null ? mediaQuery.matches : storedValue === "true";
const { classList } = document.documentElement;
if (isEnabled) {
classList.add(DARK_MODE_CLASS);
} else {
classList.remove(DARK_MODE_CLASS);
}
}
/**
* Called when the user toggles the dark mode manually through the UI.
* @param isEnabled Whether the dark mode should be enabled.
*/
function onToggle(isEnabled) {
localStorage.setItem(DARK_MODE_STORAGE_KEY, isEnabled);
updateDarkMode();
}
Fixes issue #4365 by persisting theme setting to local storage.