-
Notifications
You must be signed in to change notification settings - Fork 320
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
Spike to understand how far turning our Sass variables to CSS custom properties would bring us #4900
Comments
Initial findings documented in this google doc (internal to team only) |
Important This is a writeup of a speculative technical spike into 'dark mode' in the context of things built using the Design System that may appear embedded in webviews in apps. This may ultimately not go anywhere, and doesn't mean that dark mode for the wider web version of GOV.UK is likely or even viable. As a first attempt at spiking this we tried defining the applied CSS palette as CSS variables in Sass, however we quickly ran into issues where we were trying to treat the underlying value as a colour in Sass (for example passing We then explored instead exposing the 'applied' palette as CSS variables on the We then updated references to those Sass variables elsewhere in the codebase to instead reference the CSS variables. By exposing that 'abstraction layer' in CSS we can make changes to it 'at runtime', dynamically changing which colour should be used for the page background, text colour etc. As long as components consistently use those abstractions, we can avoid individual components needing to be aware of the existence of a 'dark mode' at all (at least in the majority of cases). However, unsurprisingly, we aren't doing this consistently and there are some other pain points that came up during the spike. Pain points we identifiedI've used the 'Note' callout to highlight things that we may want to do today, regardless of whether we plan to take any other action off the back of this spike. Hard-coded use of light-grey to delineate bits of user interfaceThere are multiple components that are hard-coded to use light-grey as a way to delineate a bit of the user interface, including:
These components currently use light-grey in our spike, often with white or light-coloured text on top. Note We should work out a name for this functional use of colour and create a new variable for it, and then update these components to reference it. We couldn't easily come up with a good descriptive name for this. It might help to look at what other design systems do in this space. We also identified that the cookie banner was hard-coded to use light-grey as a background colour (fixed in cd9bfc8). We think that the cookie banner uses light-grey for the same reason the footer does – it's the 'canvas' colour used for the Note We should update the cookie banner to use EDIT: ✅ Done in #4919 If we proceed with using CSS variables, we can then use the respective CSS variable instead and dark mode would 'just work'. Hardcoded use of light-grey for hover statesSome components have a hard-coded reference to light-grey as part of a hover state:
These components currently use light-grey when hovered in our spike, often with white or light-coloured text on top. Note We should create a new LinksThe We're doing this by calculating a 99% alpha version of the text colour at compile time in Sass: govuk-frontend/packages/govuk-frontend/src/govuk/helpers/_links.scss Lines 229 to 235 in 2dd9c90
However, now we want to change what 'text style' means at runtime in CSS, so there's a conflict. As this bug has now been fixed in Safari we could consider removing the workaround, depending on usage stats. Alternately, depending on whether the versions of Safari we're targeting support it, we could switch to using a CSS function that does something similar at runtime like: color: hsl(from var(--govuk-text-colour) h s l / 0.99); File uploadWe're setting the native file input element to use govuk-frontend/packages/govuk-frontend/src/govuk/components/file-upload/_index.scss Lines 8 to 10 in 2dd9c90
However, we're not setting a background colour for the button so in the spike we end up with white text on a grey button. We'd need to look at the file input component in more detail and understand what's possible in terms of styling the button and how it appears in the different browsers. EDIT: Resolved by setting Note We should investigate other situations where the fact we're setting the text colour but not the background colour on the file input button may be an issue. There's a chance we might be failing WCAG 1.4.3, 1.4.6 and 1.4.8 due to due to specifying foreground colors without specifying background colors or vice versa (F24) HeaderThe header has a hardcoded reference to white for the bottom border, which is designed to blend with the body of the page. This means that we ended up with a stripe of white in our spike (fixed in 9fcff9b): The header also has two hardcoded colours which are not part of the colour palette:
Note There is another piece of work happening around navigation which is likely to involve changes to the header component, which means it's not worth investing much time on improvements. I think it may still be worth:
FooterThe footer includes an image of the coat of arms of the United Kingdom, which is a single-color PNG in light-grey Depending on the colours we end up using, we may need a way to recolour it depending on mode. This might mean switching out to an SVG (which has been explored before, but discounted because the complexity of the paths results in a comparatively larger file) or using e.g. Warning textThe '!' icon within the warning text component is hardcoded to use black for its background and border and white for the text colour. This means that in dark mode although the exclamation mark is visible, the circle is not and the icon has less overall prominence. Note We should update the warning text component so that the icon matches the text colour – using EDIT: ✅ Done in #4906 If we proceed with using CSS variables, we can then use the respective CSS variables instead and dark mode would 'just work'. (This still assumes that warning text will always appear against the body – really we want the exclamation mark to 'punch through' the circle and use whatever background is behind it. But this is an existing issue we can probably consider out of scope…) Multiple components hardcoded to use whiteThere are a number of components that are hardcoded to use white, including:
Although this may be a gap in how we define colours at a system level, we didn't spend much time on this as it seems likely that all of these components will continue to use white in the same way in dark mode. TagsTags appear very 'bright' in the spike and need further consideration. This is possibly the only place where it seems inevitable that the component will need some awareness of 'mode', and it would be difficult to abstract it away given it references the colour palette directly. Other unanswered questionsWhat happens to things that are already 'dark'? What does that mean for 'inverse' modifiers?There seems to be a general theme emerging that things that already use light text on a dark background would continue to do so in dark mode, rather than being 'inverted' to dark on light, including things like:
This suggests that things like inverse breadcrumbs, back links and buttons should remain the same in dark mode. Which does raise the question of whether 'inverse' is the right name for those modifiers. We briefly talked about '--on-dark' as an alternative. Text inputs and textareasShould text inputs and textareas continue to use black text on a white background, or should they match the body background and text colour? Still to do…
|
We need to think about print styles as part of this – we only want dark mode to apply to screen media, when printing we want to keep the dark on light theme. Note We might want to set up visual regression for our print styles in Percy to catch anything that falls through the gaps, rather than rely on manual testing (and remembering that this is a thing!) |
The |
The spike-css-custom-properties branch holds a codebase where dark mode is implemented through custom properties. |
What
Try introducting CSS custom properties for each of the variables in
_colours-applied.scss
settings file to identify potential breakages from doing so.Assess the impact of having turned those to CSS custom properties in browsers that don't support CSS custom properties (IE11), and potentially older Sass compilers.
With the variables in place, provide a second set of values for them that would trigger a dark mode and identify what works or breaks.
Why
This provides a cheap way to find out the impact of us moving to CSS variables for running a dark mode.
Who needs to work on this
Developers
Who needs to review this
Developers, potentially with designer and accessibility specialist support
Done when:
The text was updated successfully, but these errors were encountered: