RFC: Apps Dark Mode Support #7734
Conversation
Co-authored-by: Georges Lebreton <[email protected]>
| dark: decidePaletteDark(format), | ||
| text: { |
There was a problem hiding this comment.
It'd be a big change but I'd be very open to moving the existing properties into a palette.light, instead of having them at the top level, that way the palette structure is
{
light: { text: { ... } },
dark: { text: { ... } }.
}It'd be a lot of files to change so worth doing in another PR, but I think this would make it really clear that we support both light & dark mode when working on the project, and also lay the groundwork for expansion to support dark mode on the site in the future!
| @@ -18,6 +19,27 @@ import { AgeWarning } from './AgeWarning'; | |||
| import { DesignTag } from './DesignTag'; | |||
| import { HeadlineByline } from './HeadlineByline'; | |||
|
|
|||
| const darkModeCss = | |||
There was a problem hiding this comment.
Could this live in its own file?
| supportsDarkMode={ | ||
| renderingTarget === 'Apps' | ||
| } |
There was a problem hiding this comment.
Rather than deciding this at the layout level, then passing supportsDarkMode to every component, maybe we should only pass renderingTarget around (as this will probably propegate across a lot of the components anyway), then have the darkModeCss function take renderingTarget instead supportsDarkMode and then it becomes the source-of-truth on which rendering targets support dark mode or not (one place)
|
"This PR is stale because it has been open 30 days with no activity. Unless a comment is added or the “stale” label removed, this will be closed in 3 days" |
|
This PR was closed because it has been stalled for 3 days with no activity. |
Co-authored-by: Georges Lebreton [email protected]
What does this change?
This RFC demonstrates dark mode support for apps articles by extending the existing
decidePalette. There is some excellent prior art about the topic of dark mode in DCR, particularly around CSS Variables in #5454 and #4241.This proposal is an alternative option for consideration. It addresses some of the concerns around determining colour values using CSS variables, but possibly at the cost of increased verbosity in components.
Why?
Dark mode support is an existing feature in our apps, and it's something we want to keep as we implement apps articles in DCR.
Would close #7071
Screenshots
Apps article
In this example, the article headline component turns orange when dark mode is enabled.
Web articles remain unchanged.