Add some missing variables in light-theme

[xvw]

While trying the latest version of Odoc (to test with destructive substitution) I noticed that, on Firefox, some colours were not displayed.
According to https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties, you can directly describe a fallback in the accessor of a variable (and this had already been used for a rule).
After this modification, the output is the same of 1.5.2.

[lubegasimon]

Thanks for the PR, could you please attach screen shots?

[dbuenzli]

That kind of trumps the idea of variable. Could you maybe describe what the problem is ?

[xvw]

@lubegasimon

Thanks for the PR, could you please attach screen shots?
Sure

Before:

After:

@dbuenzli

That kind of trumps the idea of variable. Could you maybe describe what the problem is ?

I do not have any strong opinion about CSS variable but in my understanding:

• If the variable is not valid, it take the previous rule value
• If the variable is not defined, it fill the rule with "an unset variable" (so a kind of empty value).

Another fix would have been to ensure that all variables declared in one mode (light vs dark) are declared in the other.

[dbuenzli]

Another fix would have been to ensure that all variables declared in one mode (light vs dark) are declared in the other.

That's a better fix. You hardcoded a value for the same variables in different places. That precisely the problem variables are supposed to solve :-)

[xvw]

I'll do it tomorow. Thanks for the feedback!

[xvw]

I´ve duplicated missing variables in dark-theme and removing default fallback. I still don´t really understand why:

@media (prefers-color-scheme: dark) {
  :root { ... }

Where values seems "close" to dark theme, but not exactly same.

@dbuenzli

That's a better fix. You hardcoded a value for the same variables in different places. That precisely the problem variables are supposed to solve :-)

Yes I agree, but my goal was to provide a quick fix, according to the actual approach. In my opinion:

r {
   rule: default_value;
   rule: var(--var-for-rule);
}

is absolutely identical of :

r {
   rule: var(--var-for-rule,  default_value);
}

Except that the scond one is working.

[dbuenzli]

Yes I agree, but my goal was to provide a quick fix, according to the actual approach.

Ok maybe I didn't understand the approach then. But from a maintenance and understanding perspective it looks like a rather bad approach to me, because now you have duplicated the raw value default_value all over the code base.

If you use variables, the good strategy in my opinion is to define them in :root use them everywhere where you want them to be used. Now to provide a theme just override those variable values you want to change.

[xvw]

No, I think it was my message (the last one) that was not clear. I completely agree that the ideal would be to describe all variables in :root and only modify for additional templates, the variables that change.

But this was not what was originally done. So in my first proposal, I "just fixed the code". Now I duplicated the missing variables, as you suggested, and removed the fallbacks.

[jonludlam]

Thanks, @xvw!

[zoj613]

IS there a way to force the default theme to be light instead of dark? Generating docs via github pages workfflow leads to docs that have a dark theme. I'm unsure how to change this setting.