Update color CSS variables in Liquid to only use RGB

[KaichenWang]

Why are these changes introduced?

Address #83 (comment)

What approach did you take?

1. Simplify CSS variables in Liquid - Use only RGB definition of each color setting:

Before

/* Define color and its RGB equivalent in Liquid */
--color-base-text: {{ settings.colors_text }};
--color-base-text-rgb: {{ settings.colors_text.red }}, {{ settings.colors_text.green }}, {{ settings.colors_text.blue }};

/* Usage in CSS */
color: var(--color-base-text); /* Opacity = 1 */
color: rgba(var(--color-base-text-rgb), 0.5); /* Opacity < 1 */

After

/* Define only RGB color in Liquid */
--color-base-text: {{ settings.colors_text.red }}, {{ settings.colors_text.green }}, {{ settings.colors_text.blue }};

/* Usage in CSS */
color: rgb(var(--color-base-text)); /* Opacity = 1 */
color: rgba(var(--color-base-text), 0.5); /* Opacity < 1 */

2. Refactor usage of CSS variables to accommodate simplification mentioned in point 1
3. Use same Liquid color settings in giftcard template and password layout
4. Additional updates

Other considerations

Also explored (but did not include in this PR) a solution for addressing:

When color setting is set to none the R,G,B property values are 0,0,0, which is "black"

Potential solution adds some unwanted complexity to color system, so a separate issue will be created to allow for further exploration and discussion.

Demo links

• Store
• Editor

Checklist

• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[Thibaut]

👍 overall, nice cleanup!

[Thibaut]

Not a fan of these "percentage" variables. I think rgba(var(--color-base-text-rgb), 0.2) is more readable. What do we gain by using variables that mimic rgba?

[KaichenWang]

Mostly to make the CSS a bit less verbose

div {
  color: rgba(var(--color-foreground-rgb), 0.2);
}

div {
  color: var(--color-foreground-20);
}

But yes I think we can use the former instead of the latter if it's less ambiguous

[KaichenWang]

@Thibaut Updated. See details

[bertiful]

I find this a bit confusing—I'd expect -rgb to be the one with the rgb function attached. Thoughts on instead having a single color, e.g. --color-foreground: var(--color-base-text) for both so we can rely on a single custom property for all references?

[ludoboludo]

Then the one thing about this approach as I was thinking about it is that we need to add rbg(...) around the var(--color-base-text) variables in a few files which then adds more lines/characters overall.

But I do find it clearer to have one single custom property rather than 2.

[KaichenWang]

I like that suggestion. We just need to ensure we don't introduce another kind of ambiguity:

Currently we have 2 tyoes of color variables:

1. with suffix rgb - meant to be wrapped with rgb(...) or rgba(...)
2. without suffix rgb - meant to be used as-is

By getting rid of rgb suffix type variables, when using a variable, it's not immediately clear if we need to wrap a variable in rgb(...) or not.

Example:

:root,
.color-background-1 {
  --color-link: var(--color-base-outline-button-labels);
  --color-link-hover: rgba(var(--color-base-outline-button-labels), 0.85);
}

.color-background-2,
.color-inverse,
.color-accent-1,
.color-accent-2 {
  --color-link: var(--color-foreground);
  --color-link-hover: rgba(var(--color-foreground), 0.7);
}

--color-link would need to be wrapped in rgb(...), but --color-link-hover would be used as-is.

Trying to figure out an approach that would avoid this ambiguity.

[KaichenWang]

@chrisberthe Updated. See details

[ludoboludo]

Then the one thing about this approach as I was thinking about it is that we need to add rbg(...) around the var(--color-base-text) variables in a few files which then adds more lines/characters overall.

But I do find it clearer to have one single custom property rather than 2.

[KaichenWang]

Update

Following feedback, these updates were made:

1. All color variables are now in R,G,B format

Before

There were 2 types of color variables:

1. with suffix -rgb - meant to be wrapped with rgb(...) or rgba(...)
2. without suffix -rgb - meant to be used as-is

After

• Simplification with removal of suffix -rgb variables
• All color variables now need to be wrapped with rgb(...) or rgba(...)

2. Removed variables for foreground colors with opacity

Before

Variables --color-foreground-XX (with XX denoting opacity percentage) were defined and could be used throughout the theme's CSS

/* example: foreground color with 20% opacity */
color: var(--color-foreground-20);

After

This abstraction was removed in favour of simplicity. Instead, use rgba() in combination with --color-foreground variable.

/* example: foreground color with 20% opacity */
color: rgba(var(--color-foreground), 0.2);

[bertiful]

This is great, thanks for updating 👍🏻 Will approve once this comment is addressed.

[ludoboludo]

Looks good to me 👌

Left a comment to remove a variable where it's not needed.

I do feel like we're losing in ease of customization by hard coding the alpha value in quite a few places vs having css variables set for the different alpha values we need across the theme (like we had with var(--color-foreground-20)).
I did find the variable names straight forward and if you'd need to make a change then you can update it across the theme in one place rather than having to search for each 0.2 values for example.
Not sure how often we will come across the situation where the opacity will need to be changed (designers choose to make a change to the theme or theme support customization for individual merchants) but something to keep in mind 🤔

cc: @Thibaut

[ludoboludo]

Removing those I guess makes it a little longer to update if ever needed when we go back and forth with designers.

[bakura10]

Hi !

I really like the approach of using one variable, that is used either in rgb() or rgba() filter based on the use case. Here are a few additions that would make it simpler to work with colors:

• A new "rgb" variable that would output all the three components: {{ color.rgb }}

• It would be extremely useful to be able to extend the default filter to work with transparent color:

{% assign my_color = section.settings.background | default: settings.background }}

It would assign settings.background if the "section" color is set to rgba(0,0,0,0).

We are using a lot of sections where we want to fallback on default color if none has been set.

[bakura10]

To illustrate a bit more, here is what we have all over our new theme:

I would love to be able to rewrite those with something like that:

--heading-color: {{ section.settings.text_color | default: settings.heading_color | color_extract: 'rgb' }}

The "rgb" would be a new value for color_extract that would automatically output the three components (but without the "rgb()" part as the color_to_rgb does.

Would that be doable? It would remove ton of boilerplate code here.