Page MenuHomePhabricator

Consider providing light mode CSS variables in Less mixin form
Open, MediumPublic2 Estimated Story Points

Description

In T360343#9665324, @Jdrewniak asks for the base (light mode) color CSS variables to be available as a Less mixin as well, in addition to the mixin we already provide for dark mode.

We should consider whether we want to do this, and how. For example, we could make this a separate file, or we could add it to the existing -experimental.less file (which currently only contains Less variable settings).

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
CCiufo-WMF triaged this task as Medium priority.Mon, Apr 1, 5:14 PM
Catrope set the point value for this task to 2.Mon, Apr 1, 5:14 PM

Just to clarify the rationale for this request:
For the night-mode feature, the skins modify the :root selector quite heavily. We support night-mode via media-query as well as a user-preference, requiring us to redeclare the night-mode color palette in two place. We duplicate the day-mode color palette in the same way, but instead of using a mixin, we use the Less :extends feature: i.e: :root:extend( .skin-invert ) {...}

A light-mode color palette mixin would be an alternative to that :extend() approach (both get the job done, but IMO a mixin looks cleaner).

Is this still a relevant request if the steps outlined in T354889#9720761 (folding the experimental build into the main build) are complete?

@CCiufo-WMF yes, a light-mode-palette mixin would still be relevant if the experimental build is folded into the main build.
It's really a matter of deciding which of the following two options is preferable:

using extend
:root:extend( .skin-invert ) {...}

vs.

using mixin
:root, .skin-invert {
  .mixin-light-mode-palette();
}

These are both equally functional, but I'm partial to the mixin because :extend is a relatively obscure Less feature compared to mixins.

CCiufo-WMF renamed this task from Consider providing experimental CSS variables in Less mixin form to Consider providing light mode CSS variables in Less mixin form.Fri, Apr 19, 5:47 PM

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

[mediawiki/skins/MinervaNeue@master] Import codex-design-tokens-experimental.css as Less file

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

🖕 The patch above is tagged with this task just to illustrate the kinds of issues we can run into when using the :extend feature. In that case, it didn't work as expected when combined with the Less @import (inline) option.

Change #1022119 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Import codex-design-tokens-experimental.css as Less file

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

Change #1023500 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[design/codex@main] build, tokens: provide default/light mode vars in a mixin

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

Change #1023500 merged by jenkins-bot:

[design/codex@main] build, tokens: provide a "mode reset" mixin

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