Add basic prefers-color-scheme support#3
Add basic prefers-color-scheme support#3ZoeBijl wants to merge 3 commits intosarahfossheim:prefers-color-schemefrom
Conversation
| saved: localStorage.getItem('selected-theme'), | ||
| dark: 'blue', // Theme to load when prefers-colors-scheme = light | ||
| light: 'pink', // Theme to load when prefers-colors-scheme = dark | ||
| default: 'orange' // Theme to load when prefers-colors-scheme isn’t supported |
There was a problem hiding this comment.
Not sure if these comments here are helpful/needed.
In addition: might be worth mentioning that without JS it’ll default to whatever you set in your CSS with :root.
| prefersColorMediaQuery.addEventListener('change', () => { | ||
| setDefaultTheme(); | ||
| applyInitialTheme(); | ||
| }); |
There was a problem hiding this comment.
I feel like this and line 65+66 can be combined somehow. But not sure where to go next tbh.
There was a problem hiding this comment.
I think so too 🤔 I need to sleep on it and revisit it with fresh eyes tomorrow, but I think something along these lines inside the applyInitialTheme function could maybe simplify it:
- If a theme is saved in the local storage: apply the saved theme.
- Otherwise, if
prefers-color-schemeis supported: applythemes.darkorthemes.lightbased on the media query. - If the
prefers-color-schemeis not supported: do nothing (it will apply based on the:root).
sarahfossheim
left a comment
sarahfossheim
left a comment
There was a problem hiding this comment.
Looks good ✨ I'm just unsure what would happen with the prefersColorMediaQuery when prefers-color-scheme isn't supported (which is only the case for IE if I'm not mistaken).
Once it's approved, let's merge in a separate branch (so the main branch is in sync with the steps of the tutorial) and maybe add an extra note about it in the readme file 😊
| const prefersColorMediaQuery = window.matchMedia("(prefers-color-scheme: dark)"); | ||
| const themes = { |
There was a problem hiding this comment.
I'm not very familiar with window.matchMedia, what would the return value be when the media query is not supported?
| } | ||
|
|
||
| const setDefaultTheme = () => { | ||
| themes.default = (prefersColorMediaQuery.matches === true) ? themes.dark : themes.light |
There was a problem hiding this comment.
On line 31 the default theme is set to orange when prefers-color-scheme isn't supported:
default: 'orange' // Theme to load when prefers-colors-scheme isn’t supported
Would this line overwrite it to the light theme (pink) when the media query isn't supported?
There was a problem hiding this comment.
Oh good question, should add that to the todo list:
- check which theme loads if pcs isn’t supported
There was a problem hiding this comment.
for what it’s worth, if you check your website in IE11 none of the themes load at the moment.
that sounds meaner than i mean it 🫣. i was being lazy about making a test page with the pcs version.
There was a problem hiding this comment.
ok, according to my test page it just returns a true or false, so no errors.
There was a problem hiding this comment.
i think you’re right that the line always overwrites the thing, which is a problem. i’ll work on that.
background info:window.matchMedia("bleh") would return:
MediaQueryList {
media: 'bleh',
matches: false,
onchange: null
}
so given that IE11 supports matchMedia it would just always return false.
There was a problem hiding this comment.
i’ve pushed a fix for this. it’ll now correctly set the default theme if pcs isn’t supported.
| saved: localStorage.getItem('selected-theme'), | ||
| dark: 'blue', // Theme to load when prefers-colors-scheme = light | ||
| light: 'pink', // Theme to load when prefers-colors-scheme = dark | ||
| default: 'orange' // Theme to load when prefers-colors-scheme isn’t supported |
| prefersColorMediaQuery.addEventListener('change', () => { | ||
| setDefaultTheme(); | ||
| applyInitialTheme(); | ||
| }); |
There was a problem hiding this comment.
I think so too 🤔 I need to sleep on it and revisit it with fresh eyes tomorrow, but I think something along these lines inside the applyInitialTheme function could maybe simplify it:
- If a theme is saved in the local storage: apply the saved theme.
- Otherwise, if
prefers-color-schemeis supported: applythemes.darkorthemes.lightbased on the media query. - If the
prefers-color-schemeis not supported: do nothing (it will apply based on the:root).
PCS: `prefers-color-scheme`
|
ok, i think i need to work on this a bit more. if there’s a saved state it shouldn’t switch themes via pcs. that also means that if pcs is responsible for the current theme it shouldn’t save state. |
2 participants
Add support for
prefers-color-scheme(PCS).Settings, you can set which theme should be used:
Supports live switching theme when PCS changes.
Todo:
Tested to work with: