Page MenuHomePhabricator

[SPIKE] Investigate Effort of Integrating CSS Variables with OOUI for dark mode
Closed, ResolvedPublic2 Estimated Story PointsSpike

Description

Summary

Currently we have a number of interface elements that utilize OOUI for presentation. As we evaluate various dark mode options, it would be helpful to better understand what the effort would be for these components to adopt CSS variables.

Parameters

The focus of this spike is exclusively on estimating technical effort. We should assume this question is separate from the following questions, which we will be working through elsewhere:

  • Ownership over and responsibility for maintenance of OOUI
  • Whether we even want to make these changes in OOUI at all
  • Whether the Design Systems Team will be using CSS variables for its own internal use case (we have https://phabricator.wikimedia.org/T353172 for this which DST will be working on )

Acceptance Criteria

  • We have made up a companion proof of concept demonstrating an OOUI component and/or page using CSS variables
  • Discuss with the DST team and write up a plan or existing blockers for the following
  • We have outlined multiple possible implementations
  • In light of these options, we have written out a potential implementation plan for this conversion so as to make more concrete exactly what would be implemented to adopt CSS variables across all of OOUI
  • We have produced a high-level, time-based estimate for what would be required to implement this plan
  • If necessary/informative, we have broken this estimate down by component where possible / called out differences in implementation
  • We have collaborated with DST and others who have worked with OOUI around establishing parameters and checked our understanding of how tokens currently work and what the path to adoption would be
  • Document outcome of conversations above
  • Create a proof of concept patch for how CSS Variables could be integrated into OOUI
  • If open questions remain, create a follow-up spike for next sprint with these questions

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptDec 18 2023, 5:52 PM
ovasileva renamed this task from [SPIKE] Investigate Effort of Integrating CSS Variables with OOUI to [SPIKE 1 week] Investigate Effort of Integrating CSS Variables with OOUI.Dec 18 2023, 6:32 PM
ovasileva updated the task description. (Show Details)
NBaca-WMF renamed this task from [SPIKE 1 week] Investigate Effort of Integrating CSS Variables with OOUI to [SPIKE] Investigate Effort of Integrating CSS Variables with OOUI.Dec 18 2023, 6:33 PM
NBaca-WMF renamed this task from [SPIKE] Investigate Effort of Integrating CSS Variables with OOUI to [SPIKE] Investigate Effort of Integrating CSS Variables with OOUI for dark mode.
NBaca-WMF updated the task description. (Show Details)
NBaca-WMF set the point value for this task to 8.

Change 984854 had a related patch set uploaded (by Jdrewniak; author: Jdrewniak):

[oojs/ui@master] [POC] WikiMediaUI theme using CSS Custom properties

https://gerrit.wikimedia.org/r/984854

Initial findings

I started this investigation by seeing if I could replace Less variables with CSS variables in the existing OOUI WikimediaUI theme. The results are pretty good. There were some up-front challenges but aside from a few special cases, the outcome is pretty usable:

👉 https://people.wikimedia.org/~jdrewniak/OOUI-dark-mode/ 👈

Challenges

Less color functions

The WikimediaUI theme uses built-in Less color functions such as lighten() and darken(). These Less functions require the parameter to be a color value. CSS variables on the other hand, are strings, and are not compatible with these functions. To address this issue, I created .lighten() & .darken() mixins that check if the parameter is a color, if it is, they return the built-in Less function, if it's not, they return the standards-based color-mix() function, mixing the provided color with white to lighten, and with black to darken.

lighten mixin
.lighten( @color, @percent ) {
	@result: if( ( iscolor( @color ) ), lighten( @color, @percent ), color-mix( in srgb, @color, white @percent ) );
}

The Less mix() function was also used in the WikimediaUI theme. I replaced this with the native color-mix() function since it's somewhat analogous.

Limitation of this approach
Browser support. color-mix() is a very new CSS function. It's supported in Chrome 111 and iOS 16. Also, I'm not sure if color-mix() with black/white produce the same results that lighten() and darken() would.

Alternative approaches

  • Use hard-coded colors instead of the Less lighten()/darken()/mix() functions. This would be challenging since these function are used in mixins which accept variable colors, but we could add extra parameters for the extra lighter/darker colors.
  • Use color-mix() but provide a fall-back (not sure what a suitable fallback would be).
Icons

It seems there is some precedent for rendering OOUI in dark-mode, at least in the scope of this ticket: https://phabricator.wikimedia.org/T180890.
For that solution, the icons were inverted using filter:invert() I think we can continue down that road and invert icons that are black for dark-mode.

Limitations
Invert is only suitable for black icons

Alternative approaches
If we want more robust control of icon colors, we would need to think about this more deeply. Codex has adopted inlined data-uri SVG's for it's icons, which can be modified to accept different colors, but cannot be changed dynamically on the client (as far as I know).
On mobile, image masks (mask-image) have been used to change icon colors by editing the element's background-color, which gives greater flexibility and might be a more suitable approach for dark-mode.

Changing Less variables to CSS variables

This was mostly a matter of find and replace. Although I did this with my text editor, it's probably a good opportunity to use a script or codemod tool that can automate this step across repos.

Next steps/open questions

  • Share this approach more broadly & gather feedback
  • Consider alternative approaches
  • Consider how this approach would impact OOUI on different skins
  • Given browser support considerations, would this be a breaking change?
  • Should this be a seperate theme?
  • Can we scope these changes to a specific skin?
ovasileva removed the point value for this task.Jan 4 2024, 6:01 PM
ovasileva subscribed.

removing estimate to re-estimate remaning work for sprint 1

Pausing this work and moving to next sprint. Will split off into more detailed spikes and follow-up conversations with DST.

Jdlrobson set the point value for this task to 2.

We are inching closer to needing to make a decision here so we'd like to allocate some time to talk about this. We need to define scope for this ticket and desired outcomes.

Jdlrobson lowered the priority of this task from High to Medium.Mar 18 2024, 9:37 PM

Per what @ovasileva and I talked about offline, the Editing and Web Teams consider this ticket a blocker for deploying "night mode" to everyone by default.

Jdlrobson raised the priority of this task from Medium to High.Mar 21 2024, 4:43 PM

@Jdrewniak to create a ticket for next sprint to get a decision about which of our 3 options for OOUI to pursue:

  • OOUI will not support night mode
  • OOUI will use design tokens so that OOUI interfaces are themed in night mode
  • We will update important OOUI interfaces to work in night mode (similar to how we did for Echo notifications)

After several conversations I've documented the different options in the following google doc

https://docs.google.com/document/d/17KcPgBX8SilyA093ZQfOVyO_U9LXpD-OVq5cuPMM0iE/edit#heading=h.qxos0yjuelzf

The doc outlines the following options:

  1. Make the OOUI wikimedia-ui-legacy theme use CSS variables
  2. Create a new OOUI theme using CSS variables
  3. Make OOUI use CSS variables with skin style overrides
  4. Use invert for OOUI pages/components
  5. Leave OOUI pages/components in the day theme

Next steps is to get input on the feasibility/maintenance cost etc. of these different options.

THanks @Jdrewniak !

Next steps is to get input on the feasibility/maintenance cost etc. of these different options.

Is there a phabricator ticket for that?\

Change #1014640 had a related patch set uploaded (by Jdrewniak; author: Jdrewniak):

[mediawiki/skins/MinervaNeue@master] [POC] Create custom OOUI styles with CSS vars for night mode

https://gerrit.wikimedia.org/r/1014640

Change #1014686 had a related patch set uploaded (by Jdrewniak; author: Jdrewniak):

[mediawiki/core@master] [POC] Use invert to support night-mode on mobile RecentChanges

https://gerrit.wikimedia.org/r/1014686

I assume this one is not yet complete since we don't have a decision.

@Jdrewniak to update with current plan and write out follow-up ticket before closing.

Change #1024424 had a related patch set uploaded (by Jdrewniak; author: Jdrewniak):

[oojs/ui@master] [POC] WikimediaUI theme using Codex experimental build.

https://gerrit.wikimedia.org/r/1024424

There will be some extra work with REMs. Jan pushed a patch to fix the issue raised with demo.

I've outlined the next steps for this migration in the following two patches:

Solving these two issue will enable us to upgrade OOUI to use the latest version of Codex and the standard codex-design-token build. After these issues have been resolved, we can (hopefully) upgrade OOUI to the latest version of Codex, which would make it compatible with dark-mode.

In terms of sequencing, I'm thinking:

  1. Make OOUI compatible with rem's,
  2. Update OOUI from 1.3.5 to 1.3.6 (switches units to rems)
  3. Make OOUI compatible with CSS custom properties
  4. Upgrade OOUI to Codex v1.4 or latest
Jdrewniak updated the task description. (Show Details)

Thanks @Jdrewniak LGTM! I'll sign this off tomorrow morning. I see two unticked checkboxes:

  • We have produced a high-level, time-based estimate for what would be required to implement this plan
  • If necessary/informative, we have broken this estimate down by component where possible / called out differences in implementation
  • I assume the former will be done by estimating the two tickets you've created?
  • It sounds like the second no longer applies - should we remove it from the acceptance criteria?

@Jdlrobson yes, I think given the two tasks and the plan outlined about, I think those two checkboxes can now be checked.

Jdlrobson claimed this task.

This LGTM Jan! thanks for pushing this along and excited that we have a concrete path forward!