Style Engine: add typography and color to backend

[ramonjd]

Tracking Issue: #38167

What?

An alternative to

• Style Engine: add typography to backend #40082

that takes into account classname generation from slugs, as well as required classnames such as has-text-color, which is applied to blocks even with a custom text color.

Why?

We need to cater for block support classnames that both:

• require slug substitution
• should display regardless of whether there is preset or inline CSS

How?

By accepting an array for the block style value that contains metadata.

Example usage:

$block_styles = array(
    'color' => array(
        'text'       => '#ccc',
        'background' => 'var:preset|background-color|burned-toast',
    ),
    'typography' => array(
        'fontSize' =>'var:preset|font-size|biiiiiig',

    ),
    'spacing' => array(
        'margin' = array(
            'top'    => '12rem',
            'left'   => '2vh',
            'bottom' => '2px',
            'right'  => '10em',
        ),
    ),
);


$styles = gutenberg_get_style_engine()->generate( block_styles );

/*
$styles outputs:

array(
    'css' => 'color: #ccc; margin-top: 12rem; margin-left: 2vh; margin-bottom: 2px; margin-right: 10em;',
    'classnames' => 'has-text-color has-background has-burned-toast-background-color has-biiiiiig-font-size',
)
*/

Testing Instructions

For dynamic blocks (Site Title, Navigation, Post Title, Post Date for example), check that the color, spacing and typography block supports CSS and classnames are output correctly on the frontend.

Some example block code (using TwentyTwentyTwo theme presets)

<!-- wp:site-title {"style":{"typography":{"lineHeight":1.7,"textTransform":"uppercase","letterSpacing":"113px","textDecoration":"line-through"},"color":{"background":"#c2c9d0"},"spacing":{"padding":{"top":"130px","right":"130px","bottom":"130px","left":"130px"},"margin":{"top":"133px","right":"133px","bottom":"133px","left":"133px"}},"elements":{"link":{"color":{"text":"var:preset|color|secondary"}}}},"textColor":"secondary","fontSize":"large","fontFamily":"source-serif-pro"} /-->

<!-- wp:navigation {"ref":50,"layout":{"type":"flex","orientation":"horizontal"},"style":{"typography":{"textTransform":"lowercase","textDecoration":"line-through","fontStyle":"italic","fontWeight":"100"}},"fontFamily":"someAwesomeFont-right-here"} /-->

<!-- wp:search {"label":"Search","buttonText":"Search","textColor":"pale-pink"} /-->

<!-- wp:navigation {"ref":50,"textColor":"pale-pink","overlayBackgroundColor":"vivid-purple","overlayTextColor":"luminous-vivid-orange","style":{"typography":{"fontStyle":"italic","fontWeight":"800","lineHeight":3.4,"textDecoration":"line-through","textTransform":"lowercase"}},"fontSize":"large"} /-->

<!-- wp:post-title {"style":{"typography":{"textTransform":"uppercase","fontStyle":"italic","fontWeight":"800","fontSize":"52px"},"spacing":{"margin":{"bottom":"137px"}},"elements":{"link":{"color":{"text":"#e13f3f"}}},"color":{"text":"#ea2020"}},"backgroundColor":"foreground"} /-->

<!-- wp:post-date {"style":{"typography":{"letterSpacing":"77px"}},"fontSize":"medium"} /-->

<!-- wp:post-date {"style":{"typography":{"letterSpacing":"-5px","fontStyle":"italic","fontWeight":"300","lineHeight":6.1,"textTransform":"lowercase"}},"gradient":"pale-ocean","fontSize":"x-large","fontFamily":"system-font"} /-->

Run npm run test-unit-php /var/www/html/wp-content/plugins/gutenberg/packages/style-engine/phpunit/class-wp-style-engine-test.php

[youknowriad]

Thanks for working on this. Looking at the PR description makes me wonder, why didn't we just use the theme.json format instead of adding a new one. In other words, instead of doing

    'color' => array(
        'background' => array( 'slug' => 'burned-toast' ),
    ),

why not

    'color' => array(
       'background' => 'var:preset|color|burned-toast' 
   )

and pass the settings object as a second argument.

// The settings here is very similar to theme.json settings.
const settings = { color: { presets: [ ... ] };

generate( styles, settings );

Something like this will allow us to address block supports, theme.json style generation and the upcoming "sections" block in the same way I think. The difference here with what we have is the addition of the "settings" object to the API. It feels natural addition though.

Anyway, this is a great PR to move things forward a bit and it's also very relevant to our upcoming work in the patterns/sections blocks. cc @jorgefilipecosta @oandregal @mcsf

Edit: It seems we don't need the "settings" object to generate the right styles. Since we only need the slug, so we can leave that out for later if necessary.

[youknowriad]

The other thing I'm wondering is whether the style engine should return the "class" property or just use the CSS variable inside the "style" property (not change the output format).

I know this is a change compared to what we already do though. and I'm not arguing that we should remove these classnames, but to more consider them as separate from the style engine. In other words, classes we may add, are added to describe the "state" of the block but not necessary its "style". I hope that makes sense.

Another thing to think about is whether the "presets" styles generation need to be part of the style engine. I'm not convinced personally yet, especially since it needs to know about block registration... but I think it's something we can think of and consider later.

[ramonjd]

Thanks @youknowriad

As always your comments are very helpful. I'll definitely experiment with the preset pattern string argument.

why not

    'color' => array(
       'background' => 'var:preset|color|burned-toast' 
   )

No reason why not 😆 I feel a bit face-palmed not having thought of it myself!

Another thing to think about is whether the "presets" styles generation need to be part of the style engine.

Kind of related to the above example, I wasn't sure to what extent the style engine should be aware of specific Gutenberg/theme.json implementation. The classnames logic, however, probably moves the needle in the direction of "yes, it needs to know how to build classnames from settings".

The style engine is a close-knit member of the Gutenberg family so I suppose some tight coupling can be tolerated 😄

Maybe the limits of this coupling could be backwards compatibility. My view (so far) is that the plugin should always take care of that rather than expect the style engine to know about these things. The example is the backwards compat method in typgraphy.php.

Anyway, it's given me plenty of things to think about and experiment with. Thanks again! 🙇

[ramonjd]

why didn't we just use the theme.json format instead of adding a new one.

Why not indeed! I'm testing this out in a369005

The other thing I'm wondering is whether the style engine should return the "class" property or just use the CSS variable inside the "style" property (not change the output format)

This is an interesting idea, and something that relates to the work @glendaviesnz is looking into for

• Use a var for user presets to avoid having to use ! important to enforce them #40335

I suppose we'd still need to output the classnames for backwards compatibility (?) since themes might be relying on them for overriding.

[oandregal]

Hey, catching up a bit. Perhaps this was already discussed in previous PRs, sorry if that's the case. Happy to learn up.

I'm about to look at #40318 in the following days. The goal is to allow sections to declare its own settings (presets, but also things like enable/disable blockGap, I presume). My first instint is: why not using the existing theme.json code that already has all that logic?

Looking at this issue and comments (not the actual PR contents), I see the following:

$styles = array(
    'color' => array(
        'text'       => '#ccc',
        'background' => 'var:preset|background-color|burned-toast',
    ),
    'typography' => array(
        'fontSize' =>'var:preset|font-size|biiiiiig',

    ),
    'spacing' => array(
        'margin' = array(
            'top'    => '12rem',
            'left'   => '2vh',
            'bottom' => '2px',
            'right'  => '10em',
        ),
    ),
);
// The settings here is very similar to theme.json settings.
$settings = array( color => ... );

gutenberg_get_style_engine()->generate( $styles, $settings );

I wonder if we could do this instead:

$settings = array( ... );
$styles = array( ... );
$theme_json = array(
  'styles' => $styles,
  'settings' => $settings
);

$theme_json = WP_Theme_JSON( $theme_json);
$stylesheet = $theme_json->get_stylesheet( array('styles') );

In a first pass, it seems like the "style engine" could be a light wrapper on top of the existing theme.json code, hiding those details for the block supports code. A blocker I see to do this is that we've inlined layout/alignment styles in this code, which is only relevant for "global styles" but not for "block styles". It should be doable to move it elsewhere.

Do you have thoughts on this approach?

[aaronrobertshaw]

Nice work @ramonjd. The style engine looks to be coming along well 👌

I've taken this for a quick spin and it is working as advertised for me.

✅ Unit tests pass
✅ Correct inline styles and classes are applied to the frontend
✅ Dynamic blocks that skip serialization and manually apply styles/classes continue to work

I left a few minor questions and one comment highlighting that we might need to update get_css_rules to handle border radius styles as they won't fit the $style_property-$key pattern.

[aaronrobertshaw]

Would there be an opportunity to create a util function somewhere to reduce the repetition of this code across each block support?

[ramonjd]

For sure. I did consider making it a part of the style engine itself, but it seemed too block support-specific. I went with classnames and css as properties because I think they better describe the return values. Well, at lest better than the class and style properties.

[aaronrobertshaw]

Is this still an open question or has the testing been completed?

[ramonjd]

I've been testing this and can't yet see any issues. Though I've been tossing up whether to reinstate this function as it is on trunk entirely. There's some code repetition, but it might save a migration headache.

[aaronrobertshaw]

This will need tweaking to be able to handle border-radius values e.g. border-top-right-radius. That is, if we wish to make good on the docblock comment.

[ramonjd]

Definitely. My next "experiment" was going to use this branch as a base to include border support. I reckon a few things will need to change.

[youknowriad]

In a first pass, it seems like the "style engine" could be a light wrapper on top of the existing theme.json code, hiding those details for the block supports code. A blocker I see to do this is that we've inlined layout/alignment styles in this code, which is only relevant for "global styles" but not for "block styles". It should be doable to move it elsewhere.

@oandregal It's clear that theme.json stylesheet generation and block-style stylesheet generation are the same things and should share similar logic. I think the reasoning so far is that we shouldn't tie block supports logic to theme.json though (introduces too many unnecessary dependencies). So the idea is to have a generic "style engine" and use it in both "theme.json" and "block supports". And the style engine would be stateless/dependency less.

So basically what you're suggesting, but the other way around: theme.json is the wrapper on top of style engine.

[cbirdsong]

The other thing I'm wondering is whether the style engine should return the "class" property or just use the CSS variable inside the "style" property (not change the output format).

I would discourage this, as classes can be used as cues for other styling. In my themes I use a pattern like this to automatically set the text color if one is not explicitly set:

:where(.has-dark-background-color, .has-other-dark-background-color, .etc):not(.has-text-color) {
  color: white;
}

Inline styles are also incompatible with a strict content security policy.

[youknowriad]

To be clear my suggestion is not to remove these classes from blocks :). It's to remove the classes from the output of the style engine. The classes would remain in the blocks but to indicate the block state but they won't be used for styling in Core.

[ramonjd]

Thanks again for all the feedback!

In a first pass, it seems like the "style engine" could be a light wrapper on top of the existing theme.json code, hiding those details for the block supports code.

It might be worth trying this out just to see. Block supports do contain a fair amount of unique logic, and I'm not sure how cleanly the style engine could subsume it.

There might be a few use cases in the future where we'd want the style engine to accept theme.json settings as an argument regardless. Presumably, with all style information the style engine could organize and reconcile styles right across the app. Furthermore, (and imagining for a moment there are no other constraints) we could output a single stylesheet containing all styles with this information.

What I think the current iteration is trying to achieve is to get the block-level styles working before we move backwards (or upwards) from block supports to sections and global styles. Layout will be a cognitive challenge. And there are conundrums such as the one raised in https://github.com/WordPress/gutenberg/pull/39870/files#diff-324697e41855298e2f2c74b078f174e0cbc9075cef736ce9c1e2c169bf64652eR182 where we're trying use values from global styles at the block support level.

The solutions I hope will become more apparent as we work our way through the style "levels" and gather requirements and knowledge to inform the next steps.

[ramonjd]

Adding border support based on this code over at:

• Style Engine: add border to backend #40531

Border throws a few unique scenarios into the mix and I've had to rejig the data structures and custom value callbacks.

[jorgefilipecosta]

Hi @ramonjd this PR seems ready and is on a good path 👍 It is required by the section-specific theme.json #40318. How can we help this PR being merged? Is there any pending discussion that we need to decide on or it just needs a final review and a green approval?

[ramonjd]

How can we help this PR being merged? Is there any pending discussion that we need to decide on or it just needs a final review and a green approval?

Thanks @jorgefilipecosta !! 🙇

As an iteration I also think it's in decent shape.

Internally (not in the API), things will change a little bit as I add border supports over here:

• Style Engine: add border to backend #40531

So we can also continue to iterate in that PR or subsequent ones.

[jorgefilipecosta]

Hi @ramonjd,

I'm trying to review this PR. As a user there is no difference e.g: there is not something I can do on this branch that I could not do on trunk right? This is just an engine change not noticeable to the user?

If I paste the following block on the code editor:

<!-- wp:site-title {"style":{"color":{"background":"var:preset|color|luminous-vivid-orange"}}} /-->

On the editor view, I see the orange color, (the front end style engine supported already this format) but if I save the post and load the frontend view the background is not applied. With this PR shouldn't the background be applied?

[jorgefilipecosta]

From the API point of view, I guess we could do

$styles       = gutenberg_style_engine_generate( array( 'color' => $color_block_styles ) );
or 
$styles       = gutenberg_style_engine_get_styles( array( 'color' => $color_block_styles ) );

And avoid the need to call gutenberg_get_style_engine. That call may be done inside gutenberg_style_engine_generate.

Not directly related to this PR but regarding the class WP_Style_Engine. It seems it will be part of WordPress 6.0 and it's a public class everyone can use and we need to maintain forever. Are we totally confident this class is ready and useful for public usage?
To me this class seems like a helper to a function that generates styles like gutenberg_style_engine_generate or gutenberg_style_engine_get_styles. I guess we could mark for now the class WP_Style_Engine as private with something similar to what we did to the theme.json class:

 * This class is for internal core usage and is not supposed to be used by extenders (plugins and/or themes).
 * This is a low-level API that may need to do breaking changes. Please,
 * use gutenberg_style_engine_get_styles instead.
 *
 * @access private

[ramonjd]

And avoid the need to call gutenberg_get_style_engine. That call may be done inside gutenberg_style_engine_generate.

That's a fair point. I'll look into it 👍 The easier it is to call the better.

It seems it will be part of WordPress 6.0 and it's a public class everyone can use and we need to maintain forever. Are we totally confident this class is ready and useful for public usage?

Good question.

We decided not to port it over for WordPress 6.0, but it's relevant for 6.1.

The JS version of the style engine has been in Gutenberg for some time now, and the backend is trying to mirror its interface and functionality as best as it can.

Theoretically, the only public-facing API method should be gutenberg_style_engine_generate. That method – its arguments and return value – shouldn't need to change.

However innards of the class will be in flux for some time, so I guess the honest answer is "not quite".

To me this class seems like a helper to a function that generates styles like gutenberg_style_engine_generate or gutenberg_style_engine_get_styles.

Yeah, that's a fair way of looking at it. Ultimately, the style engine should be a black box while we're still working on its functionality.

