T345056: dark mode #186
T345056: dark mode #186
Conversation
|
There are several "line too long" warnings that should be addressed here. |
|
loading state, can we make cta bar not invert? |
| .wikipediapreview-header-closebtn { | ||
| filter: unset; | ||
| } | ||
| } |
There was a problem hiding this comment.
Some of the duplication with less mixins could be avoided with CSS custom properties (are they okay to use in this project?). Example:
/* Define default light theme custom properties */
:root {
--background-color: #ffffff;
--text-color: #000000;
}
/* Override custom properties for dark theme based on media query preference */
@media (prefers-color-scheme: dark) {
:root {
--background-color: #000000;
--text-color: #ffffff;
}
}
/* Explicit class for light theme */
.light-theme {
--background-color: #ffffff;
--text-color: #000000;
}
/* Explicit class for dark theme */
.dark-theme {
--background-color: #000000;
--text-color: #ffffff;
}
/* Use custom properties */
body {
background-color: var(--background-color);
color: var(--text-color);
}There was a problem hiding this comment.
I don't see why it wouldn't be okay to use them, I refactored to use CSS custom properties and indeed it seems a bit cleaner (this way seems to be more of a standard - https://web.dev/articles/prefers-color-scheme#stylesheet_architecture), thanks
There was a problem hiding this comment.
:root represent the whole document, probably the <html> or <body> tag. It's important to remember that this is a small component hosted within any website. If we want to use an approach like that, everything has to be prefixed so that even if it bleeds into the host, it has no side effects on it.
There was a problem hiding this comment.
LESS variables would probably give you a similar coding style but with actual isolation.
There was a problem hiding this comment.
Oh that's a good point, I can update using LESS variables and maybe prefix it anyway as something like --wikipediapreview-primary-background-color
There was a problem hiding this comment.
Leaving it with prefixes for now, let's make a decision next week
There was a problem hiding this comment.
this looks good now i think, but can we still use mixin() here so we don't repeat the --var settings?
There was a problem hiding this comment.
Do we need to repeat the styles for the "light" variants? Aren't those the defaults?
There was a problem hiding this comment.
can we also write in README to explain how the site owner custom their dark mode styling?
custom code like this, and explain what's the var there
@media (prefers-color-scheme: dark) {
.wikipediapreview {
--wikipediapreview-primary-background-color: #202122;
--wikipediapreview-secondary-background-color: #202122;
--wikipediapreview-primary-color: #eaecf0;
--wikipediapreview-filter-setting: invert(1);
}
}
There was a problem hiding this comment.
Great feedback all, thanks
README.md
Outdated
| @media (prefers-color-scheme: dark) { | ||
| .wikipediapreview { | ||
| --wikipediapreview-primary-background-color: #202122; | ||
| --wikipediapreview-secondary-background-color: #202122; |
https://phabricator.wikimedia.org/T345056
Demo: https://wikimedia.github.io/wikipedia-preview/T345056-dark-mode/
This the first step for supporting dark mode in the Wordpress plugin: the dark mode feature will be native to the core library and the plugin requests it as necessary. I've made the spanish demo page an example with the dark view, for simplicity.