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

Use the Style Engine in layout block-supports #42366

Closed
wants to merge 10 commits into from
Closed

Use the Style Engine in layout block-supports #42366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fdf7abf
use WP_Style_Engine_CSS_Declarations_Gutenberg in layout.php
aristath Jul 12, 2022
27ed2c6
say what the array is
aristath Jul 12, 2022
29b4874
cleanup: this is not necessary, moved $selector to the initial array
aristath Jul 12, 2022
96ecea3
$selector is already defined in the array init
aristath Jul 15, 2022
20f45b0
Use the style-engine store
aristath Jul 15, 2022
76fa947
set_declarations was renamed to add_declarations
aristath Jul 19, 2022
4d6f59c
Remove selectors from a store once we generate CSS
aristath Jul 19, 2022
b760d78
Shazam
aristath Jul 19, 2022
7aa60b5
Added a param to the get_css() method
aristath Jul 19, 2022
b67963c
short and sweet
aristath Jul 19, 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
95 changes: 63 additions & 32 deletions lib/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,33 @@ function gutenberg_register_layout_support( $block_type ) {
* @return string CSS style.
*/
function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support = false, $gap_value = null, $should_skip_gap_serialization = false, $fallback_gap_value = '0.5em', $block_spacing = null ) {
gutenberg_set_layout_style( $selector, $layout, $has_block_gap_support, $gap_value, $should_skip_gap_serialization, $fallback_gap_value, $block_spacing );

// Use a unique store to avoid conflicts with other stores and implementations.
$store = WP_Style_Engine_CSS_Rules_Store_Gutenberg::get_store( 'block-supports/layout/' . md5( $selector ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this is an interesting way to solve it. Will this effectively involve registering dozens of stores if a post contains dozens of blocks that opt-in to layout support?

It looks like @ramonjd is attempting to solve a similar problem as this PR in #42452. In that PR there's a wp_style_engine_enqueue_block_supports_styles function (here) which would call a method of WP_Style_Engine to add the declarations to the block-supports store. If I'm following that PR correctly, when the main WP_Style_Engine class is constructed, it registers the hooks for output, so the behaviour is encapsulated a bit further. I think the result then would be that we'd only need the one block-supports store, and it gets output once? Though there's a bit of discussion here (https://github.com/WordPress/gutenberg/pull/42452/files#r922931702) that for that process to work, we might need to flag at a store level whether or not it's to be enqueued?

So many little bits to figure out 😀

Copy link
Member Author

Choose a reason for hiding this comment

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

Will this effectively involve registering dozens of stores if a post contains dozens of blocks that opt-in to layout support?

Nope! The gutenberg_get_layout_style is no longer used anywhere and can probably be removed. I tweaked it here to use a unique store per selector just to cover all bases in case someone uses the function in a plugin etc. So it's basically just for backwards-compatibility purposes 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, thanks for clarifying, I was probably reading too quickly! Good idea thinking of backwards compatibility — I wonder if in practice anyone actually uses it directly? I think I'd always thought of it as an internal / de-facto private method, but yeah, there very well could be someone attempting to use it in a plugin or theme 🤔

Copy link
Member

@ramonjd ramonjd Jul 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

If I'm following that PR correctly, when the main WP_Style_Engine class is constructed, it registers the hooks for output, so the behaviour is encapsulated a bit further. I think the result then would be that we'd only need the one block-supports store, and it gets output once?

That's how I've built it 👍

Is there a reason why we'd want multiple stores for block supports?

I'm open to accusations of egocentric bias, but I think #42452 might do want we want without the need for granular stores. I've tested it using the CSS storing steps in this PR.

we might need to flag at a store level whether or not it's to be enqueued?

I've solved this by breaking apart the compile (into CSS strings) and parser (parsing raw block style values) functionalities, which should set us up for further abstraction.

I wonder if in practice anyone actually uses it directly? I think I'd always thought of it as an internal / de-facto private method, but yeah, there very well could be someone attempting to use it in a plugin or theme

I reckon we should encourage using the internal method in practice (in code and eventually in documentation), and I think it'd be neater for our own purposes anyway because

  1. The use of the classname is abstracted
  2. The store key exists in one place (fewer places to update)
  3. Abstracting logic should mean that making changes (which will be inevitable during these early phases) will give us fewer headaches.

If theme authors really want to hook into Gutenberg's stores, they'll be able to do it via any filters we build in later as demonstrated in #42493

Copy link
Member

@ramonjd ramonjd Jul 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Is there a reason why we'd want multiple stores for block supports?

Ooooh! I see that we'd have to return the layout styles in gutenberg_get_layout_style for backwards compatibility. 🤦 It is a bit fiddly.

It'd still be nice not to have to expose the classes too early.

I'm sure we'll come up with something.

$processor = new WP_Style_Engine_Processor_Gutenberg( $store );
return $processor->get_css( true );
}

/**
* Adds the provided layout to the "block-supports/layout" Style-Engine store.
*
* @param string $selector CSS selector.
* @param array $layout Layout object. The one that is passed has already checked the existence of default block layout.
* @param boolean $has_block_gap_support Whether the theme has support for the block gap.
* @param string $gap_value The block gap value to apply.
* @param boolean $should_skip_gap_serialization Whether to skip applying the user-defined value set in the editor.
* @param string $fallback_gap_value The block gap value to apply.
* @param array $block_spacing Custom spacing set on the block.
*
* @return bool Returns true if the layout style was successfully generated, otherwise false.
*/
function gutenberg_set_layout_style( $selector, $layout, $has_block_gap_support = false, $gap_value = null, $should_skip_gap_serialization = false, $fallback_gap_value = '0.5em', $block_spacing = null ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to create a new function, or can the logic that's now in gutenberg_get_layout_style move to render_layout_support_flag? It can be hard to remove these global-namespace functions once they're introduced, so it'd be good to see if there are ways to avoid adding additional functions where we can.

Separately to hooking in the style engine, one of the tasks of #39336 is to try moving content justification to a preset-like approach, so that part of the logic might eventually be removed from the body of this function — so (in theory at least) we might be able to eventually reduce the complexity of this function a little bit. That can totally happen well after this PR lands, though, so not a blocker for the work here.

$layout_type = isset( $layout['type'] ) ? $layout['type'] : 'default';

$style = '';
// Get the block-supports Style-Engine Store.
$store = WP_Style_Engine_CSS_Rules_Store_Gutenberg::get_store( 'block-supports/layout' );

if ( 'default' === $layout_type ) {
$content_size = isset( $layout['contentSize'] ) ? $layout['contentSize'] : '';
$wide_size = isset( $layout['wideSize'] ) ? $layout['wideSize'] : '';
Expand All @@ -55,14 +79,16 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
$wide_max_width_value = wp_strip_all_tags( explode( ';', $wide_max_width_value )[0] );

if ( $content_size || $wide_size ) {
$style = "$selector > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {";
$style .= 'max-width: ' . esc_html( $all_max_width_value ) . ';';
$style .= 'margin-left: auto !important;';
$style .= 'margin-right: auto !important;';
$style .= '}';
$store->add_rule( "$selector > :where(:not(.alignleft):not(.alignright):not(.alignfull))" )->add_declarations(
array(
'max-width' => $all_max_width_value,
'margin-left' => 'auto !important',
'margin-right' => 'auto !important',
)
);

$style .= "$selector > .alignwide { max-width: " . esc_html( $wide_max_width_value ) . ';}';
$style .= "$selector .alignfull { max-width: none; }";
$store->add_rule( "$selector > .alignwide" )->add_declarations( array( 'max-width' => $wide_max_width_value ) );
$store->add_rule( "$selector .alignfull" )->add_declarations( array( 'max-width' => 'none' ) );

if ( isset( $block_spacing ) ) {
$block_spacing_values = gutenberg_style_engine_get_block_supports_styles(
Expand All @@ -75,11 +101,11 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
// They're added separately because padding might only be set on one side.
if ( isset( $block_spacing_values['declarations']['padding-right'] ) ) {
$padding_right = $block_spacing_values['declarations']['padding-right'];
$style .= "$selector > .alignfull { margin-right:calc($padding_right * -1); }";
$store->add_rule( "$selector > .alignfull" )->add_declarations( array( 'margin-right' => "calc($padding_right * -1)" ) );
}
if ( isset( $block_spacing_values['declarations']['padding-left'] ) ) {
$padding_left = $block_spacing_values['declarations']['padding-left'];
$style .= "$selector > .alignfull { margin-left: calc($padding_left * -1); }";
$store->add_rule( "$selector > .alignfull" )->add_declarations( array( 'margin-left' => "calc($padding_left * -1)" ) );
}
}
}
Expand All @@ -89,8 +115,18 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
$gap_value = isset( $gap_value['top'] ) ? $gap_value['top'] : null;
}
if ( $gap_value && ! $should_skip_gap_serialization ) {
$style .= "$selector > * { margin-block-start: 0; margin-block-end: 0; }";
$style .= "$selector > * + * { margin-block-start: $gap_value; margin-block-end: 0; }";
$store->add_rule( "$selector > *" )->add_declarations(
array(
'margin-block-start' => '0',
'margin-block-end' => '0',
)
);
$store->add_rule( "$selector > * + *" )->add_declarations(
array(
'margin-block-start' => $gap_value,
'margin-block-end' => '0',
)
);
}
}
} elseif ( 'flex' === $layout_type ) {
Expand All @@ -113,7 +149,7 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
}

if ( ! empty( $layout['flexWrap'] ) && 'nowrap' === $layout['flexWrap'] ) {
$style .= "$selector { flex-wrap: nowrap; }";
$store->add_rule( $selector )->add_declarations( array( 'flex-wrap' => 'nowrap' ) );
}

if ( $has_block_gap_support ) {
Expand All @@ -123,9 +159,7 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
$gap_value = $gap_row === $gap_column ? $gap_row : $gap_row . ' ' . $gap_column;
}
if ( $gap_value && ! $should_skip_gap_serialization ) {
$style .= "$selector {";
$style .= "gap: $gap_value;";
$style .= '}';
$store->add_rule( $selector )->add_declarations( array( 'gap' => $gap_value ) );
}
}

Expand All @@ -136,29 +170,26 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
* by custom css.
*/
if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
$style .= "$selector {";
$style .= "justify-content: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= '}';
$store->add_rule( $selector )->add_declarations( array( 'justify-content' => $justify_content_options[ $layout['justifyContent'] ] ) );
}

