Page MenuHomePhabricator

Develop a strategy for using CSS Custom Properties in Vector
Closed, ResolvedPublic2 Estimated Story Points

Description

In general, CSS variables in the MediaWiki ecosystem are packaged for the Less CSS preprocessor. In order to support font customization in a maintainable way, we need to use CSS Custom Properties instead (in both the Minerva and Vector skins).

There are a number of factors to consider when deciding how we should implement CSS Custom Properties:

  • Should we pull in a set of CSS custom properties from a centralized source (such as Codex?)
  • CSS Custom Properties are globally available in the browser, should we somehow limit their application?
  • Should we namespace the ones used in Vector?
  • Should we scope them to something other than the :root element?
  • Can we set these values to rems?
  • Do we want to share these values across skins or develop them on a per-skin basis?
  • Should we use an intermediate step like assigning CSS Custom Properties to Less variabes? (i.e. @fontSizeLarge: var(--font-size-large);?
  • How should CSS Custom Properties interact with the mediawiki.skins.variables system (if at all)?

Outcome

  • Summarise discussion
  • Document any decisions
  • Create follow up tasks as needed

Event Timeline

There are discussions also happening around T296689 that it is probably worth discussing with them also.

ovasileva set the point value for this task to 2.Sep 14 2023, 5:40 PM

I've been thinking about this quite a bit and there are a number of different factors to consider when introducing CSS custom properties into the codebase. Broadly:

  1. How do we introduce a new technology into the codebase
  2. How do we handle the fact that this technology mimics existing functionality.
  3. How does this technology behave as a consumer of an existing API (Codex tokens)
  4. Hoe does this technology act as an API that's accessible in-browser.

As a new technology

Generally, we should limit the scope of the initial implementation as much as possible. Since this is our first pass at implementing CSS custom properties in this ecosystem, we want the ability to change the implementation later on. The only way we get that optionality is if we limit interdependencies and scope to our initial use-case.

We can achieve this by only implementing the CSS custom properties for font-size first, as well as name-spacing the variables with --vector- so it's clear they are limited to the Vector skin (that should at least discourage their usage outside Vector).

As something that mimics existing functionality

We use Less variables extensively throughout the codebase. Limiting the scope, as mentioned above, means that we will be using a mix of CSS custom properties and Less variables. How do we handle this mix? By separating Less variables and CSS Custom properties as much as possible. That means we should keep them in separate files and not cross-reference them, so if our Less variables are in a file called variables.less, our CSS custom properties should be in a file called CSSCustomProperties.less.

In order to accommodate both Less & CSS variables, we should change the @import pattern we currently use in Less, where the variables.less and mixins.less are imported in every individual file, as well as in the skin.less file that imports those individual files. Instead, we should leave only the import in skin.less and import CSSCustomProperties.less into that file too (because importing CSS custom properties into every individual file would mean duplicating the CSS output).

As something that consumes an existing API

The new CSS custom properties should align with (or eventually be superseded by) Codex tokens. In order to achieve this alignment, we can assign the Less Codex token to CSS custom properties,
e.g. --vector-font-size-base: @font-size-base; . This approach creates a cross-references that I mentioned should be avoided earlier, but it might be the simplest way to integrate Codex initially.

As a globally accessible API itself

Although we should try to limit the initial scope and contain these variables as much as possible, the fact is that once CSS Custom properties are in the browser, they can be modified by any other CSS that's loaded on the page, including user styles, TemplateStyles, gadgets, and site styles or extensions. We can discourage usage by names-spacing the variables with the skin name as mentioned previously, as well as limiting the initial scope, but ultimately once they hit the browser they are a defacto public API.

That being said, there is one use-case for these initial variables that I think we should encourage, which is language-specific font-size changes.

As mentioned in our earlier analysis of cross-language font-size considerations, creating different default font-sizes for different languages would place a large maintenance burden on the skin, and these setting are already being applied on-wiki. However, they are being applied in suboptimal ways that vary across wikis. These new variables could at least standardize how wikis change their default font-size, which would be an improvement over the status quo.

As an API, we should also consider how CSS custom properties fit into the new Stable interface policy for frontend code (assume the implementation eventually stabilizes, which won't be the case at first).

Summary

Each factor above compromises on the other. By limiting scope, we mix two technologies, by consuming an existing api we create cross-references, and by exposing this as a global api we create implicit dependencies. But, such is the nature of software. Next steps: POC.

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

[mediawiki/skins/Vector@master] [POC] Initial CSS Custom Properties implementation

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

Jdlrobson added subscribers: egardner, Jdlrobson.

Web team met today with @egardner and @Volker_E to discuss CSS properties. Documenting action items here:

ACTION [Jan]: Write up in Phabricator to describe the system we are trying to build and the requirements, constraints and why we think the current system cannot be used.
ACTION [Eric] Reach out to Justin and prioritize a dark mode color palette for our variables
ACTION [Volker] Clarify font-size product need on Web side and bring possible gaps back to Codex tokens

I am excited about the possibility of using CSS custom properties in MediaWiki, but I also think we would benefit from thinking carefully about how this approach might fit in (or not fit in) with the existing MediaWiki skin variables system (which is LESS based).

@Jdlrobson and @Jdrewniak, would you say that the biggest appeal of using CSS custom properties for the custom font size use case is the potential for shipping leaner CSS files to the user?

A naive example of font-size customization that relies only on LESS might look something like this (simplified/illustrative):

input.less
@vector-font-size-small: 14px;
@vector-font-size-medium: 16px;
@vector-font-size-large: 18px;

@vector-font-size-base: @vector-font-size-medium;
@vector-font-size-heading: @vector-font-size-base * 1.5;

body {
  font-size: @vector-font-size-base;
}

h1 { 
  font-size: @vector-font-size-heading; 
}

.vector-body--small-text {
  @vector-font-size-base: @vector-font-size-small;
  font-size: @vector-font-size-base;

  h1 { font-size: @vector-font-size-heading; }
}

.vector-body--large-text {
  @vector-font-size-base: @vector-font-size-large;
  font-size: @vector-font-size-base;

  h1 { font-size: @vector-font-size-heading; }
}
output.css
body {
  font-size: 16px;
}
h1 {
  font-size: 24px;
}
.vector-body--small-text {
  font-size: 14px;
}
.vector-body--small-text h1 {
  font-size: 21px;
}
.vector-body--large-text {
  font-size: 18px;
}
.vector-body--large-text h1 {
  font-size: 27px;
}

The problem with the above is that you need a new .vector-body--{size} h1 rule for each size that you support, bloating the final output.


An alternate approach that relied on CSS variables could avoid this problem, because you could just define your heading style like:

h1 {
  font-size: calc(var(--vector-font-size-base), 1.5);
}

Then any h1 element which appeared inside a context where --vector-font-size-base had been modified would calculate the appropriate value without shipping additional CSS to the client.


The idea of taking a few Codex token values in LESS and then exposing them as CSS custom properties (as Jan's proposed patch does) is certainly interesting. But if we are going to start introducing CSS variables into MediaWiki, I wonder if there are any benefits to doing so in a more centralized way – or if there are things we could do on the Codex side to simplify this process (like shipping more variables that preserve calculations so that you only need to override a single value to get updated derived values, etc).

@egardner The task description (which I wrote :P ) asserts that we need CSS custom properties "In order to support font customization in a maintainable way". This statement is somewhat vague, but a lot of it does come down to bytes shipped. In the naive example, the problem is not so much that we need an extra top-level selector, but that each font-size declaration under each top-level selector gets redeclared as well. If we change just 3 different font-sizes and provide 3 different options, The bloat becomes exponential.

input-3x3.less
@vector-font-size-small: 14px;
@vector-font-size-medium: 16px;
@vector-font-size-large: 18px;
@vector-font-size-x-large: 24px;

@vector-font-size-base: @vector-font-size-medium;
@vector-font-size-h1: @vector-font-size-base * 1.5;
@vector-font-size-h2: @vector-font-size-base * 1.3;
@vector-font-size-h3: @vector-font-size-base * 1.2;

body {
  font-size: @vector-font-size-base;
}

h1 { 
  font-size: @vector-font-size-h1; 
}

h2 { 
  font-size: @vector-font-size-h2; 
}

h3 { 
  font-size: @vector-font-size-h1; 
}

.vector-body--small-text {
  @vector-font-size-base: @vector-font-size-small;
  font-size: @vector-font-size-base;
  h1 { font-size: @vector-font-size-h1 }
  h2 { font-size:  @vector-font-size-h2}
  h3 { font-size:  @vector-font-size-h3}
}

.vector-body--large-text {
  @vector-font-size-base: @vector-font-size-large;
  font-size: @vector-font-size-base;
  h1 { font-size: @vector-font-size-h1; }
  h2 { font-size: @vector-font-size-h2; }
  h3 { font-size: @vector-font-size-h3; }
}

.vector-body--x-large-text {
  @vector-font-size-base: @vector-font-size-large;
  font-size: @vector-font-size-base;
  h1 { font-size: @vector-font-size-h1; }
  h2 { font-size: @vector-font-size-h2; }
  h3 { font-size: @vector-font-size-h3; }
}
output-3x3.less
body {
  font-size: 16px;
}
h1 {
  font-size: 24px;
}
h2 {
  font-size: 20.8px;
}
h3 {
  font-size: 24px;
}
.vector-body--small-text {
  font-size: 14px;
}
.vector-body--small-text h1 {
  font-size: 21px;
}
.vector-body--small-text h2 {
  font-size: 18.2px;
}
.vector-body--small-text h3 {
  font-size: 16.8px;
}
.vector-body--large-text {
  font-size: 18px;
}
.vector-body--large-text h1 {
  font-size: 27px;
}
.vector-body--large-text h2 {
  font-size: 23.4px;
}
.vector-body--large-text h3 {
  font-size: 21.6px;
}
.vector-body--x-large-text {
  font-size: 18px;
}
.vector-body--x-large-text h1 {
  font-size: 27px;
}
.vector-body--x-large-text h2 {
  font-size: 23.4px;
}
.vector-body--x-large-text h3 {
  font-size: 21.6px;
}

With CSS custom properties on the other hand, adding extra font-size options becomes much cheaper, and font-sizes don't have to be redeclared.
We can achieve the equivalent output as above with the following:

customProps-3x3.css
html {
  /*font-size base options */
  --vector-font-size-small: 14px;
  --vector-font-size-medium: 16px;
  --vector-font-size-large: 18px;
  --vector-font-size-large: 24px;
  /* context-specific variables */
  --vector-font-size-base: var(--vector-font-size-medium);
  --vector-font-size-h1: calc(var(--vector-font-size-base) * 1.5);
  --vector-font-size-h2: calc(var(--vector-font-size-base) * 1.3);
  --vector-font-size-h3: calc(var(--vector-font-size-base) * 1.2);
}

html.vector-body--small-text {
  --vector-font-size-base: var(--vector-font-size-small);
}

html.vector-body--large-text {
  --vector-font-size-base: var(--vector-font-size-large);
}

html.vector-body--x-large-text {
  --vector-font-size-base: var(--vector-font-size-x-large);
}

body {
  font-size: var(--vector-font-size-base);
}

h1 {
  font-size: var(--vector-font-size-h1);
}

h2 {
  font-size: var(--vector-font-size-h2);
}

h3 {
  font-size: var(--vector-font-size-h3);
}

The differences become larger the more code there is...

Regarding centralization, the initial proposal of vector-specific custom properties is intended to make it easier to eventually switch to a centralized solution by tightly scoping usage to a specific use-case and discouraging usage outside of Vector. Thereby, making these variables easier to change later on. We can further discourage unintended usage by creating a parser linting rule that would block on-wiki usage, or even a StyleLint rule that would block usage outside a specific repo.

However, if we assume that once these variables are in production, that they will eventually spread outside the intended use-case, then scoping them to Vector (initial proposal) becomes a liability (and technical debt). I understand this perspective, and if that's the scenario we plan for, then naming the variables as if they come from a centralized source from the outset makes sense too.

In the customProps-3x3.css example above, the CSS custom properties have to be declared on the <html> element, as opposed to the more common :root element. It turns out :root has higher specificity than <html>, so the subsequent overrides for font-size options, which would depend on a class name modifiers (i.e. html.vector-body--large-text ) wouldn't work if the CSS custom properties were declared on :root.

For reasons like this, I think using Vector or Minerva as a testing ground for CSS custom properties, before rolling out a centralized solution, would help us better understanding the use-cases of a centralized api or the edge-cases we might run into.

Taking this conversation into account, I've updated the POC patch to reflect some of the concerns mentioned above.
The POC patch:

  1. Creates a new Less file that defines a new CSS custom property in Vector
  2. Limits the initial usage to Vector, marking the variables as @private (inspired by the Stable interface policy)
  3. Uses the Codex Token naming convention instead of a skin-specific one (--font-size-medium instead of --vector-font-size-medium)
  4. Uses the existing Codex token value @fontSizeMedium as the basis for the new custom property
  5. Converts that value from em to rem using the Less convert() method, because we would like to switch Vector 2022 over to rems eventually.

@Volker_E is sticking to the Codex naming convention preferable over naming the tokens skin-specific? From our last meeting, we talked about the risk of spreading skin-specific token names across the ecosystem, and how that might be more of a liability than using token that are consistently named. I think by mimicking the Codex variable names, it'll be easier for us to eventually upstream them to core.

@egardner I hope that the example above illustrates the limitations of using Less variables for this purpose (certainly in terms of scalability).

Per the following action item:

ACTION [Jan]: Write up in Phabricator to describe the system we are trying to build and the requirements, constraints and why we think the current system cannot be used.

I started to write an RFC for a generic theming system for MediaWiki, but realized that the scope of such an endeavour is quite large, and I personally do not have a design for such a system, nor can I anticipate what the potential use-case or edge-cases will be. I maintain that a better approach would be to iterate on a very narrowly scoped solution, and see what issues or edge-cases arise through that.

@egardner I hope that the example above illustrates the limitations of using Less variables for this purpose (certainly in terms of scalability).

Yes, I think so. Thanks for humoring me here.

I also appreciate the addition of @private tags – one of my biggest concerns was that other people might start relying on these new variables in a haphazard way, so documenting clearly their intended purpose should help mitigate that.

Per the following action item:

ACTION [Jan]: Write up in Phabricator to describe the system we are trying to build and the requirements, constraints and why we think the current system cannot be used.

I started to write an RFC for a generic theming system for MediaWiki, but realized that the scope of such an endeavour is quite large, and I personally do not have a design for such a system, nor can I anticipate what the potential use-case or edge-cases will be. I maintain that a better approach would be to iterate on a very narrowly scoped solution, and see what issues or edge-cases arise through that.

Even a write-up about the more limited solution being pursued here will be useful I think.

@egardner in terms of documentation, I think adding ADRs to Vector (and this being one of the first) would be an appropriate place to describe the decisions here as well as the concerns/trade-offs related to it.

I've spun out T348984 - Implement initial CSS Custom properties in Vector as the outcome of this ticket, were the focus is on implementing CSS properties in Vector per this discussion. With that I think we can resolve this ticket and move on the implementation details.