Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Style engine: generate root global styles from global styles object #42143

Closed
wants to merge 6 commits into from
Closed

Style engine: generate root global styles from global styles object #42143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4965b94
Adding a new method to generate global styles: wp_style_engine_genera…
ramonjd Jul 5, 2022
78e79d7
Allowing for custom properties
ramonjd Jul 8, 2022
e8a0dd0
Created a valid custom property check
ramonjd Jul 11, 2022
629e8c6
Clean up.
ramonjd Jul 11, 2022
31ef9cb
Reverting emptytheme theme.json
ramonjd Jul 11, 2022
5cc7119
Removing blockGap support at the root level after the changes in http…
ramonjd Jul 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,19 +617,16 @@ public function get_stylesheet( $types = array( 'variables', 'styles', 'presets'
* @return string Styles for the block.
*/
public function get_styles_for_block( $block_metadata ) {
$node = _wp_array_get( $this->theme_json, $block_metadata['path'], array() );

$node = _wp_array_get( $this->theme_json, $block_metadata['path'], array() );
$selector = $block_metadata['selector'];
$settings = _wp_array_get( $this->theme_json, array( 'settings' ) );

// Get a reference to element name from path.
// $block_metadata['path'] = array('styles','elements','link');
// Make sure that $block_metadata['path'] describes an element node, like ['styles', 'element', 'link'].
// Skip non-element paths like just ['styles'].
$is_processing_element = in_array( 'elements', $block_metadata['path'], true );

$current_element = $is_processing_element ? $block_metadata['path'][ count( $block_metadata['path'] ) - 1 ] : null;

$is_processing_element = in_array( 'elements', $block_metadata['path'], true );
$current_element = $is_processing_element ? $block_metadata['path'][ count( $block_metadata['path'] ) - 1 ] : null;
$element_pseudo_allowed = isset( static::VALID_ELEMENT_PSEUDO_SELECTORS[ $current_element ] ) ? static::VALID_ELEMENT_PSEUDO_SELECTORS[ $current_element ] : array();

// Check for allowed pseudo classes (e.g. ":hover") from the $selector ("a:hover").
Expand All @@ -654,8 +651,6 @@ function( $pseudo_selector ) use ( $selector ) {
$declarations = static::compute_style_properties( $node, $settings, null, $this->theme_json );
}

$block_rules = '';

// 1. Separate the ones who use the general selector
// and the ones who use the duotone selector.
$declarations_duotone = array();
Expand All @@ -666,20 +661,32 @@ function( $pseudo_selector ) use ( $selector ) {
}
}

/*
* Reset default browser margin on the root body element.
* This is set on the root selector **before** generating the ruleset
* from the `theme.json`. This is to ensure that if the `theme.json` declares
* `margin` in its `spacing` declaration for the `body` element then these
* user-generated values take precedence in the CSS cascade.
* @link https://github.com/WordPress/gutenberg/issues/36147.
*/
$block_rules = '';

// 2. Generate and append the rules that use the general selector.
if ( static::ROOT_BLOCK_SELECTOR === $selector ) {
/*
* Reset default browser margin on the root body element.
* This is set on the root selector **before** generating the ruleset
* from the `theme.json`. This is to ensure that if the `theme.json` declares
* `margin` in its `spacing` declaration for the `body` element then these
* user-generated values take precedence in the CSS cascade.
* @link https://github.com/WordPress/gutenberg/issues/36147.
*/
$block_rules .= 'body { margin: 0; }';
}

// 2. Generate and append the rules that use the general selector.
$block_rules .= static::to_ruleset( $selector, $declarations );
/*
Style engine is generating the root styles only.
This way we can iteratively introduce it to start generating styles.elements, and styles.blocks.
*/
$block_rules .= gutenberg_style_engine_generate_global_styles(
$node,
array( 'selector' => $selector )
);

} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the simple if/else to figure out how we can incrementally switch over to the style engine by just tackling root to begin with 👍

$block_rules .= static::to_ruleset( $selector, $declarations );
}