if ( ! empty( $layout['verticalAlignment'] ) && array_key_exists( $layout['verticalAlignment'], $vertical_alignment_options ) ) {
$style .= "$selector {";
$style .= "align-items: {$vertical_alignment_options[ $layout['verticalAlignment'] ]};";
$style .= '}';
$store->add_rule( $selector )->add_declarations( array( 'align-items' => $vertical_alignment_options[ $layout['verticalAlignment'] ] ) );
}
} else {
$style .= "$selector {";
$style .= 'flex-direction: column;';
$store->add_rule( $selector )->add_declarations(
array(
'flex-direction' => 'column',
'align-items' => 'flex-start',
)
);
if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
$style .= "align-items: {$justify_content_options[ $layout['justifyContent'] ]};";
} else {
$style .= 'align-items: flex-start;';
$store->add_rule( $selector )->add_declarations( array( 'align-items' => $justify_content_options[ $layout['justifyContent'] ] ) );
}
$style .= '}';
}
}

return $style;
return ! empty( $store->get_all_rules() );
}

/**
Expand Down Expand Up @@ -244,12 +275,12 @@ function gutenberg_render_layout_support_flag( $block_content, $block ) {
// If a block's block.json skips serialization for spacing or spacing.blockGap,
// don't apply the user-defined value to the styles.
$should_skip_gap_serialization = gutenberg_should_skip_block_supports_serialization( $block_type, 'spacing', 'blockGap' );
$style = gutenberg_get_layout_style( ".$block_classname.$container_class", $used_layout, $has_block_gap_support, $gap_value, $should_skip_gap_serialization, $fallback_gap_value, $block_spacing );
$has_style = gutenberg_set_layout_style( ".$block_classname.$container_class", $used_layout, $has_block_gap_support, $gap_value, $should_skip_gap_serialization, $fallback_gap_value, $block_spacing );

// Only add container class and enqueue block support styles if unique styles were generated.
if ( ! empty( $style ) ) {
if ( $has_style ) {
$class_names[] = $container_class;
wp_enqueue_block_support_styles( $style );
gutenberg_enqueue_style_engine_store( 'block-supports/layout' );
}

// This assumes the hook only applies to blocks with a single wrapper.
Expand Down
26 changes: 22 additions & 4 deletions lib/compat/wordpress-6.1/script-loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@
* @param int $priority To set the priority for the add_action.
*/
function gutenberg_enqueue_block_support_styles( $style, $priority = 10 ) {
$action_hook_name = 'wp_footer';
if ( wp_is_block_theme() ) {
$action_hook_name = 'wp_head';
}
$action_hook_name = wp_is_block_theme() ? 'wp_head' : 'wp_footer';
add_action(
$action_hook_name,
static function () use ( $style ) {
Expand All @@ -36,6 +33,27 @@ static function () use ( $style ) {
);
}

/**
* This function takes care of adding inline styles
* from a Style-Engine store.
*
* @param string $store_name The name of the store.
*/
function gutenberg_enqueue_style_engine_store( $store_name ) {
Copy link
Member

Choose a reason for hiding this comment

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

I think I'm starting to come around to leaving the work of enqueuing to the consumer like this (I'm slow 😄) especially in the case of Gutenberg, where there are existing functions that we can repurpose.

Maybe later we could think about writing public utility methods in the style engine to take some of the work of enqueuing away from other consumers.

The gutenberg_enqueue_block_support_styles function above is another great example of where we could reuse/recycle, especially given the gymnastics we seem to have to go through to ensure backwards compatibility for such functions.

$action_hook_name = wp_is_block_theme() ? 'wp_head' : 'wp_footer';
add_action(
$action_hook_name,
static function () use ( $store_name ) {
$css = ( new WP_Style_Engine_Processor_Gutenberg(
WP_Style_Engine_CSS_Rules_Store_Gutenberg::get_store( $store_name )
) )->get_css( true );
if ( $css ) {
echo '<style id="wp-style-engine-' . esc_attr( str_replace( '/', '-', $store_name ) ) . '">' . $css . '</style>';
}
}
);
}

/**
* This applies a filter to the list of style nodes that comes from `get_style_nodes` in WP_Theme_JSON.
* This particular filter removes all of the blocks from the array.
Expand Down
9 changes: 8 additions & 1 deletion packages/style-engine/class-wp-style-engine-processor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,24 @@ public function __construct( WP_Style_Engine_CSS_Rules_Store $store ) {
/**
* Get the CSS rules as a string.
*
* @param bool $remove_printed_rules Whether to remove printed rules.
*
* @return string The computed CSS.
*/
public function get_css() {
public function get_css( $remove_printed_rules = false ) {
// Combine CSS selectors that have identical declarations.
$this->combine_rules_selectors();

// Build the CSS.
$css = '';
$rules = $this->store->get_all_rules();
foreach ( $rules as $rule ) {
// Add the CSS.
$css .= $rule->get_css();
if ( $remove_printed_rules ) {
// Remove the rule from the store to avoid double-rendering.
$this->store->remove_rule( $rule->get_selector() );
}
}
return $css;
}
Expand Down