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

Allow longhand and shorthand properties in theme.json and block attributes #31641

Merged
merged 9 commits into from
Jun 18, 2021
Merged

Allow longhand and shorthand properties in theme.json and block attributes #31641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
945356b
Update theme json for per corner border radii
aaronrobertshaw May 10, 2021
265a812
Update border radius block support for per corner radii
aaronrobertshaw May 10, 2021
e7f0c7c
Update block support style generation for named subpaths
aaronrobertshaw May 10, 2021
114c3d7
Update global styles renderer for custom named css properties
aaronrobertshaw May 10, 2021
4cdeee1
Make border radius, margin and padding config consistent
aaronrobertshaw Jun 15, 2021
6a2a1a1
Flatten theme.json properties metadata
aaronrobertshaw Jun 16, 2021
aafe091
Update spacing panel to support shorthand theme styles
aaronrobertshaw Jun 16, 2021
c6c5595
Remove unused has_properties function
aaronrobertshaw Jun 17, 2021
9b8f2c7
Filter empty longhand CSS styles from inline style generation
aaronrobertshaw Jun 17, 2021
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
19 changes: 14 additions & 5 deletions lib/block-supports/border.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,21 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
) {
$border_radius = $block_attributes['style']['border']['radius'];

// This check handles original unitless implementation.
if ( is_numeric( $border_radius ) ) {
$border_radius .= 'px';
if ( is_array( $border_radius ) ) {
// We have individual border radius corner values.
foreach ( $border_radius as $key => $radius ) {
// Convert CamelCase corner name to kebab-case.
$corner = strtolower( preg_replace( '/(?<!^)[A-Z]/', '-$0', $key ) );
$styles[] = sprintf( 'border-%s-radius: %s;', $corner, $radius );
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Can the border radius be null? Do we need to check for that like we do for spacing?

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 believe the original code here for the border support was added prior to _wp_array_get getting wider usage so the code approaches the check a little differently to the spacing support.

Along with checking for the feature support, it also does an isset check which would return false for a null value. Would making the supports' code consistent in approach be best done in a separate PR? I'm happy to take care of that in a follow up.

// This check handles original unitless implementation.
if ( is_numeric( $border_radius ) ) {
$border_radius .= 'px';
}

$styles[] = sprintf( 'border-radius: %s;', $border_radius );
}

$styles[] = sprintf( 'border-radius: %s;', $border_radius );
}

// Border style.
Expand Down
10 changes: 8 additions & 2 deletions lib/block-supports/spacing.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,25 @@ function gutenberg_apply_spacing_support( $block_type, $block_attributes ) {

if ( $has_padding_support ) {
$padding_value = _wp_array_get( $block_attributes, array( 'style', 'spacing', 'padding' ), null );
if ( null !== $padding_value ) {

if ( is_array( $padding_value ) ) {
foreach ( $padding_value as $key => $value ) {
$styles[] = sprintf( 'padding-%s: %s;', $key, $value );
}
} elseif ( null !== $padding_value ) {
$styles[] = sprintf( 'padding: %s;', $padding_value );
}
}

if ( $has_margin_support ) {
$margin_value = _wp_array_get( $block_attributes, array( 'style', 'spacing', 'margin' ), null );
if ( null !== $margin_value ) {

if ( is_array( $margin_value ) ) {
foreach ( $margin_value as $key => $value ) {
$styles[] = sprintf( 'margin-%s: %s;', $key, $value );
}
} elseif ( null !== $margin_value ) {
$styles[] = sprintf( 'margin: %s;', $margin_value );
}
}

Expand Down
182 changes: 46 additions & 136 deletions lib/class-wp-theme-json-gutenberg.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,8 @@ class WP_Theme_JSON_Gutenberg {
'text' => null,
),
'spacing' => array(
'margin' => array(
'top' => null,
'right' => null,
'bottom' => null,
'left' => null,
),
'padding' => array(
'bottom' => null,
'left' => null,
'right' => null,
'top' => null,
),
'margin' => null,
'padding' => null,
),
'typography' => array(
'fontFamily' => null,
Expand Down Expand Up @@ -208,64 +198,39 @@ class WP_Theme_JSON_Gutenberg {
/**
* Metadata for style properties.
*
* Each property declares:
*
* - 'value': path to the value in theme.json and block attributes.
* Each element is a direct mapping from the CSS property name to the
* path to the value in theme.json & block attributes.
*/
const PROPERTIES_METADATA = array(
'background' => array(
'value' => array( 'color', 'gradient' ),
),
'background-color' => array(
'value' => array( 'color', 'background' ),
),
'border-radius' => array(
'value' => array( 'border', 'radius' ),
),
'border-color' => array(
'value' => array( 'border', 'color' ),
),
'border-width' => array(
'value' => array( 'border', 'width' ),
),
'border-style' => array(
'value' => array( 'border', 'style' ),
),
'color' => array(
'value' => array( 'color', 'text' ),
),
'font-family' => array(
'value' => array( 'typography', 'fontFamily' ),
),
'font-size' => array(
'value' => array( 'typography', 'fontSize' ),
),
'font-style' => array(
'value' => array( 'typography', 'fontStyle' ),
),
'font-weight' => array(
'value' => array( 'typography', 'fontWeight' ),
),
'letter-spacing' => array(
'value' => array( 'typography', 'letterSpacing' ),
),
'line-height' => array(
'value' => array( 'typography', 'lineHeight' ),
),
'margin' => array(
'value' => array( 'spacing', 'margin' ),
'properties' => array( 'top', 'right', 'bottom', 'left' ),
),
'padding' => array(
'value' => array( 'spacing', 'padding' ),
'properties' => array( 'top', 'right', 'bottom', 'left' ),
),
'text-decoration' => array(
'value' => array( 'typography', 'textDecoration' ),
),
'text-transform' => array(
'value' => array( 'typography', 'textTransform' ),
),
'background' => array( 'color', 'gradient' ),
'background-color' => array( 'color', 'background' ),
'border-radius' => array( 'border', 'radius' ),
'border-top-left-radius' => array( 'border', 'radius', 'topLeft' ),
'border-top-right-radius' => array( 'border', 'radius', 'topRight' ),
'border-bottom-left-radius' => array( 'border', 'radius', 'bottomLeft' ),
'border-bottom-right-radius' => array( 'border', 'radius', 'bottomRight' ),
'border-color' => array( 'border', 'color' ),
'border-width' => array( 'border', 'width' ),
'border-style' => array( 'border', 'style' ),
'color' => array( 'color', 'text' ),
'font-family' => array( 'typography', 'fontFamily' ),
'font-size' => array( 'typography', 'fontSize' ),
'font-style' => array( 'typography', 'fontStyle' ),
'font-weight' => array( 'typography', 'fontWeight' ),
'letter-spacing' => array( 'typography', 'letterSpacing' ),
'line-height' => array( 'typography', 'lineHeight' ),
'margin' => array( 'spacing', 'margin' ),
'margin-top' => array( 'spacing', 'margin', 'top' ),
'margin-right' => array( 'spacing', 'margin', 'right' ),
'margin-bottom' => array( 'spacing', 'margin', 'bottom' ),
'margin-left' => array( 'spacing', 'margin', 'left' ),
'padding' => array( 'spacing', 'padding' ),
'padding-top' => array( 'spacing', 'padding', 'top' ),
'padding-right' => array( 'spacing', 'padding', 'right' ),
'padding-bottom' => array( 'spacing', 'padding', 'bottom' ),
'padding-left' => array( 'spacing', 'padding', 'left' ),
'text-decoration' => array( 'typography', 'textDecoration' ),
'text-transform' => array( 'typography', 'textTransform' ),
);

const ELEMENTS = array(
Expand Down Expand Up @@ -374,29 +339,6 @@ private static function sanitize( $input, $valid_block_names, $valid_element_nam
return $output;
}

/**
* Given a CSS property name, returns the property it belongs
* within the self::PROPERTIES_METADATA map.
*
* @param string $css_name The CSS property name.
*
* @return string The property name.
*/
private static function to_property( $css_name ) {
static $to_property;
if ( null === $to_property ) {
foreach ( self::PROPERTIES_METADATA as $key => $metadata ) {
$to_property[ $key ] = $key;
if ( self::has_properties( $metadata ) ) {
foreach ( $metadata['properties'] as $property ) {
$to_property[ $key . '-' . $property ] = $key;
}
}
}
}
return $to_property[ $css_name ];
}

/**
* Returns the metadata for each block.
*
Expand Down Expand Up @@ -556,7 +498,7 @@ private static function flatten_tree( $tree, $prefix = '', $token = '--' ) {
private static function get_property_value( $styles, $path ) {
$value = _wp_array_get( $styles, $path, '' );

if ( '' === $value ) {
if ( '' === $value || is_array( $value ) ) {
return $value;
}

Expand All @@ -576,21 +518,6 @@ private static function get_property_value( $styles, $path ) {
return $value;
}

/**
* Whether the metadata contains a key named properties.
*
* @param array $metadata Description of the style property.
*
* @return boolean True if properties exists, false otherwise.
*/
private static function has_properties( $metadata ) {
if ( array_key_exists( 'properties', $metadata ) ) {
return true;
}

return false;
}

/**
* Given a styles array, it extracts the style properties
* and adds them to the $declarations array following the format:
Expand All @@ -612,34 +539,16 @@ private static function compute_style_properties( $styles ) {
return $declarations;
}

$properties = array();
foreach ( self::PROPERTIES_METADATA as $name => $metadata ) {
// Some properties can be shorthand properties, meaning that
// they contain multiple values instead of a single one.
// An example of this is the padding property.
if ( self::has_properties( $metadata ) ) {
foreach ( $metadata['properties'] as $property ) {
$properties[] = array(
'name' => $name . '-' . $property,
'value' => array_merge( $metadata['value'], array( $property ) ),
);
}
} else {
$properties[] = array(
'name' => $name,
'value' => $metadata['value'],
);
}
}
foreach ( self::PROPERTIES_METADATA as $css_property => $value_path ) {
$value = self::get_property_value( $styles, $value_path );

foreach ( $properties as $prop ) {
$value = self::get_property_value( $styles, $prop['value'] );
if ( empty( $value ) ) {
// Skip if empty or value represents array of longhand values.
if ( empty( $value ) || is_array( $value ) ) {
continue;
}

$declarations[] = array(
'name' => $prop['name'],
'name' => $css_property,
'value' => $value,
);
}
Expand Down Expand Up @@ -1252,13 +1161,14 @@ private static function remove_insecure_styles( $input ) {

foreach ( $declarations as $declaration ) {
if ( self::is_safe_css_declaration( $declaration['name'], $declaration['value'] ) ) {
$property = self::to_property( $declaration['name'] );
$path = self::PROPERTIES_METADATA[ $property ]['value'];
if ( self::has_properties( self::PROPERTIES_METADATA[ $property ] ) ) {
$declaration_divided = explode( '-', $declaration['name'] );
$path[] = $declaration_divided[1];
$path = self::PROPERTIES_METADATA[ $declaration['name'] ];

// Check the value isn't an array before adding so as to not
// double up shorthand and longhand styles.
$value = _wp_array_get( $input, $path, array() );
if ( ! is_array( $value ) ) {
gutenberg_experimental_set( $output, $path, $value );
}
gutenberg_experimental_set( $output, $path, _wp_array_get( $input, $path, array() ) );
}
}
return $output;
Expand Down
4 changes: 4 additions & 0 deletions lib/compat.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ function gutenberg_override_reusable_block_post_type_labels() {
*/
function gutenberg_safe_style_attrs( $attrs ) {
$attrs[] = 'object-position';
$attrs[] = 'border-top-left-radius';
$attrs[] = 'border-top-right-radius';
$attrs[] = 'border-bottom-right-radius';
$attrs[] = 'border-bottom-left-radius';
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this change backported to Core at any point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears to me that the changes were backported and were done so in (WordPress/wordpress-develop@f034bc8).

Copy link
Contributor

Choose a reason for hiding this comment

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

Part of my confusion is that the function indicates that it should be removed once the minimum WP version is 5.8, I guess it should have been updated to 5.9 after this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was an oversight of mine sorry. This change didn't make it into a release until 11.0 and therefore WP 5.9.

An additional attribute was added to this filter a few months later (#36280) that would also make the minimum version requirement 5.9 before it can be removed.

I've created a tiny PR to fix this comment in #38359.


return $attrs;
}
Expand Down
19 changes: 13 additions & 6 deletions packages/block-editor/src/hooks/style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
* External dependencies
*/
import {
capitalize,
first,
forEach,
get,
has,
isEmpty,
isString,
kebabCase,
map,
omit,
Expand Down Expand Up @@ -78,11 +78,18 @@ export function getInlineStyles( styles = {} ) {
const subPaths = STYLE_PROPERTY[ propKey ].properties;
// Ignore styles on elements because they are handled on the server.
if ( has( styles, path ) && 'elements' !== first( path ) ) {
if ( !! subPaths ) {
subPaths.forEach( ( suffix ) => {
output[
propKey + capitalize( suffix )
] = compileStyleValue( get( styles, [ ...path, suffix ] ) );
// Checking if style value is a string allows for shorthand css
// option and backwards compatibility for border radius support.
const styleValue = get( styles, path );

if ( !! subPaths && ! isString( styleValue ) ) {
Object.entries( subPaths ).forEach( ( entry ) => {
const [ name, subPath ] = entry;
const value = get( styleValue, [ subPath ] );

if ( value ) {
output[ name ] = compileStyleValue( value );
}
} );
} else {
output[ propKey ] = compileStyleValue( get( styles, path ) );
Expand Down
Loading