It's tough because we want to iterate, but we also want to make it clear that things are still experimental.

I guess we could mark for now the class WP_Style_Engine as private with something similar to what we did to the theme.json

Great idea, thanks.

We don't really want to expose the implementation details, so it's definitely something we should communicate here.

[ramonjd]

Hi @jorgefilipecosta !

Thanks again for giving this PR your attention 🙇

I'm trying to review this PR. As a user there is no difference e.g: there is not something I can do on this branch that I could not do on trunk right? This is just an engine change not noticeable to the user?

That is correct. All we are doing during the initial phases of the style engine is moving the generation of styles over to the new class.

Nothing should change in terms of how the frontend appears.

So in order to test, aside from making sure the unit tests are okay, is to create a bunch of dynamic blocks and ensure that they appear as expected on the frontend.

On the editor view, I see the orange color, (the front end style engine supported already this format) but if I save the post and load the frontend view the background is not applied.

This is also the behaviour on trunk. Is that a bug?

Should it be background-color: var(--wp--preset--color--luminous-vivid-orange);?

The code responsible for taking care of this is in block-supports/colors.php

At the moment block supports differentiates only between presets as defined in block attributes, e.g.,

<!-- wp:site-title {"backgroundColor":"luminous-vivid-orange"} /-->

And raw (or custom) background color values.

<!-- wp:site-title {"style":{"color":{"background":"#ffffff"}}} /-->

So if the block appears as you've described

<!-- wp:site-title {"style":{"color":{"background":"var:preset|color|luminous-vivid-orange"}}} /-->

The inline style will be style="background-color: var:preset|color|luminous-vivid-orange;"

BUT, it does work a little differently on this PR in that we strip the background-color: var:preset|color|luminous-vivid-orange; on the frontend as it does nothing anyway.

Should it be background-color: var(--wp--preset--color--luminous-vivid-orange);?

With this PR shouldn't the background be applied?

It's something we could take care of in the style engine, definitely assuming it's a requirement (?) what do you think?

[youknowriad]

It's great to see this moving forward. The only thing I'm uncertain of personally is whether retaining classnames in the return value of the style engine is a good thing. I understand that it was done to not change everything now, I think that's fine but it's something that we should evaluate.

For instance, I know that the !important we're battling with right now are the result of mixing "inline styles" and "classnames" and custom block CSS. We'll have more leverage to deal with specificity if all styles were generated in the same way.

[ramonjd]

The only thing I'm uncertain of personally is whether retaining classnames in the return value of the style engine is a good thing. I understand that it was done to not change everything now, I think that's fine but it's something that we should evaluate.

Makes sense. I share this uncertainty. It was the reason why I started off with two methods – one to get the CSS and one for the classnames – in the first pass of this change.

As we build the features out I hope it'll become clearer.

For instance, I know that the !important we're battling with right now are the result of mixing "inline styles" and "classnames" and custom block CSS. We'll have more leverage to deal with specificity if all styles were generated in the same way.

Ultimately returning an object gives us a little more flexibility, even if we decide not to return classnames or to only return classnames 😄 . It's also possible that we discover some other metadata/sugar is required in the return value.

It's hard for me to know until we start tackling layout and, eventually, global styles.

[ramonjd]

Oh! I forgot to say. One thing I'm aware of is that the JS version of generate currently outputs something like .some-selector { margin: 12px; padding: 10px; }.

I think the backend should have this functionality as well via $options.

I'm looking at the JS as the next step in #40665 to see where we have to reconcile any differences.

Thanks again!!

[jorgefilipecosta]

Do we support these in both frontend and backend style engine?

Not yet since we've only added spacing and padding in the frontend. The preset fun kicks in when we deal with colors, typography and borders. 🥳

For colors the backend already supports the style shape and they appear correctly (it always supported them I think) I just need some adjustments for the UI to reflect a color is applied when using a preset in the format "var:", but it is a simple change. So we are not far.