Skip to content
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

Color Mode Switcher Shows wrong icon in auto mode. Root cause contributes to occasional flickering. #546

Open
nenadvicentic opened this issue Jan 2, 2025 · 0 comments

Comments

@nenadvicentic
Copy link

Currently, color mode switcher displays Light or Dark icon, when no color mode has been stored in the localStorage.

The culprit is following code:

const getPreferredTheme = () => {
const storedTheme = getStoredTheme()
if (storedTheme) {
return storedTheme
}
return window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light'
}
:

The function getPreferredTheme converts non-stored item (null) to light or dark value on the output, Thus, wrong active color mode button is matched.

Further, handling of actual theme set in IU is splitted between the above function and following function:

const setTheme = theme => {
if (theme === 'auto') {
document.documentElement.setAttribute('data-bs-theme', (window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light'))
} else {
document.documentElement.setAttribute('data-bs-theme', theme)
}
}

The function setTheme has special case for input parameter auto. Therefore cases where there is no value stored in localStorage and when value auto has been stored are handled in a slightly different way.

Further, value auto makes sense only for the color-swicher component, as a part of element ID in [data-bs-theme-value="auto"]. In CSS only values light and dark are valid.

Further yet, the event handler:

window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', () => {
const storedTheme = getStoredTheme()
if (storedTheme !== 'light' && storedTheme !== 'dark') {
setTheme(getPreferredTheme())
}
})

Currently works correctly, only if there are only light and dark color modes stored in localStorage, and there are no custom color modes added by user.

Fix could be following:

  1. Threat localStorage.getItem('theme') === null and localStorage.getItem('theme') === 'auto' in the same manner. Or even better, do not store any value in auto mode in localStorage.
  2. Use auto suffix only for color-switcher UI handling, as a fallback value for localStorage.getItem('theme') === null.
nenadvicentic pushed a commit to nenadvicentic/examples that referenced this issue Jan 2, 2025
…ntributes to occasional flickering. twbs#546

- Unifies 'auto' and `null` handling for `theme` to simplify code and avoid occasional flickering.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant