Issues/29574 css color variables #29575

helgatheviking · 2021-04-02T16:08:34Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Replace SCSS color variables with CSS color variables. This will allow a theme to override the CSS root variable and have that change propagate to all the places where that color variable is used. Currently, overriding the root variable does nothing as the colors are still hard-coded into the CSS when built from the SCSS.

Closes #29574 .

How to test the changes in this Pull Request:

Rebuild the CSS files, 'npm run build:assets`
View elements that expect to show the WooCommerce coloring

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully run tests with your changes locally?

Changelog entry

Enter a summary of all changes on this Pull Request. This will appear in the changelog if accepted.

ninetyninew · 2021-04-04T09:31:22Z

I noticed here the styles with comment "// Required for default theme compatibility" in woocommerce.scss remain the original, it would be good to have these variable based too for consistency - I'm wondering whether the styles changed and these should use the variables with the old line remaining above so for compatibility if the browser (IE11 and down) does not support CSS variables those original ones are used as a fallback?

claudiosanches · 2021-04-21T20:18:43Z

assets/css/woocommerce.scss

 						text-decoration: none;

 						&:hover {
 							text-decoration: none;
-							color: lighten($secondarytext, 10%);
+							color: lighten(var( --wc-secondary )text, 10%);


Those aren't possible, SCSS needs some value to be able to process using the lighten() function, so this will cause a compilation error, also there's a syntax error here, since var( --wc-secondary )text isn't valid.

Those SCSS functions will need to get removed, at least seems like we are using lighten() and darken(), I haven't yet checked if anything else needs to be replaced, but this two functions are used in many ways, what makes even harder to try to keep, and feels like will require a small refactoring... At least for what I could remember CSS, feels like that we could use CSS filters to some, but probably will not work when trying to only apply to a border or something like, and feels like that we could introduce some more variables and start to use rgba() or hsla() to help defining those other colors.

claudiosanches

Thanks for your help.

There's some syntax errors in many files from the templates, some occurrences of var( --wc-highlight )s-color, also I could find some some SCSS functions that will not work if using CSS variables, I left some thoughts about it in a inline comment.

Anyway, those errors make impossible to compile SCSS, and feels like that it will require a bigger refactoring.

Also seems like there's one extra unrelated change about tax rates in the abstract WC_Order class.

Thanks.

(cherry picked from commit aca7c28)

helgatheviking · 2021-04-22T01:21:30Z

@claudiosanches ooof... some copy/paste fails. I've tried to clean up my commit and pull. But you're definitely right... not sure how to handle the lighten() and darken() bits.

I guess you could do something like

--wc-secondary_light_10: #{$secondary_light10};
--wc-secondary: #{$secondary};
--wc-secondary: #{$secondary_dark10};

but you could collect a lot of variable pretty quickly that way.

barryhughes

We could probably apply the same rules of thumb here as for variables generally.

For instance, if we are doing darken($secondary, 10%) multiple times across the source then it may as well become a variable. A 'one off' on the other hand like darken($secondary, 60%) that happens in only one location could perhaps just be translated to the CSS equivalent (ie, using the hsl() function) in situ—unless we feel using a variable adds clarity/improved readability.

Please also note there were a few other syntax errors stopping things from compiling successfully (some were flagged in the earlier discussion, and I marked a few more).

Thanks for all the work here so far!

barryhughes · 2021-05-04T16:43:02Z

assets/css/twenty-twenty-one.scss

-$highlights-color: #88a171;
+:root {
+	--wc-highlight: #88a171;
+}:


The trailing colon needs to be removed:

Suggested change

}:

}

barryhughes · 2021-05-04T16:43:28Z

assets/css/twenty-nineteen.scss

-$highlights-color: #0073aa;
+:root {
+	--wc-highlight: #0073aa;
+}:


The trailing colon needs to be removed:

Suggested change

}:

}

barryhughes · 2021-05-04T16:43:53Z

assets/css/twenty-twenty.scss

-$highlights-color: #cd2653;
+:root {
+	--wc-highlight: #cd2653;
+}:


The trailing colon needs to be removed:

Suggested change

}:

}

barryhughes · 2021-05-04T17:01:19Z

assets/css/woocommerce.scss

-	background-color: $primary;
-	color: $primarytext;
+	background-color: var( --wc-primary );
+	color: var( --wc-primary )text;


I think Claudio already flagged, but we need to clear up instances like this in order for Grunt to compile things successfully:

Suggested change

color: var( --wc-primary )text;

color: var( --wc-primary );

(If we can successfully compile, it will be easier to evaluate this change 😃 )

helgatheviking · 2021-05-04T17:40:20Z

@barryhughes quick scan shows there's 3%, 5%, 10%, 15%, 20% and 40% darken calculations on the secondary color. 15% and 40% appear a single time but the others appear more than once.

lighten() is used less frequently and I don't see any duplicates (in woocommerce.scss anyway).

I'm not particularly familiar with hsl() but it seems to require 3 numbers and I'm not at all sure how we'd get those from the HEX code that is the color variable... unless we convert the hex colors to rgb?

I can fix the rogue : right now, but fixing the darken/lighten probably depends on how we want to define the colors... would need your feedback there.

peterfabian · 2021-10-13T13:01:59Z

Do you folks want to have this another look @claudiosanches @barryhughes or do we want to do a bigger refactor?

barryhughes · 2021-10-13T15:54:04Z

Hey @peterfabian — I think solving #29574 would be valuable for themers and this PR is on the right track, but of course still needs work (and input to guide Helga or whomever takes it over). Less sure as to priority.

helgatheviking · 2021-10-13T16:47:47Z

I think it's super valuable for plugin devs too! :)

claudiosanches · 2021-10-13T16:56:44Z

@peterfabian this PR still needs some work, there's still a invalid value:

color: var( --wc-primary )text;

Also we probably could consider some refactoring to remove all dependency from SASS.

peterfabian · 2021-10-15T09:01:40Z

Not a super high priority, but since quite a bit of work has been done both by @helgatheviking and us, it might make sense to timebox it to an hour or two to push it forward. What time investment do you folks think this would need?

Though, I'm wondering if this work needs an update wrt the changes in WP 5.8 (theme.json)

helgatheviking · 2021-10-15T14:25:10Z

Fixed the color: var( --wc-primary )text; copy/paste fails in 94b0304

But that still leaves what to do about the use of darken() and lighten() functions. They must be passed a color and not a CSS variable.

Possible options:

unify the various darken/lighten functions and create a few variables like:
primary_10_lighter
primary_10_darker
convert variable colors to hsl or rgb? which I think can then use a 4th param for opacity? Not at all sure if that would work with css variables, just spitballing a bit.

I think this might work? but then anyone overriding the root color will always have to use rgb.

:root {
  /* #f0f0f0 in decimal RGB */
  --color: 240, 240, 240;
}

#element {
  background-color: rgba(var(--color), 0.8);
}

erikyo · 2022-01-25T19:14:18Z

If I may join the conversation, I also think @helgatheviking PR it's a good idea and it makes sense and to renovate the WooCommerce styles a bit, making them a bit more reusable and manageable.

Some week ago I did a PR to change the a font that me me notice that in almost all the custom styles of the templates is used @font-face { when it would be enough to do as it was done in woocommerce.scss using @import "fonts";. This obviously forces extra work, unwanted differentiations between templates etc... not really what we can achieve with scss.

if you need for these things i can help, with scss i'm at home😉! (and not new to this)

barryhughes · 2022-01-25T20:19:09Z

I think we are onboard with the over-arching idea, and see the benefits, but there are also some complexities to figure out (and right now we unfortunately don't have ready-made answers, and are prioritizing other pieces of work).

Note a finished thought, but perhaps there is scope to switch to HSL color values as a way to make this easier to manage? However, we would need input from someone with more experience in this area (and, as ever, we'd need to consider backwards compatibility and the relative comfort of third-party developers and customizers):

:root {
    /* Define the HSL components of our key colors */
    --signature-hue: 302;
    --signature-saturation: 58.9%;
    --signature-lightness: 64.7%;
    
    /* Assemble them into shorthands */
    --signature-color: hsl( var(--signature-hue), var(--signature-saturation), var(--signature-lightness) );
    --signature-color-dark: hsl( var(--signature-hue), var(--signature-saturation), calc( var(--signature-lightness) * 0.8 ) );
}

/* Using pre-defined shorthands */
#a { background: var(--signature-color); }
#b { background: var(--signature-color-dark); }

/* "One-off" variations */
#c { background: hsl( var(--signature-hue), var(--signature-saturation), calc( var(--signature-lightness) * 1.2 ) ); }

☝🏽 For instance, in the above example, we can imagine Orchid (#DA70D6) having been set as our signature color via the theme customizer. We'd then take care of breaking this down into its consituent HSL values and output them at some appropriate point. From there, we could assemble more convenient composite values (like --signature-color).

This might help to cleanly solve some of the issues with the huge number of darkness-based variations. On the other hand, perhaps a design review would reveal that we can easily reduce the range of colors we are using by default.

tl;dr—thank you for the work and ideas, and please continue to develop this if you feel so inclined: owing to other pressures, though, we might not be able to give this our full focus until some later point.

erikyo · 2022-01-26T17:58:05Z

I really like this approach to generate tones variations and personally I love hsl colours but I know sometimes they are not very appreciated by users who find them complicated to use. In addition, WordPress guys have decided to use rgb and rgba so maybe it would be practical to align their choice. Let me know your thought about.

But, assuming the methods for creating tonal variations and transparencies works, isn't it worth making this leap forward?
Here is a summary of the colours used in woocommerce.scss which, if we exclude those duplicated or used once or twice, can be decimated. The situation is much better in template styles like twenty-twenty-one where themes custom properties were used and only a little of wc related colors are still there. The tones that require shades (primary, white to black, green, red, blue and maybe yellow) or opacity (--black: 0; and 132 or calc(var(--black) + 132)) are very few I think it is all more than feasible.

These are my thoughts on how colour variables should be used, but totally personal
Depends on in what manner you want to set the style of WooCommerce because as a plugin should provide abstract variables to be filled with colours and not a new color palette.
Let me explain: in my opinion --wc-orange isn’t correct because we don't need a new colour (is a wordpress related thing imho). Red will be used both for errors and for a "hot sale" label, green for accept buttons and for in-stock labels with the result that when I want to change a color I can’t change the variable because this will change everything that has that colour. In my opinion it's more correct to create --wc--alert or --wc--button--color that is filled with a color or custom prop depending on case. This also makes the stylesheet more intelligible because it would be like --wc--notice--alert: red or --wc--notice--success: green

barryhughes · 2022-01-26T18:28:45Z

Thanks for following-up 🙃

I know sometimes they are not very appreciated by users who find them complicated to use.

That is definitely a concern I share (and, personally, I don't find I can easily picture what a color looks like if I am given its HSL value, whereas an RGB or it's hex equivalent are quite easy to transform in my mind...but, perhaps for others the opposite is true).

WordPress guys have decided to use rgb and rgba so maybe it would be practical to align their choice.

Also agree in the sense that consistency is important. Given that and my last comment it probably seems odd I suggested an HSL-based solution at all, but the core reason was to solve the technical dilemma whereby:

We still use SASS functions, but in at least some cases they are incompatible with CSS variables.
Pure CSS alternatives to those functions exist, but mostly only work with HSL values (not RGB, RGBA, Hex, etc).

In my opinion it's more correct to create --wc--alert or --wc--button--color that is filled with a color or custom prop depending on case.

💯 ... more of this (--wc-highlight) and less of this (--wc-green) makes sense to me.

Depends on in what manner you want to set the style of WooCommerce because as a plugin should provide abstract variables to be filled with colours and not a new color palette.

So does the following sound like an accurate summary of what you are suggesting (or, if I'm misinterpreting, could you post a revised summary)?

We favor color variables named by purpose ('alert', not 'red').
We describe them using RGB and RGBA notation.
For variants (think 'alert-dark' or 'alert-light') we define those directly, unless suitable pure CSS functions are available.

erikyo · 2022-01-26T18:45:28Z

Yes, everything is correct! i did not mention that the colours could be provided by theme.json but this is a later thing that can be added afterwards.

erikyo · 2022-01-27T22:40:08Z

So as far as I can understand there are therefore two possible ways:

Use rgb but this implies that the colour variations are pre-generated. Like here. RGB can't have “fast” functions to desaturate or change lightness, rgb colours don't allow this unless we do complex mathematical calcs which I think would be a mess to do live while painting the page
Give up and use hsl colours, this would make everything easy and immediate. I know I said last time that it was preferable to use rgb (and I still think so at least for the WordPress environment) but after looking at all the styles more in depth an how they are designed, I guess it is inevitable (but as you said it could be an opportunity to optimize the palette or standardise some colors).
You may plan to switch from hsl to rgb later when css5 level 5 will be widespread, and the problem will solve itself with relative rgb colors https://www.w3.org/TR/css-color-5/#relative-RGB

I prepared a scss function which I think could solve the problem, please have a look at it

if it seems acceptable to you as a solution to use scss/hsl I can convert all the styles into short! let me know!

Can I tell you the trick of how to visualize hsl colours rapidly? First think that its representation is not a cube like rgb but a cylinder. The first parameter (the radius, so it's a wheel) defines the tone: 0 is red, 120 is green and 240 is blue (around this values, before and after of course). The next two parameters define saturation (from grey to coloured) and brightness (from white to black). It's really handy and when you get the hang of this, and it's much more intuitive than rgb. I know without having to check that hsl(60, X, Y) is yellow because it is between red and green or that hsl(180, 100%, 80%) will be a light blue because it is near the blue value and with an high saturation and lightness.

barryhughes · 2022-01-27T23:13:23Z

I prepared a scss function which I think could solve the problem

Hopefully I've not skipped part of your solution, but I think the core problem here is that SCSS/SASS is an ahead-of-time technology (OK, that may not be strictly true as a hard rule, but that is how it is currently used in this project), and so the result of calls to any SCSS functions is, in effect, going to be a bunch of hardcoded color values.

Whereas, at least as I understood it, what @helgatheviking was shooting for (and what I think has a lot of value) is a situation where true CSS variables can be used to define a base palette (which can be dynamically set through interactions with the theme customizer).

I realize it's possible to make SASS understand CSS variables, but even then we'd lose the dynamic component of the solution.

Can I tell you the trick of how to visualize hsl colours rapidly?

That is an awesome explanation, thank you 🙃

erikyo · 2022-01-27T23:24:47Z

Sorry but i guess I've not explained the code properly, because what the sass function actually does is create the custom properties with the hsl colours as you showed me, and not use static scss vars as before. And what is generated is completely dynamic and no longer dependent on sass that was used to emulate it's functions but dinamically, give it a try!

In order to set the main color and its variations:

	--wc-hue: 312;
	--wc-hue--highlight: calc(var(--wc-hue) - 150);

	--wc-sat: 39%;
	--wc-sat--secondary: calc(var(--wc-sat) - 21%);

	--wc-luma: 64%;
	--wc-luma-white: 100%;

note --wc-hue and --wc-hue-highlight were used for two different color, as --wc-sat--secondary has been used to define the secondary color.

/**
 * This is intended to replace .variables.scss 
 * WooCommerce CSS Variables (not needed in this example but still needed to compile styles)
 */
$woocommerce:   	#a46497 !default;
$green:         	#7ad03a !default;
$red:           	#a00 !default;
$orange:        	#ffba00 !default;
$blue:          	#2ea2cc !default;

$primary:           #a46497 !default;                                  // Primary color for buttons (alt)
$primarytext:       desaturate(lighten($primary, 50%), 18%) !default;    // Text on primary color bg

$secondary:         desaturate(lighten($primary, 40%), 21%) !default;    // Secondary buttons
$secondarytext:     desaturate(darken($secondary, 60%), 21%) !default;   // Text on secondary color bg

$highlight:         adjust-hue($primary, 150deg) !default;               // Prices, In stock labels, sales flash
$highlightext:      desaturate(lighten($highlight, 50%), 18%) !default;  // Text on highlight color bg

$contentbg:         #fff !default;                                     // Content BG - Tabs (active state)
$subtext:           #767676 !default;                                  // small, breadcrumbs etc

@function set_value($val, $val2) {
	@if (type-of($val) == string) {
		@return calc(#{$val} + #{$val2});
	} @else {
		@return $val + $val2;
	}
}

@function gen-custom-prop($color, $hue: false, $saturation: false, $luminosity: false, $alpha: false) {

	@if (type_of($color) == string) {
		@error("Error in _variables.scss: colors needs to be a list like 150, 100%, 50% and not " + $color)
	}

	$h: nth($color, 1);
	$s: nth($color, 2);
	$l: nth($color, 3);

	@if($hue != false) {
		$h: set_value($h, $hue);
	}

	@if($saturation != false) {
		$s: set_value($s, $saturation);
	}

	@if($luminosity != false) {
		$l: set_value($l, $luminosity);
	}

	@if($alpha != false) {
		@return unquote("hsla(#{$h}, #{$s}, #{$l}, #{$alpha})");
	}

	@return unquote("hsl(#{$h}, #{$s}, #{$l})");
}

// primary color
$wc-primary: var(--wc-hue), var(--wc-sat), var(--wc-luma);
$wc-primary-text: var(--wc-hue), calc(var(--wc-sat) - 18%), calc(var(--wc-luma) + 50%);

// secondary color
$wc-secondary: var(--wc-hue), var(--wc-sat--secondary), calc(var(--wc-luma) + 40%);
$wc-secondary-text: var(--wc-hue), var(--wc-sat--secondary), calc(var(--wc-luma) - 60%);

// highlight
$wc-highlight: var(--wc-hue--highlight), var(--wc-sat), var(--wc-luma);
$wc-highlight-text: var(--wc-hue--highlight), calc(var(--wc-sat) + 18%), calc(var(--wc-luma) + 50%);

// export vars as CSS vars
:root {
        // following 6 props are intended to be set dynamically
	--wc-hue: 312;
	--wc-hue--highlight: calc(var(--wc-hue) + 150);

	--wc-sat: 26%;
	--wc-sat--secondary: calc(var(--wc-sat) - 21%);

	--wc-luma: 52%;
	--wc-luma-white: 100%;

	--woocommerce: #{$woocommerce};
	--wc-green: #{$green};
	--wc-red: #{$red};
	--wc-orange: #{$orange};
	--wc-blue: #{$blue};


	--wc-primary: hsl(#{$wc-primary});
	--wc-primary-text: hsl(#{$wc-primary-text});
	--wc-secondary: hsl(#{$wc-secondary});
	--wc-secondary-text: hsl(#{$wc-secondary-text});
	--wc-highlight: hsl(#{$wc-highlight});
	--wc-highlight-text: hsl(#{$wc-highlight-text});

	--wc-content-bg: hsl(0,0%,var(--wc-luma-white));
	--wc-subtext: hsl(0,0%,calc(var(--wc-luma-white) * .46));
	--wc-black: hsl(0,0%,0%);

	// tests
	--woo-color-0:  #{gen-custom-prop( $wc-primary )};
	--woo-color-1:  #{gen-custom-prop( (1,2%,3%) )};
	--woo-color-2:  #{gen-custom-prop( (1,2%,3%) , 1, 2%, -10%, .5 )};
	--woo-color-3:  #{gen-custom-prop( ( var(--wc-hue), 10%, 50% ) )};
	--woo-color-4:  #{gen-custom-prop( $wc-primary, -150 )};
	--woo-color-5:  #{gen-custom-prop( $wc-primary, false, -10% )};
	--woo-color-6:  #{gen-custom-prop( $wc-primary, false, false, 10% )};
	--woo-color-7:  #{gen-custom-prop( $wc-primary, false, false, false, 50% )};
}

ps. I may be biased, but I don't think that SCSS/SASS will ever be abandoned: since it is a turing complete language it is definitely something else compared to what css is (or will be). I have always intended scss as a process parallel to css that on a large project allows you to keep everything maintainable and with neat code.

erikyo · 2022-01-28T18:02:32Z

you can try it here changing the --color-hue / --color-sat / --color-luma and all colors will change dynamically. This will work also for alternate colors generate with the SCSS function (that works destructuring and rebuilding colors with calc() or executing the calculation with values nothing more)
https://codepen.io/erikyo/pen/ExbjpYg

which can be dynamically set through interactions with the theme customizer

I don't know if this is a good idea because the customiser will probably be abandoned. Anyway in the past I have done this "error" and it works with custom properties, see here. But ok, it would need a style configuration panel that allows to choose a primary secondary and highlight color (as it is done for WooCommerce mails).

Anyway I want to mention this changes allow the possibility to override very easy the whole WooCommerce color palette without anything other. For example I will be able to set the primary colour to red using css only just adding :root{--wc-hue: 0;} to my stylesheet. Or I can change the checkout page highlight color with .page.checkout{--wc-hue--highlight: 0;} etc. I think the advantages it brings are undeniable!

@helgatheviking, what do you think about those changes?

barryhughes · 2022-01-28T22:36:09Z

Sorry @erikyo, I obviously wasn't paying enough attention first time round (the codepen was helpful, though, so thank you). This looks like a nice approach.

I don't know if this is a good idea because the customiser will probably be abandoned.

Yes, true—not the best example perhaps—but my point was rather about keeping things dynamic such that if the basic palette is changed (by altering one of the core CSS variables), be that through interaction with the theme customizer or something else, everything else should change accordingly without re-compiling the stylesheet (and it looks like your solution solves this very thing).

For example I will be able to set the primary colour to red using css only just adding :root{--wc-hue: 0;} to my stylesheet. Or I can change the checkout page highlight color with .page.checkout{--wc-hue--highlight: 0;} etc. I think the advantages it brings are undeniable!

Agreed: I like this a lot.

I'm flagging for discussion just so I can check in with the team and see if anything else is needed here (this should also give an opportunity for others to chime in), but I think we're on to something good here: thanks for all the work and discussion so far—hugely appreciated 👍

erikyo · 2022-01-29T11:22:09Z

hi @barryhughes, very happy to hear your feedback so, starting with what was already there took me very little time to update and get the style built! So that the discussion can start with something that works.
https://github.com/erikyo/woocommerce/tree/css-custom-prop

I also created a branch with the update to dart-sass... now it's the mainstream one and if I wanted to use/include some styles in my own sheet this change would ensure that woo styles are compatible
trunk...erikyo:renovate/dart-sass

helgatheviking · 2022-01-29T14:46:48Z

I love where this convo is going! It's a bit beyond me now, but yeah purpose-named color variables would be great. where I was getting stuck was on Woo's current use of darken and lighten but it seems like you've solved that, which is awesome.

erikyo · 2022-01-29T15:28:46Z

This is the rebased version but I am not able to add it to this pull request 😅 Anyway could be the starting point for an initial testing.

I love where this convo is going!

me too! Honestly, I had never thought about using hsl but @barryhughes's suggestion (and necessity) made me try that way. I think it's a good balance until there are relative rgb colours, then "maybe" it will be time to align with WordPress.

With this in mind, I changed the base colours and used those of the block-editor (with fallback on the previous statically defined ones). I had commented on an issue on wordpress to make available the --wp--preset--success or --wp--preset--error etc variables but with no luck.

helgatheviking · 2022-01-29T15:45:29Z

if i had to guess, we'll need to close this PR and move to yours.

erikyo · 2022-01-29T16:00:03Z

@helgatheviking could you cherry-pick my changes?

helgatheviking · 2022-01-29T21:41:33Z

I tried my best! If that's not good for any reason, I think it may be easier for you to submit a new PR?

erikyo · 2022-01-29T23:12:43Z

More than perfect! Thanks and sorry for the trouble but at least in this way everything remains in this pull request that you have opened and that deserves to be closed by you! thank you for your efforts!
In case there are any problems I will make a pull request directly to your fork, this morning I had rebased the whole styles to the head of trunk and it would be a shame to lose it. sorry 😅

Later the stylesheet can be improved and optimized, these fixes I have done are a fix to build the style but for example all the colors such as black and white still need to be replaced.

erikyo · 2022-01-30T13:51:54Z

I updated in my branch the scss style to make it compatible with the version used here on WooCommerce repo (sass 1.45.0 which is dart-sass but probably an earlier version than the one I was using). I've changed a little the style so it can be compiled also with node-sass and now it's 100% compatible with both. This change has forced me to introduce two new variables "--wc-luma--secondary-text" and "--wc-luma--secondary" but I think they are very convenient for user styling.

I've also updated the WooCommerce style into my blog, you can try to change the variables with devTools, and you'll notice that the colours of the buttons, price, tabs and labels are going to change automatically and live (and it's really cool because I no longer need to overwrite woocommerce.css to colour the shop elements).

- updated style - adds a rule to cssmin in order to avoid color "minification" - add a new file dedicated to custom properties - enqueue the stylesheet with custom props into header inline style

erikyo · 2022-02-11T15:55:57Z

hi @helgatheviking and @barryhughes

just pushed an update that completes the transition to hsl, let me know your thoughts...
I created for convenience two other functions of sass wc-hsl and wc-shade that serve as a proxy for the generation of colors so that if in the future you want to change you can do because they are written in a standard way.
To replace all the colors I had to use a loop that regex match the hex color (or rgb/rgba) and replace it with the one provided, but this made me count that were used about 140 different colors some really similar (there was the whole range from fff to f0f0f0 and each step has only 0.3% tonal variation, more or less is nothing especially since not all monitors have this accuracy).

The only real change I made to the hue of some colors because with rgb keep a constant hue it's very difficult but with hsl not (and that why imho hsl are very handy - see screenshot). all blue colors are linked to the variable with the hue value of blue, red ones with var(-wc--red-hue) etc

This should explain the negative point of using rgb for theming, when it comes to colors shades hex/rgb/rgba are not very precise and always have a certain tonal variance. If you need a dark purple with rgb creating a mathematical formula will drive you crazy (it should be -20, -5, -30) and probably you deviate from the original hue. With hsl i will keep the hue constant and i will just subtract a value from the original brightness. The only issue is that with FSE and theme.json hsl colors aren't 100% supported (or at least, this is what is documentated because i actually made a test fse/woo theme and it seems to work even if it has hsl colors)

As the custom prop stylesheet I've added it to the already inlined by woocommerce (using file_get_contents tell me is there are security issues with that), in order inject the css variables in the header. i've added a hook "enqueue_woo_global_styles" (similar to the wordpress one) in order to let the user to modify or return false.
This inline style would also be very good with the shop structure so as to have a minimum of above the fold style that immediately "blocks" the containers, avoiding shifting.

last and least, edited the grunt config in order to NOT minify colors. This would not really be necessary but with this hsl approach it becomes a bit out of place to use rgb.

I think it's a good starting point to rationalize the style and make it uniform, I've already removed a few errors and I've verified that the painting has not slowed down. Obviously this will make it easier in the future but for the moment it is a mere fantasy since isn't working yet on Canary Chrome.

barryhughes · 2022-02-15T19:00:11Z

@erikyo—would you be happy to create a fresh PR based on your own branch? We can link back to this one for context, and it will make review a little easier.

barryhughes · 2022-03-11T00:25:20Z

Closing in favor of #31902 (thanks for creating that—and apologies for the further delay, there are a few high priority items we're tackling first).

erikyo · 2022-03-11T09:52:41Z

👌no problem at all!

When you are ready I've some changes to the frontend also for the performance concern, please take in charge also this pr, will avoid to use flexslider to generate thumbnail images improving the CLS

fix for flexslider cls https://github.com/Mte90/woocommerce/tree/flexslider-custom-nav-no-template

helgatheviking force-pushed the issues/29574-css-color-variables branch from 858fe3c to 3822161 Compare April 2, 2021 16:50

claudiosanches reviewed Apr 21, 2021

View reviewed changes

claudiosanches suggested changes Apr 21, 2021

View reviewed changes

helgatheviking force-pushed the issues/29574-css-color-variables branch from 3822161 to aca7c28 Compare April 22, 2021 01:14

helgatheviking changed the base branch from trunk to master April 22, 2021 01:15

Swap SCSS variables for CSS variables. Closes woocommerce#29574.

baac6f0

(cherry picked from commit aca7c28)

helgatheviking force-pushed the issues/29574-css-color-variables branch from aca7c28 to baac6f0 Compare April 22, 2021 01:18

helgatheviking changed the base branch from master to trunk April 22, 2021 01:19

barryhughes requested changes May 4, 2021

View reviewed changes

Resolve rogue trailing colons from :root

6dd19f1

Repair wc-primary-text typo

94b0304

barryhughes added the needs: discussion Issue that either needs discussion or pending decision (not for support requests). label Jan 28, 2022

Cherry pick 322090c from @erikyo

e4ffc97

barryhughes mentioned this pull request Feb 1, 2022

[feature] Theming WooCommerce SASS (variables) #25873

Closed

erikyo mentioned this pull request Feb 15, 2022

Feature/css custom props - followup #29575 #31902

Closed

ObliviousHarmony added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Feb 21, 2022

barryhughes closed this Mar 11, 2022

Issues/29574 css color variables #29575

Issues/29574 css color variables #29575

Conversation

helgatheviking commented Apr 2, 2021

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Changelog entry

ninetyninew commented Apr 4, 2021 • edited Loading

claudiosanches Apr 21, 2021

Choose a reason for hiding this comment

claudiosanches Apr 21, 2021

Choose a reason for hiding this comment

claudiosanches left a comment

Choose a reason for hiding this comment

helgatheviking commented Apr 22, 2021 • edited Loading

barryhughes left a comment

Choose a reason for hiding this comment

barryhughes May 4, 2021

Choose a reason for hiding this comment

barryhughes May 4, 2021

Choose a reason for hiding this comment

barryhughes May 4, 2021

Choose a reason for hiding this comment

barryhughes May 4, 2021

Choose a reason for hiding this comment

helgatheviking commented May 4, 2021 • edited Loading

peterfabian commented Oct 13, 2021

barryhughes commented Oct 13, 2021

helgatheviking commented Oct 13, 2021

claudiosanches commented Oct 13, 2021

peterfabian commented Oct 15, 2021 • edited Loading

helgatheviking commented Oct 15, 2021 • edited Loading

erikyo commented Jan 25, 2022 • edited Loading

barryhughes commented Jan 25, 2022

erikyo commented Jan 26, 2022

barryhughes commented Jan 26, 2022

erikyo commented Jan 26, 2022

erikyo commented Jan 27, 2022 • edited Loading

barryhughes commented Jan 27, 2022 • edited Loading

erikyo commented Jan 27, 2022 • edited Loading

erikyo commented Jan 28, 2022

barryhughes commented Jan 28, 2022

erikyo commented Jan 29, 2022 • edited Loading

helgatheviking commented Jan 29, 2022

erikyo commented Jan 29, 2022 • edited Loading

helgatheviking commented Jan 29, 2022

erikyo commented Jan 29, 2022

helgatheviking commented Jan 29, 2022

erikyo commented Jan 29, 2022

erikyo commented Jan 30, 2022 • edited Loading

erikyo commented Feb 11, 2022 • edited Loading

barryhughes commented Feb 15, 2022

barryhughes commented Mar 11, 2022

erikyo commented Mar 11, 2022 • edited Loading

ninetyninew commented Apr 4, 2021 •

edited

Loading

helgatheviking commented Apr 22, 2021 •

edited

Loading

helgatheviking commented May 4, 2021 •

edited

Loading

peterfabian commented Oct 15, 2021 •

edited

Loading

helgatheviking commented Oct 15, 2021 •

edited

Loading

erikyo commented Jan 25, 2022 •

edited

Loading

erikyo commented Jan 27, 2022 •

edited

Loading

barryhughes commented Jan 27, 2022 •

edited

Loading

erikyo commented Jan 27, 2022 •

edited

Loading

erikyo commented Jan 29, 2022 •

edited

Loading

erikyo commented Jan 29, 2022 •

edited

Loading

erikyo commented Jan 30, 2022 •

edited

Loading

erikyo commented Feb 11, 2022 •

edited

Loading

erikyo commented Mar 11, 2022 •

edited

Loading