CSS: cleaner pattern for variable use

[agjohnson]

Here are a couple options for cleaning up our CSS variable patterns. I started to experiment with some of these to see how it felt to override them and author rules around them. I wanted to note this before moving on.

Current issues

• We redefine the defaults used for variables in more than one place -- we use font-family: var(--readthedocs-font-family, "Lato", ...) in multiple places.
• We use --addons prefixed variables inconsistently, so the pattern feels unclear but also redundant.
• We create more variables than necessary, for instance --readthedocs-flyout-dd-font-size.
• Usage of calc() instead of em rules makes variable values probably more confusing than avoiding em helps. em feels safer in a shadow DOM, so rem doesn't feel as necessary as working in the parent DOM.
• We haven't decided or dictated what selectors we want users to use to override variables.

Solutions

Set variable defaults on parent DOM :root

This would be a stylesheet added to document.adoptedStyleSheets:

@layer defaults {
  :root {
    --readthedocs-font-size: 16px;
    --readthedocs-font-family: monospace;
    --readthedocs-flyout-font-size: var(--readthedocs-font-size);
  }
}

And when we use the variable, we don't need to set a default value now:

:host {
  font-size: var(--readthedocs-flyout-font-size);
}

And users can override these variables with:

:root {
  --readthedocs-flyout-font-size: 24px;
}

• Good: We only define the variable defaults once
• Good: No intermediate --addons variables
• Meh: We have to defince the default values outside each addon

Users set defaults on our elements, not :root

Another approach would be to set the variable defaults on each addon Element :host:

/* flyout.css */
:host {
  --readthedocs-flyout-font-size: var(--readthedocs-font-size);
  --readthedocs-flyout-color: red;
}

.container {
  font-size: var(--readthedocs-flyout-font-size);
}

This would differ from the approach above in that users would override variables on elements, not on the parent DOM :root:

readthedocs-flyout {
  --readthedocs-flyout-font-size: 24px;
}

This works because the parent DOM readthedocs-flyout is the same element as the shadow DOM :host. We can't do the above pattern if we instruct users to override variables at :root, this is the reason we have the --addons prefixed variables.

• Good: Variable defaults are set in each addon and right at the top of the CSS. Super easy to work with.
• Meh?: Not quite as nice for users as setting the variables at the parent DOM :root, which is also a fairly common pattern
• Meh?: Some variables can be set at :root while others can't be overridden. If users try to set variables at :root styles will seeming randomly not work.

Standardize local scoped variables

This is what we started to do so far. We set defaults at the addons' CSS file, but use a secondary variable name to give a locally scope variable to the addon (currently these are the --addons prefixed variables).

A couple things we could do to make this pattern better:

• Rename the local --addons prefix variables so it's less redundant and more clear these are local variables -- --local-flyout-font-size, --flyout-font-size, --local-font-size, etc.
• Always use this pattern, so that we have a clear place to define/find variables and their defaults.
• Mix with a pattern above to give global defaults for variables outside addons, like --readthedocs-font-size and --readthedocs-font-family.

:host {
  --local-font-size: var(--readthedocs-flyout-font-size, var(--readthedocs-font-size));
  --local-font-family: var(--readthedocs-flyout-font-family, var(--readthedocs-font-family));
}

.container {
  font-size: var(--local-font-size);
}

• Good: Users can override at :root still
• Good: Defaults are obvious and mostly easy to maintain
• Good: Usage of the local variable feels clean
• Meh: It's confusing to require two levels of defaults for some variables -- var(--readthedocs-flyout-..., var(--readthedocs-..., "and still sometimes a default value too"))

[humitos]

I think I would feel more comfortable with the pattern "Set variable defaults on parent DOM :root". It's the easiest to follow in my opinion and the easiest to communicate to users.

Regarding the downside point, "We have to defince the default values outside each addon", I think we could have a <addon>.defaults.css file per addon that's automatically added by the AddonsBase class using document.adoptedStyleSheets. With that, it will be easy to know where these defaults are defined for each addon.

As a side note, I saw you mentioned:

Usage of calc() instead of em rules makes variable values probably more confusing than avoiding em helps. em feels safer in a shadow DOM, so rem doesn't feel as necessary as working in the parent DOM.

and then you used px to define the --readthedocs-font-size which called my attention. I re-read the sentence from the description and I'm not sure to understand why we changed from rem to px. Can you expand on that?

Edit: oh, should we keep using rem but we should tell people to use px if they are going to override these variables? 🤔 . I just saw #487 where you kept using rem in our definitions...

[agjohnson]

With that, it will be easy to know where these defaults are defined for each addon.

Yeah considered that but it's still a separate file. I think we could avoid a secondary stylesheet by exporting a Lit stylesheet from each addon and combining them all into adoptedStyleSheets at the index though.

Can you expand on that?

px in the above examples are just for illustration of what users might do with it, not a recommendation. Generally, rem is best for our use as it's a relative length.

[agjohnson]

Here's an example using @layer inside the existing CSS: 5d2a33f

This feels like the best of the first two examples. I like that this keeps all CSS variables defined in the exact place we'll be using them, inside each addon CSS file, and still supports overriding at the parent html