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.

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

Block Supports: Allow skipping serialization of individual features #36293

Merged
merged 16 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 11 additions & 21 deletions lib/block-supports/border.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function gutenberg_register_border_support( $block_type ) {
* @return array Border CSS classes and inline styles.
*/
function gutenberg_apply_border_support( $block_type, $block_attributes ) {
if ( gutenberg_skip_border_serialization( $block_type ) ) {
if ( gutenberg_should_skip_block_supports_serialization( $block_type, 'border' ) ) {
return array();
}

Expand All @@ -54,7 +54,8 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
// Border radius.
if (
gutenberg_has_border_feature_support( $block_type, 'radius' ) &&
isset( $block_attributes['style']['border']['radius'] )
isset( $block_attributes['style']['border']['radius'] ) &&
! gutenberg_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'radius' )
) {
$border_radius = $block_attributes['style']['border']['radius'];

Expand All @@ -78,7 +79,8 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
// Border style.
if (
gutenberg_has_border_feature_support( $block_type, 'style' ) &&
isset( $block_attributes['style']['border']['style'] )
isset( $block_attributes['style']['border']['style'] ) &&
! gutenberg_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'style' )
) {
$border_style = $block_attributes['style']['border']['style'];
$styles[] = sprintf( 'border-style: %s;', $border_style );
Expand All @@ -87,7 +89,8 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
// Border width.
if (
gutenberg_has_border_feature_support( $block_type, 'width' ) &&
isset( $block_attributes['style']['border']['width'] )
isset( $block_attributes['style']['border']['width'] ) &&
! gutenberg_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'width' )
) {
$border_width = $block_attributes['style']['border']['width'];

Expand All @@ -100,7 +103,10 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
}

// Border color.
if ( gutenberg_has_border_feature_support( $block_type, 'color' ) ) {
if (
gutenberg_has_border_feature_support( $block_type, 'color' ) &&
! gutenberg_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'color' )
) {
$has_named_border_color = array_key_exists( 'borderColor', $block_attributes );
$has_custom_border_color = isset( $block_attributes['style']['border']['color'] );

Expand Down Expand Up @@ -130,22 +136,6 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
return $attributes;
}

/**
* Checks whether serialization of the current block's border properties should
* occur.
*
* @param WP_Block_type $block_type Block type.
*
* @return boolean
*/
function gutenberg_skip_border_serialization( $block_type ) {
$border_support = _wp_array_get( $block_type->supports, array( '__experimentalBorder' ), false );

return is_array( $border_support ) &&
array_key_exists( '__experimentalSkipSerialization', $border_support ) &&
$border_support['__experimentalSkipSerialization'];
}

/**
* Checks whether the current block type supports the border feature requested.
*
Expand Down
9 changes: 4 additions & 5 deletions lib/block-supports/colors.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ function gutenberg_apply_colors_support( $block_type, $block_attributes ) {

if (
is_array( $color_support ) &&
array_key_exists( '__experimentalSkipSerialization', $color_support ) &&
$color_support['__experimentalSkipSerialization']
gutenberg_should_skip_block_supports_serialization( $block_type, 'color' )
) {
return array();
}
Expand All @@ -82,7 +81,7 @@ function gutenberg_apply_colors_support( $block_type, $block_attributes ) {

// Text colors.
// Check support for text colors.
if ( $has_text_colors_support ) {
if ( $has_text_colors_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'color', 'text' ) ) {
$has_named_text_color = array_key_exists( 'textColor', $block_attributes );
$has_custom_text_color = isset( $block_attributes['style']['color']['text'] );

Expand All @@ -99,7 +98,7 @@ function gutenberg_apply_colors_support( $block_type, $block_attributes ) {
}

// Background colors.
if ( $has_background_colors_support ) {
if ( $has_background_colors_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'color', 'background' ) ) {
$has_named_background_color = array_key_exists( 'backgroundColor', $block_attributes );
$has_custom_background_color = isset( $block_attributes['style']['color']['background'] );

Expand All @@ -116,7 +115,7 @@ function gutenberg_apply_colors_support( $block_type, $block_attributes ) {
}

// Gradients.
if ( $has_gradients_support ) {
if ( $has_gradients_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'color', 'gradients' ) ) {
$has_named_gradient = array_key_exists( 'gradient', $block_attributes );
$has_custom_gradient = isset( $block_attributes['style']['color']['gradient'] );

Expand Down
17 changes: 1 addition & 16 deletions lib/block-supports/dimensions.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function gutenberg_register_dimensions_support( $block_type ) {
* @return array Block dimensions CSS classes and inline styles.
*/
function gutenberg_apply_dimensions_support( $block_type, $block_attributes ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
if ( gutenberg_skip_dimensions_serialization( $block_type ) ) {
if ( gutenberg_should_skip_block_supports_serialization( $block_type, '__experimentalDimensions' ) ) {
return array();
}

Expand All @@ -57,21 +57,6 @@ function gutenberg_apply_dimensions_support( $block_type, $block_attributes ) {
return empty( $styles ) ? array() : array( 'style' => implode( ' ', $styles ) );
}

/**
* Checks whether serialization of the current block's dimensions properties
* should occur.
*
* @param WP_Block_type $block_type Block type.
*
* @return boolean Whether to serialize spacing support styles & classes.
*/
function gutenberg_skip_dimensions_serialization( $block_type ) {
$dimensions_support = _wp_array_get( $block_type->supports, array( '__experimentalDimensions' ), false );
return is_array( $dimensions_support ) &&
array_key_exists( '__experimentalSkipSerialization', $dimensions_support ) &&
$dimensions_support['__experimentalSkipSerialization'];
}

// Register the block support.
WP_Block_Supports::get_instance()->register(
'dimensions',
Expand Down
7 changes: 7 additions & 0 deletions lib/block-supports/elements.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ function gutenberg_render_elements_support( $block_content, $block ) {
return $block_content;
}

$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );
$skip_link_color_serialization = gutenberg_should_skip_block_supports_serialization( $block_type, 'color', 'link' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought: I'm wondering what this might look like if elements is made generic as mentioned in #31524. If we can use it with any element in any style, perhaps skipping serialization will be redundant?

Copy link
Member

Choose a reason for hiding this comment

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

If we can use it with any element in any style, perhaps skipping serialization will be redundant?

That's a good question, and one I can't answer, though from a superficial reading of the supports file and the conversation of the PR you linked to, I'd probably side with you on that assumption. 😄

I'm not sure there are any plans slated for the future of the elements mechanism 🤔

cc @jorgefilipecosta

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure there are any plans slated for the future of the elements mechanism 🤔

Right now we are artificially limiting to some elements like links but the mechanism is generic and we can expand it. I'm not sure if someone is working on that.

I agree ideally the elements mechanism would remove the need to skip serialization but I guess until that mechanism is expanded we can have experimental flags to skip serializtion and then we check if we can use elements for these cases.


if ( $skip_link_color_serialization ) {
return $block_content;
}

$link_color = null;
if ( ! empty( $block['attrs'] ) ) {
$link_color = _wp_array_get( $block['attrs'], array( 'style', 'elements', 'link', 'color', 'text' ), null );
Expand Down
22 changes: 13 additions & 9 deletions lib/block-supports/layout.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ function gutenberg_register_layout_support( $block_type ) {
/**
* Generates the CSS corresponding to the provided layout.
*
* @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 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not also allow skipping serialization for the whole layout feature too? The Navigation block already implements custom logic to apply the layout styles to its inner elements and blocks, so there's definitely a use case.

Copy link
Member

Choose a reason for hiding this comment

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

Should we not also allow skipping serialization for the whole layout feature too?

Good point! Thanks for mentioning it. I think this would be a good follow up PR.

I'm guessing @aaronrobertshaw started the work here with a view of extending block supports (border, spacing et. al) that already use skipSerialization in various block.jsons today.

The layout implementation, being a little "different" might also need a level of testing that might be more suited to a dedicated PR. 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, layout is complex enough that a follow-up PR sounds right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm only just getting up to speed after some recent AFK so might be missing some context.

When originally creating this I found a few block supports that simply didn't maintain/add the ability to skip serialization as per other block supports. Layout was one example of this. It also changed several times during the long life span of this PR. Early on, I had a skip for the entire layout feature but the support was reworked which, from memory, removed the code where that could easily be performed.

I think a separate PR would definitely be a good idea given how complicated layout support is and how much it's already changed in its relatively short life.

*
* @return string CSS style.
* @return string CSS style.
*/
function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support = false, $gap_value = null ) {
function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support = false, $gap_value = null, $should_skip_gap_serialization = false ) {
$layout_type = isset( $layout['type'] ) ? $layout['type'] : 'default';

$style = '';
Expand Down Expand Up @@ -69,7 +70,7 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
if ( is_array( $gap_value ) ) {
$gap_value = isset( $gap_value['top'] ) ? $gap_value['top'] : null;
}
$gap_style = $gap_value ? $gap_value : 'var( --wp--style--block-gap )';
$gap_style = $gap_value && ! $should_skip_gap_serialization ? $gap_value : 'var( --wp--style--block-gap )';
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're skipping serialization would we still expect there to be an output on the front end, even if it's a default value?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is another tricky one. 🤣

I agree in principle, and it did cross my mind, however there's a consideration that caused me to hesitate: --wp--style--block-gap's value comes from theme.json.

See: https://github.com/WordPress/gutenberg/blob/trunk/docs/how-to-guides/themes/theme-json.md#what-is-blockgap-and-how-can-i-use-it

So my reasoning at the time was, since skipSerialization acts at the block level, and is indeed meant to offer finer-grained control of block supports, it probably shouldn't override the theme's settings.

Skipping serialization, as far as I understand it, skips user-facing block customization, that is the editor styles controls.

Aside from all that, blockGap is another one of those anomalies. It straddles block supports and layout, and doesn't make life easy when working with both.

Hopefully we can consolidate/homogenize the use of layout and blockGap in the effort to make styles and layouts consistent. See #39336

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this is a tough one. On the whole I'm more inclined to leave it out because the theme only defines the variable value, not where to use it, and I'd expect skipSerialization to not output anything. But I think we can leave it as is and revisit later; outputting a couple of default styles shouldn't be a huge deal.

$style .= "$selector > * { margin-block-start: 0; margin-block-end: 0; }";
$style .= "$selector > * + * { margin-block-start: $gap_style; margin-block-end: 0; }";
}
Expand Down Expand Up @@ -99,7 +100,7 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
$gap_column = isset( $gap_value['left'] ) ? $gap_value['left'] : '0.5em';
$gap_value = $gap_row === $gap_column ? $gap_row : $gap_row . ' ' . $gap_column;
}
$gap_style = $gap_value ? $gap_value : 'var( --wp--style--block-gap, 0.5em )';
$gap_style = $gap_value && ! $should_skip_gap_serialization ? $gap_value : 'var( --wp--style--block-gap, 0.5em )';
$style .= "gap: $gap_style;";
} else {
$style .= 'gap: 0.5em;';
Expand Down Expand Up @@ -172,7 +173,10 @@ function gutenberg_render_layout_support_flag( $block_content, $block ) {
$gap_value = preg_match( '%[\\\(&=}]|/\*%', $gap_value ) ? null : $gap_value;
}

$style = gutenberg_get_layout_style( ".$class_name", $used_layout, $has_block_gap_support, $gap_value );
// 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( ".$class_name", $used_layout, $has_block_gap_support, $gap_value, $should_skip_gap_serialization );
// This assumes the hook only applies to blocks with a single wrapper.
// I think this is a reasonable limitation for that particular hook.
$content = preg_replace(
Expand Down
25 changes: 6 additions & 19 deletions lib/block-supports/spacing.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ function gutenberg_register_spacing_support( $block_type ) {
* @return array Block spacing CSS classes and inline styles.
*/
function gutenberg_apply_spacing_support( $block_type, $block_attributes ) {
if ( gutenberg_skip_spacing_serialization( $block_type ) ) {
if ( gutenberg_should_skip_block_supports_serialization( $block_type, 'spacing' ) ) {
return array();
}

$attributes = array();
$has_padding_support = gutenberg_block_has_support( $block_type, array( 'spacing', 'padding' ), false );
$has_margin_support = gutenberg_block_has_support( $block_type, array( 'spacing', 'margin' ), false );
Expand All @@ -52,9 +53,11 @@ function gutenberg_apply_spacing_support( $block_type, $block_attributes ) {
}

$style_engine = WP_Style_Engine_Gutenberg::get_instance();
$skip_padding = gutenberg_should_skip_block_supports_serialization( $block_type, 'spacing', 'padding' );
$skip_margin = gutenberg_should_skip_block_supports_serialization( $block_type, 'spacing', 'margin' );
$spacing_block_styles = array();
$spacing_block_styles['padding'] = $has_padding_support ? _wp_array_get( $block_styles, array( 'spacing', 'padding' ), null ) : null;
$spacing_block_styles['margin'] = $has_margin_support ? _wp_array_get( $block_styles, array( 'spacing', 'margin' ), null ) : null;
$spacing_block_styles['padding'] = $has_padding_support && ! $skip_padding ? _wp_array_get( $block_styles, array( 'spacing', 'padding' ), null ) : null;
$spacing_block_styles['margin'] = $has_margin_support && ! $skip_margin ? _wp_array_get( $block_styles, array( 'spacing', 'margin' ), null ) : null;
$inline_styles = $style_engine->generate(
array( 'spacing' => $spacing_block_styles ),
array(
Expand All @@ -69,22 +72,6 @@ function gutenberg_apply_spacing_support( $block_type, $block_attributes ) {
return $attributes;
}

/**
* Checks whether serialization of the current block's spacing properties should
* occur.
*
* @param WP_Block_type $block_type Block type.
*
* @return boolean Whether to serialize spacing support styles & classes.
*/
function gutenberg_skip_spacing_serialization( $block_type ) {
$spacing_support = _wp_array_get( $block_type->supports, array( 'spacing' ), false );

return is_array( $spacing_support ) &&
array_key_exists( '__experimentalSkipSerialization', $spacing_support ) &&
$spacing_support['__experimentalSkipSerialization'];
}

// Register the block support.
WP_Block_Supports::get_instance()->register(
'spacing',
Expand Down
21 changes: 11 additions & 10 deletions lib/block-supports/typography.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ function gutenberg_apply_typography_support( $block_type, $block_attributes ) {
return array();
}

$skip_typography_serialization = _wp_array_get( $typography_supports, array( '__experimentalSkipSerialization' ), false );
if ( $skip_typography_serialization ) {
if ( gutenberg_should_skip_block_supports_serialization( $block_type, 'typography' ) ) {
return array();
}

Expand All @@ -93,7 +92,7 @@ function gutenberg_apply_typography_support( $block_type, $block_attributes ) {
$has_text_decoration_support = _wp_array_get( $typography_supports, array( '__experimentalTextDecoration' ), false );
$has_text_transform_support = _wp_array_get( $typography_supports, array( '__experimentalTextTransform' ), false );

if ( $has_font_size_support ) {
if ( $has_font_size_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'typography', 'fontSize' ) ) {
$has_named_font_size = array_key_exists( 'fontSize', $block_attributes );
$has_custom_font_size = isset( $block_attributes['style']['typography']['fontSize'] );

Expand All @@ -104,7 +103,7 @@ function gutenberg_apply_typography_support( $block_type, $block_attributes ) {
}
}

if ( $has_font_family_support ) {
if ( $has_font_family_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'typography', 'fontFamily' ) ) {
$has_named_font_family = array_key_exists( 'fontFamily', $block_attributes );
$has_custom_font_family = isset( $block_attributes['style']['typography']['fontFamily'] );

Expand All @@ -123,42 +122,42 @@ function gutenberg_apply_typography_support( $block_type, $block_attributes ) {
}
}

if ( $has_font_style_support ) {
if ( $has_font_style_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'typography', 'fontStyle' ) ) {
$font_style = gutenberg_typography_get_css_variable_inline_style( $block_attributes, 'fontStyle', 'font-style' );
if ( $font_style ) {
$styles[] = $font_style;
}
}

if ( $has_font_weight_support ) {
if ( $has_font_weight_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'typography', 'fontWeight' ) ) {
$font_weight = gutenberg_typography_get_css_variable_inline_style( $block_attributes, 'fontWeight', 'font-weight' );
if ( $font_weight ) {
$styles[] = $font_weight;
}
}

if ( $has_line_height_support ) {
if ( $has_line_height_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'typography', 'lineHeight' ) ) {
$has_line_height = isset( $block_attributes['style']['typography']['lineHeight'] );
if ( $has_line_height ) {
$styles[] = sprintf( 'line-height: %s;', $block_attributes['style']['typography']['lineHeight'] );
}
}

if ( $has_text_decoration_support ) {
if ( $has_text_decoration_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'typography', 'textDecoration' ) ) {
$text_decoration_style = gutenberg_typography_get_css_variable_inline_style( $block_attributes, 'textDecoration', 'text-decoration' );
if ( $text_decoration_style ) {
$styles[] = $text_decoration_style;
}
}

if ( $has_text_transform_support ) {
if ( $has_text_transform_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'typography', 'textTransform' ) ) {
$text_transform_style = gutenberg_typography_get_css_variable_inline_style( $block_attributes, 'textTransform', 'text-transform' );
if ( $text_transform_style ) {
$styles[] = $text_transform_style;
}
}

if ( $has_letter_spacing_support ) {
if ( $has_letter_spacing_support && ! gutenberg_should_skip_block_supports_serialization( $block_type, 'typography', 'letterSpacing' ) ) {
$letter_spacing_style = gutenberg_typography_get_css_variable_inline_style( $block_attributes, 'letterSpacing', 'letter-spacing' );
if ( $letter_spacing_style ) {
$styles[] = $letter_spacing_style;
Expand Down Expand Up @@ -214,3 +213,5 @@ function gutenberg_typography_get_css_variable_inline_style( $attributes, $featu
'apply' => 'gutenberg_apply_typography_support',
)
);


31 changes: 31 additions & 0 deletions lib/block-supports/utils.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php
/**
* Block support utility functions.
*
* @package gutenberg
*/

/**
* Checks whether serialization of the current block's supported properties
* should occur.
*
* @param WP_Block_type $block_type Block type.
* @param string $feature_set Name of block support feature set..
* @param string $feature Optional name of individual feature to check.
*
* @return boolean Whether to serialize block support styles & classes.
*/
function gutenberg_should_skip_block_supports_serialization( $block_type, $feature_set, $feature = null ) {
if ( ! is_object( $block_type ) || ! $feature_set ) {
return false;
}

$path = array( $feature_set, '__experimentalSkipSerialization' );
$skip_serialization = _wp_array_get( $block_type->supports, $path, false );

if ( is_array( $skip_serialization ) ) {
return in_array( $feature, $skip_serialization, true );
}

return $skip_serialization;
}
1 change: 1 addition & 0 deletions lib/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ function gutenberg_is_experiment_enabled( $name ) {
// similar to the loading behaviour in `blocks.php`.
require __DIR__ . '/style-engine/class-wp-style-engine-gutenberg.php';

require __DIR__ . '/block-supports/utils.php';
require __DIR__ . '/block-supports/elements.php';
require __DIR__ . '/block-supports/colors.php';
require __DIR__ . '/block-supports/typography.php';
Expand Down
Loading