Skip to content

Commit

Permalink
Ensuring gradient classnames aren't shown
Browse files Browse the repository at this point in the history
  • Loading branch information
@ramonjd
ramonjd committed Apr 23, 2024
1 parent 13e5b6d commit 3789393
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 63 deletions.
15 changes: 9 additions & 6 deletions lib/block-supports/background.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function gutenberg_render_background_support( $block_content, $block ) {
if ( $has_background_gradient_support && ! wp_should_skip_block_supports_serialization( $block_type, 'color', 'gradients' ) ) {
$preset_gradient_color = array_key_exists( 'gradient', $block_attributes ) ? "var:preset|gradient|{$block_attributes['gradient']}" : null;
$custom_gradient_color = $block_attributes['style']['color']['gradient'] ?? null;
$background_gradient_styles['gradient'] = $preset_gradient_color ?: $custom_gradient_color;
$background_gradient_styles['gradient'] = $preset_gradient_color ? $preset_gradient_color : $custom_gradient_color;
}

if (
Expand All @@ -83,7 +83,7 @@ function gutenberg_render_background_support( $block_content, $block ) {
'background' => $background_styles,
'color' => $background_gradient_styles,
),
array( 'convert_vars_to_classnames' => true )
array( 'convert_vars_to_classnames' => ! empty( $preset_gradient_color ) && empty( $background_styles ) )
);

if ( ! empty( $styles['css'] ) ) {
Expand All @@ -92,7 +92,13 @@ function gutenberg_render_background_support( $block_content, $block ) {

if ( $tags->next_tag() ) {
$existing_style = $tags->get_attribute( 'style' );
$updated_style = '';

// Remove any existing background color if a background image and gradient is set.
if ( ! empty( $background_gradient_styles['gradient'] ) && ! empty( $background_styles ) ) {
$existing_style = preg_replace( '/background\s*:\s*' . preg_quote( $background_gradient_styles['gradient'], '/' ) . '\s*;?/', '', $existing_style, 1 );
}

$updated_style = '';

if ( ! empty( $existing_style ) ) {
$updated_style = $existing_style;
Expand All @@ -103,9 +109,6 @@ function gutenberg_render_background_support( $block_content, $block ) {

$updated_style .= $styles['css'];
$tags->set_attribute( 'style', $updated_style );
if ( ! empty( $styles['classnames'] ) ) {
$tags->add_class( $styles['classnames'] );
}
}

return $tags->get_updated_html();
Expand Down
88 changes: 39 additions & 49 deletions lib/class-wp-theme-json-gutenberg.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ class WP_Theme_JSON_Gutenberg {
'use_default_names' => false,
'value_key' => 'gradient',
'css_vars' => '--wp--preset--gradient--$slug',
'classes' => array( '.has-$slug-gradient-background' => 'background-image' ),
'properties' => array( 'background-image' ),
'classes' => array( '.has-$slug-gradient-background' => 'background' ),
'properties' => array( 'background' ),
),
array(
'path' => array( 'color', 'duotone' ),
Expand Down Expand Up @@ -217,11 +217,9 @@ class WP_Theme_JSON_Gutenberg {
*/
const PROPERTIES_METADATA = array(
'aspect-ratio' => array( 'dimensions', 'aspectRatio' ),
'background' => array( 'color', 'gradient' ),
'background-color' => array( 'color', 'background' ),
'background-image' => array(
array( 'color', 'gradient' ),
array( 'background', 'backgroundImage' ),
),
'background-image' => array( 'background', 'backgroundImage' ),
'background-position' => array( 'background', 'backgroundPosition' ),
'background-repeat' => array( 'background', 'backgroundRepeat' ),
'background-size' => array( 'background', 'backgroundSize' ),
Expand All @@ -246,6 +244,7 @@ class WP_Theme_JSON_Gutenberg {
'border-left-width' => array( 'border', 'left', 'width' ),
'border-left-style' => array( 'border', 'left', 'style' ),
'color' => array( 'color', 'text' ),
'text-align' => array( 'typography', 'textAlign' ),
'column-count' => array( 'typography', 'textColumns' ),
'font-family' => array( 'typography', 'fontFamily' ),
'font-size' => array( 'typography', 'fontSize' ),
Expand Down Expand Up @@ -438,6 +437,7 @@ class WP_Theme_JSON_Gutenberg {
'fontWeight' => null,
'letterSpacing' => null,
'lineHeight' => null,
'textAlign' => null,
'textColumns' => null,
'textDecoration' => null,
'textTransform' => null,
Expand Down Expand Up @@ -533,6 +533,7 @@ class WP_Theme_JSON_Gutenberg {
'fontWeight' => null,
'letterSpacing' => null,
'lineHeight' => null,
'textAlign' => null,
'textColumns' => null,
'textDecoration' => null,
'textTransform' => null,
Expand Down Expand Up @@ -1256,7 +1257,7 @@ public function get_stylesheet( $types = array( 'variables', 'styles', 'presets'
);

foreach ( $base_styles_nodes as $base_style_node ) {
$stylesheet .= $this->get_layout_styles( $base_style_node );
$stylesheet .= $this->get_layout_styles( $base_style_node, $types );
}
}

Expand Down Expand Up @@ -1464,7 +1465,7 @@ protected function get_block_classes( $style_nodes ) {
* @param array $block_metadata Metadata about the block to get styles for.
* @return string Layout styles for the block.
*/
protected function get_layout_styles( $block_metadata ) {
protected function get_layout_styles( $block_metadata, $types = array() ) {
$block_rules = '';
$block_type = null;

Expand All @@ -1485,7 +1486,7 @@ protected function get_layout_styles( $block_metadata ) {
$has_fallback_gap_support = ! $has_block_gap_support; // This setting isn't useful yet: it exists as a placeholder for a future explicit fallback gap styles support.
$node = _wp_array_get( $this->theme_json, $block_metadata['path'], array() );
$layout_definitions = gutenberg_get_layout_definitions();
$layout_selector_pattern = '/^[a-zA-Z0-9\-\.\ *+>:\(\)]*$/'; // Allow alphanumeric classnames, spaces, wildcard, sibling, child combinator and pseudo class selectors.
$layout_selector_pattern = '/^[a-zA-Z0-9\-\.\,\ *+>:\(\)]*$/'; // Allow alphanumeric classnames, spaces, wildcard, sibling, child combinator and pseudo class selectors.

// Gap styles will only be output if the theme has block gap support, or supports a fallback gap.
// Default layout gap styles will be skipped for themes that do not explicitly opt-in to blockGap with a `true` or `false` value.
Expand Down Expand Up @@ -1556,7 +1557,7 @@ protected function get_layout_styles( $block_metadata ) {
$spacing_rule['selector']
);
} else {
$format = static::ROOT_BLOCK_SELECTOR === $selector ? ':where(%s .%s) %s' : '%s-%s%s';
$format = static::ROOT_BLOCK_SELECTOR === $selector ? ':where(.%2$s) %3$s' : ':where(%1$s-%2$s) %3$s';
$layout_selector = sprintf(
$format,
$selector,
Expand Down Expand Up @@ -1610,6 +1611,11 @@ protected function get_layout_styles( $block_metadata ) {
foreach ( $base_style_rules as $base_style_rule ) {
$declarations = array();

// Skip outputting base styles for flow and constrained layout types if theme doesn't support theme.json. The 'base-layout-styles' type flags this.
if ( in_array( 'base-layout-styles', $types, true ) && ( 'default' === $layout_definition['name'] || 'constrained' === $layout_definition['name'] ) ) {
continue;
}

if (
isset( $base_style_rule['selector'] ) &&
preg_match( $layout_selector_pattern, $base_style_rule['selector'] ) &&
Expand All @@ -1635,8 +1641,7 @@ protected function get_layout_styles( $block_metadata ) {
}

$layout_selector = sprintf(
'%s .%s%s',
$selector,
'.%s%s',
$class_name,
$base_style_rule['selector']
);
Expand Down Expand Up @@ -2128,24 +2133,10 @@ protected static function compute_style_properties( $styles, $settings = array()
return $declarations;
}

$root_variable_duplicates = array();
$properties_to_remove = array();

foreach ( $properties as $css_property => $value_path ) {
$value = null;
// @TODO how to deal with multiple values that need to be combined?
// background-image: linear-gradient(to right, red, blue), url('foo.png');
// Background images don't support opacity yet officially put it after gradient values.
// Needs to be handled before the first is_array check below.
// Needs to be handled in conjunctions with background styles processing so we get the url value.
// This is a placeholder for now - let's abstract this out later.
// It's very specific to the `background-image` property, so maybe it can be handled in a separate function in background block supports?
// Or value_func like presets? Or in the style engine.
if ( is_array( $value_path ) && is_array( $value_path[0] ) ) {
$merged_styles = gutenberg_style_engine_get_styles( $styles );
$value = $merged_styles['declarations'][$css_property] ?? null;
}

$value = $value ?? static::get_property_value( $styles, $value_path, $theme_json );
$value = static::get_property_value( $styles, $value_path, $theme_json );

if ( str_starts_with( $css_property, '--wp--style--root--' ) && ( static::ROOT_BLOCK_SELECTOR !== $selector || ! $use_root_padding ) ) {
continue;
Expand All @@ -2157,12 +2148,12 @@ protected static function compute_style_properties( $styles, $settings = array()
}

if ( str_starts_with( $css_property, '--wp--style--root--' ) && $use_root_padding ) {
$root_variable_duplicates[] = substr( $css_property, strlen( '--wp--style--root--' ) );
$properties_to_remove[] = substr( $css_property, strlen( '--wp--style--root--' ) );
}

// Look up protected properties, keyed by value path.
// Skip protected properties that are explicitly set to `null`.
if ( is_array( $value_path ) && ! is_array( $value_path[0] ) ) {
if ( is_array( $value_path ) ) {
$path_string = implode( '.', $value_path );
if (
isset( static::PROTECTED_PROPERTIES[ $path_string ] ) &&
Expand All @@ -2172,6 +2163,16 @@ protected static function compute_style_properties( $styles, $settings = array()
}
}

// Processes background styles and merges gradient with `background-image`.
if ( 'background' === $value_path[0] && isset( $styles['background'] ) ) {
$background_styles = gutenberg_style_engine_get_styles( $styles );
$background_image_styles = ! empty( $background_styles['declarations'][ $css_property ] ) ? $background_styles['declarations'][ $css_property ] : null;
if ( $background_image_styles ) {
$value = $background_image_styles;
$properties_to_remove[] = 'background';
}
}

// Skip if empty and not "0" or value represents array of longhand values.
$has_missing_value = empty( $value ) && ! is_numeric( $value );
if ( $has_missing_value || is_array( $value ) ) {
Expand Down Expand Up @@ -2207,7 +2208,7 @@ protected static function compute_style_properties( $styles, $settings = array()
}

// If a variable value is added to the root, the corresponding property should be removed.
foreach ( $root_variable_duplicates as $duplicate ) {
foreach ( $properties_to_remove as $duplicate ) {
$discard = array_search( $duplicate, array_column( $declarations, 'name' ), true );
if ( is_numeric( $discard ) ) {
array_splice( $declarations, $discard, 1 );
Expand Down Expand Up @@ -2794,8 +2795,8 @@ public function get_root_layout_rules( $selector, $block_metadata ) {
if ( isset( $this->theme_json['settings']['spacing']['blockGap'] ) ) {
$block_gap_value = static::get_property_value( $this->theme_json, array( 'styles', 'spacing', 'blockGap' ) );
$css .= ":where(.wp-site-blocks) > * { margin-block-start: $block_gap_value; margin-block-end: 0; }";
$css .= ':where(.wp-site-blocks) > :first-child:first-child { margin-block-start: 0; }';
$css .= ':where(.wp-site-blocks) > :last-child:last-child { margin-block-end: 0; }';
$css .= ':where(.wp-site-blocks) > :first-child { margin-block-start: 0; }';
$css .= ':where(.wp-site-blocks) > :last-child { margin-block-end: 0; }';

// For backwards compatibility, ensure the legacy block gap CSS variable is still available.
$css .= static::ROOT_CSS_PROPERTIES_SELECTOR . " { --wp--style--block-gap: $block_gap_value; }";
Expand Down Expand Up @@ -3300,22 +3301,11 @@ protected static function remove_insecure_styles( $input ) {
if ( static::is_safe_css_declaration( $declaration['name'], $declaration['value'] ) ) {
$path = static::PROPERTIES_METADATA[ $declaration['name'] ];

if ( is_array( $path ) && is_array( $path[0] ) ) {
foreach ( $path as $path_part ) {
// 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_part, array() );
if ( ! is_array( $value ) ) {
_wp_array_set( $output, $path_part, $value );
}
}
} else {
// 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 ) ) {
_wp_array_set( $output, $path, $value );
}
// 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 ) ) {
_wp_array_set( $output, $path, $value );
}
}
}
Expand Down
23 changes: 19 additions & 4 deletions packages/block-editor/src/hooks/color.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,14 @@ export function addSaveProps( props, blockNameOrType, attributes ) {
? getColorClassName( 'color', textColor )
: undefined;

const gradientClass = shouldSerialize( 'gradients' )
? __experimentalGetGradientClass( gradient )
: undefined;
// Do not add gradient class if there is a background image, because the values are merged into `background-image`.
const hasBackgroundImage =
typeof style?.background?.backgroundImage === 'string' ||
typeof style?.background?.backgroundImage?.url === 'string';
const gradientClass =
! hasBackgroundImage && shouldSerialize( 'gradients' )
? __experimentalGetGradientClass( gradient )
: undefined;

const backgroundClass = shouldSerialize( 'background' )
? getColorClassName( 'background-color', backgroundColor )
Expand Down Expand Up @@ -191,6 +196,7 @@ export function addSaveProps( props, blockNameOrType, attributes ) {
shouldSerialize( 'link' ) && style?.elements?.link?.color,
}
);

props.className = newClassName ? newClassName : undefined;

return props;
Expand All @@ -208,15 +214,24 @@ function styleToAttributes( style ) {
? backgroundColorValue.substring( 'var:preset|color|'.length )
: undefined;
const gradientValue = style?.color?.gradient;

// Do not add gradient class if there is a background image, because the values are merged into `background-image`.
const hasBackgroundImage =
typeof style?.background?.backgroundImage === 'string' ||
typeof style?.background?.backgroundImage?.url === 'string';
const gradientSlug = gradientValue?.startsWith( 'var:preset|gradient|' )
? gradientValue.substring( 'var:preset|gradient|'.length )
: undefined;
const updatedStyle = { ...style };

updatedStyle.color = {
...updatedStyle.color,
text: textColorSlug ? undefined : textColorValue,
background: backgroundColorSlug ? undefined : backgroundColorValue,
gradient: gradientSlug ? undefined : gradientValue,
// @TODO this is not quite right. We don't want to add a background style value if there is a gradient.
// But we let the preset var pass to the style engine.
gradient:
! hasBackgroundImage && gradientSlug ? undefined : gradientValue,
};
return {
style: cleanEmptyObject( updatedStyle ),
Expand Down
9 changes: 8 additions & 1 deletion packages/block-editor/src/hooks/use-color-props.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,14 @@ export function getColorClassesAndStyles( attributes ) {
);
const textClass = getColorClassName( 'color', textColor );

const gradientClass = __experimentalGetGradientClass( gradient );
// Do not add gradient class if there is a background image, because the values are merged into `background-image`.
const hasBackgroundImage =
typeof style?.background?.backgroundImage === 'string' ||
typeof style?.background?.backgroundImage?.url === 'string';
const gradientClass = ! hasBackgroundImage
? __experimentalGetGradientClass( gradient )
: undefined;

const hasGradient = gradientClass || style?.color?.gradient;

// Determine color CSS class name list.
Expand Down
13 changes: 10 additions & 3 deletions packages/style-engine/src/styles/color/gradient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,22 @@
*/
import type { Style, StyleOptions } from '../../types';
import { generateRule } from '../utils';
import { VARIABLE_REFERENCE_PREFIX } from '../constants';

const gradient = {
name: 'gradient',
generate: ( style: Style, options: StyleOptions ) => {
// If there's a background image process it via backgroundImage.
const _backgroundImage = style?.background?.backgroundImage;
const hasBackgroundImage =
typeof style?.background?.backgroundImage === 'string' ||
typeof style?.background?.backgroundImage?.url === 'string';
if (
typeof _backgroundImage === 'string' ||
typeof _backgroundImage?.url === 'string'
hasBackgroundImage ||
// Temp. workaround for the case when the gradient is a variable.
// Gradient presets are usually added via the `has-` classnames
// anyway, so it's safe to turn off, I think. Not ideal.
( typeof style?.color?.gradient === 'string' &&
style.color.gradient.startsWith( VARIABLE_REFERENCE_PREFIX ) )
) {
return [];
}
Expand Down

0 comments on commit 3789393

Please sign in to comment.