// 3. Generate and append the rules that use the duotone selector.
if ( isset( $block_metadata['duotone'] ) && ! empty( $declarations_duotone ) ) {
Expand Down
1 change: 1 addition & 0 deletions packages/style-engine/class-wp-style-engine-css-declarations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ public function get_declarations() {
public function get_declarations_string() {
$declarations_array = $this->get_declarations();
$declarations_output = '';

foreach ( $declarations_array as $property => $value ) {
$filtered_declaration = esc_html( safecss_filter_attr( "{$property}: {$value}" ) );
if ( $filtered_declaration ) {
Expand Down
129 changes: 115 additions & 14 deletions packages/style-engine/class-wp-style-engine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,20 @@ protected static function get_css_var_value( $style_value, $css_vars ) {
return null;
}

/**
* Using a given path, return a value from the $block_styles object.
*
* @param array $block_styles Styles from a block's attributes object.
* @param string $ref A dot syntax path to another value in the $block_styles object, e.g., `styles.color.text`.
*
* @return string|array A style value from the block styles object.
*/
protected static function get_ref_value( $block_styles = array(), $ref = '' ) {
$ref = preg_replace( '/^styles\./', '', $ref );
$path = explode( '.', $ref );
return _wp_array_get( $block_styles, $path, null );
}

/**
* Checks whether an incoming block style value is valid.
*
Expand Down Expand Up @@ -334,20 +348,22 @@ protected static function get_classnames( $style_value, $style_definition ) {
*
* @param array $style_value A single raw style value from the generate() $block_styles array.
* @param array<string> $style_definition A single style definition from BLOCK_STYLE_DEFINITIONS_METADATA.
* @param boolean $should_skip_css_vars Whether to skip compiling CSS var values.
* @param array $block_styles Styles from a block's attributes object.
* @param array<string> $options Options passed to the public generator functions.
*
* @return array An array of CSS definitions, e.g., array( "$property" => "$value" ).
*/
protected static function get_css_declarations( $style_value, $style_definition, $should_skip_css_vars = false ) {
protected static function get_css_declarations( $style_value, $style_definition, $block_styles, $options ) {
if (
isset( $style_definition['value_func'] ) &&
is_callable( $style_definition['value_func'] )
) {
return call_user_func( $style_definition['value_func'], $style_value, $style_definition, $should_skip_css_vars );
return call_user_func( $style_definition['value_func'], $style_value, $style_definition, $block_styles, $options );
}

$css_declarations = array();
$style_property_keys = $style_definition['property_keys'];
$css_declarations = array();
$style_property_keys = $style_definition['property_keys'];
$should_skip_css_vars = isset( $options['convert_vars_to_classnames'] ) && true === $options['convert_vars_to_classnames'];

// Build CSS var values from var:? values, e.g, `var(--wp--css--rule-slug )`
// Check if the value is a CSS preset and there's a corresponding css_var pattern in the style definition.
Expand All @@ -370,6 +386,12 @@ protected static function get_css_declarations( $style_value, $style_definition,
$value = static::get_css_var_value( $value, $style_definition['css_vars'] );
}
$individual_property = sprintf( $style_property_keys['individual'], _wp_to_kebab_case( $key ) );

// If the style value contains a reference to another value in the tree.
if ( isset( $value['ref'] ) ) {
$value = static::get_ref_value( $block_styles, $value['ref'] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this assume that $block_styles will always contain the whole tree? I was wondering if for get_ref_value we might need an additional param for the complete tree (for when we add in elements / block level styles)?

Copy link
Member Author

@ramonjd ramonjd Jul 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it'd have to in this scenario, though it'll fail reasonably gracefully if the value isn't found.

It's another argument for the one-node-at-a-time approach discussed above: letting WP_Theme_JSON do the ref value fetching for us.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to have a method that accepts an entire theme.json styles tree, then it could be a separate one where we'd assume that the entire tree is passed, e.g., wp_style_engine_generate_stylesheet or 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's another argument for the one-node-at-a-time approach #42143 (comment): letting WP_Theme_JSON do the ref value fetching for us.

It's so fascinating these sorts of problems: even just the wording of your comment there makes it feel clearer that the theme JSON class is probably a better place to do ref lookups (as the owner of that data structure), and the style engine should probably treat whatever it's given as the "real" thing. I don't think there's any real right or wrong way to go about it, but these sorts of subtle distinctions really do help us tease apart what the style engine is and isn't! (Or how much scope we're comfortable taking on at any one time 😄)

}

if ( static::is_valid_style_value( $style_value ) ) {
$css_declarations[ $individual_property ] = $value;
}
Expand All @@ -387,7 +409,7 @@ protected static function get_css_declarations( $style_value, $style_definition,
*
* @param array $block_styles Styles from a block's attributes object.
* @param array $options array(
* 'selector' => (string) When a selector is passed, `generate()` will return a full CSS rule `$selector { ...rules }`, otherwise a concatenated string of properties and values.
* 'selector' => (string) When a selector is passed, the style engine will return a full CSS rule `$selector { ...rules }`, otherwise a concatenated string of properties and values.
* 'convert_vars_to_classnames' => (boolean) Whether to skip converting CSS var:? values to var( --wp--preset--* ) values. Default is `false`.
* );.
*
Expand All @@ -401,9 +423,8 @@ public function get_block_supports_styles( $block_styles, $options ) {
return null;
}

$css_declarations = array();
$classnames = array();
$should_skip_css_vars = isset( $options['convert_vars_to_classnames'] ) && true === $options['convert_vars_to_classnames'];
$css_declarations = array();
$classnames = array();

// Collect CSS and classnames.
foreach ( static::BLOCK_STYLE_DEFINITIONS_METADATA as $definition_group_key => $definition_group_style ) {
Expand All @@ -413,12 +434,17 @@ public function get_block_supports_styles( $block_styles, $options ) {
foreach ( $definition_group_style as $style_definition ) {
$style_value = _wp_array_get( $block_styles, $style_definition['path'], null );

// If the style value contains a reference to another value in the tree.
if ( isset( $style_value['ref'] ) ) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rise in new global styles features such as ref and quirky ones like --wp--style--block-gap makes me think we can extract parsing to a new class, where this stuff can take place.

$style_value = static::get_ref_value( $block_styles, $style_value['ref'] );
}

if ( ! static::is_valid_style_value( $style_value ) ) {
continue;
}

$classnames = array_merge( $classnames, static::get_classnames( $style_value, $style_definition ) );
$css_declarations = array_merge( $css_declarations, static::get_css_declarations( $style_value, $style_definition, $should_skip_css_vars ) );
$css_declarations = array_merge( $css_declarations, static::get_css_declarations( $style_value, $style_definition, $block_styles, $options ) );
}
}

Expand Down Expand Up @@ -448,26 +474,72 @@ public function get_block_supports_styles( $block_styles, $options ) {
return $styles_output;
}

/**
* Returns a stylesheet of CSS rules from a theme.json/global styles object.
*
* @param array $global_styles Styles object from theme.json.
* @param array $options array(
* 'selector' => (string) When a selector is passed, the style engine will return a full CSS rule `$selector { ...rules }`, otherwise a concatenated string of properties and values.
* );.
*
* @return string A stylesheet.
*/
public function generate_global_styles( $global_styles, $options ) {
if ( empty( $global_styles ) || ! is_array( $global_styles ) ) {
return null;
}

// The return stylesheet.
$global_stylesheet = '';

// Layer 0: Root.
$root_level_options_defaults = array(
'selector' => 'body',
);
$root_level_options = wp_parse_args( $options, $root_level_options_defaults );
$root_level_styles = $this->get_block_supports_styles(
$global_styles,
$root_level_options
);

if ( ! empty( $root_level_styles['css'] ) ) {
$global_stylesheet .= $root_level_styles['css'] . ' ';
}

// @TODO Layer 1: Elements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a question for a follow-up PR instead, but just curious about the potential layers in follow-ups. Currently the theme JSON class iterates over nodes, so generate_global_styles is currently called once in this PR for the root. In a follow-up, would we call generate_global_styles in each loop as it happens now, or would it the iteration happen in the style engine?

(I wasn't sure which would be ideal, so curious to hear your thoughts about the role/responsibility of the theme JSON class versus style engine here).

One way I was imagining, incorporating the ideas from #42222 is that (long term) we could potentially call the style engine once per iteration of the loop in the theme JSON class, but instead of it returning a css string immediately, it adds the rules to the style engine CSS rules store, and then once it finishes looping over each of the nodes, it calls the style engine method to retrieve all the generated CSS for output? I don't have any strong ideas about how any of that should work, so just thinking out loud 🙂

Copy link
Member Author

@ramonjd ramonjd Jul 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these thoughts @andrewserong !

In a follow-up, would we call generate_global_styles in each loop as it happens now, or would it the iteration happen in the style engine?

Very good questions.

To be honest, I don't know. 😄

The main motivation behind this PR was to experiment with throwing the entire merged theme.json at the style engine and generating an entire "global styles" stylesheet. I think this could be a cool feature of the public API anyway.

So you're right, this approach won't work in the current foreach ( $block_nodes as $metadata ) loop, but we eventually wouldn't need those loops if the style engine could handle an entire theme.json.

we could potentially call the style engine once per iteration of the loop in the theme JSON class, but instead of it returning a css string immediately, it adds the rules to the style engine CSS rules store, and then once it finishes looping over each of the nodes, it calls the style engine method to retrieve all the generated CSS for output

This sounds like it would be a more sensible approach, especially if it keeps any custom values parsing (such as fetching those ref values) in WP_Theme_JSON for now.

It's also similar to the approach we had in mind for block supports, so it would make doubly more sense to have a global styles analogue.

We could put this PR to the side and concentrate our efforts on the store, which we'll need first anyway. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could put this PR to the side and concentrate our efforts on the store, which we'll need first anyway. What do you think?

Yeah, that might be a good next step. Each of these explorations is a great way of working out which puzzle piece to do next, and I think the store is probably the thing that's most helpful for illuminating the path forward for this PR. It'll probably also be the thing that highlights the value of switching over to the style engine? (The clearer separation between generating and outputting styles)

But this PR's great for highlighting how we can hook in the style engine, and it's really exciting seeing all the style engine efforts starting to come together like this!


// @TODO Layer 2: Blocks.

if ( ! empty( $global_stylesheet ) ) {
return rtrim( $global_stylesheet );
}

return null;
}

/**
* Style value parser that returns a CSS definition array comprising style properties
* that have keys representing individual style properties, otherwise known as longhand CSS properties.
* e.g., "$style_property-$individual_feature: $value;", which could represent the following:
* "border-{top|right|bottom|left}-{color|width|style}: {value};" or,
* "border-image-{outset|source|width|repeat|slice}: {value};"
*
* @param array $style_value A single raw Gutenberg style attributes value for a CSS property.
* @param array $individual_property_definition A single style definition from BLOCK_STYLE_DEFINITIONS_METADATA.
* @param boolean $should_skip_css_vars Whether to skip compiling CSS var values.
* @param array $style_value A single raw Gutenberg style attributes value for a CSS property.
* @param array $individual_property_definition A single style definition from BLOCK_STYLE_DEFINITIONS_METADATA.
* @param array $block_styles Styles from a block's attributes object.
* @param array<string> $options Options passed to the public generator functions.
*
* @return array An array of CSS definitions, e.g., array( "$property" => "$value" ).
*/
protected static function get_individual_property_css_declarations( $style_value, $individual_property_definition, $should_skip_css_vars ) {
protected static function get_individual_property_css_declarations( $style_value, $individual_property_definition, $block_styles, $options ) {
$css_declarations = array();

if ( ! is_array( $style_value ) || empty( $style_value ) || empty( $individual_property_definition['path'] ) ) {
return $css_declarations;
}

$should_skip_css_vars = isset( $options['convert_vars_to_classnames'] ) && true === $options['convert_vars_to_classnames'];

// The first item in $individual_property_definition['path'] array tells us the style property, e.g., "border".
// We use this to get a corresponding CSS style definition such as "color" or "width" from the same group.
// The second item in $individual_property_definition['path'] array refers to the individual property marker, e.g., "top".
Expand All @@ -484,6 +556,11 @@ protected static function get_individual_property_css_declarations( $style_value
$style_definition = _wp_array_get( static::BLOCK_STYLE_DEFINITIONS_METADATA, $style_definition_path, null );

if ( $style_definition && isset( $style_definition['property_keys']['individual'] ) ) {
// If the style value contains a reference to another value in the tree.
if ( isset( $value['ref'] ) ) {
$value = static::get_ref_value( $block_styles, $value['ref'] );
}

// Set a CSS var if there is a valid preset value.
if ( is_string( $value ) && strpos( $value, 'var:' ) !== false && ! $should_skip_css_vars && ! empty( $individual_property_definition['css_vars'] ) ) {
$value = static::get_css_var_value( $value, $individual_property_definition['css_vars'] );
Expand Down Expand Up @@ -522,3 +599,27 @@ function wp_style_engine_get_block_supports_styles( $block_styles, $options = ar
}
return null;
}

/**
* Global public interface method to WP_Style_Engine->generate_global_styles to generate a stylesheet styles from a single theme.json style object.
* See: https://developer.wordpress.org/block-editor/reference-guides/theme-json-reference/theme-json-living/#styles
*
* Example usage:
*
* $styles = wp_style_engine_generate_global_styles( array( 'color' => array( 'text' => '#cccccc' ) ) );
* // Returns `body { color: #cccccc }`.
*
* @access public
*
* @param array $global_styles The value of a block's attributes.style.
* @param array<string> $options An array of options to determine the output.
*
* @return string A stylesheet.
*/
function wp_style_engine_generate_global_styles( $global_styles, $options = array() ) {
if ( class_exists( 'WP_Style_Engine' ) ) {
$style_engine = WP_Style_Engine::get_instance();
return $style_engine->generate_global_styles( $global_styles, $options );
}
return null;
}
Loading