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

Global styles: resolve block style variation URIs #66731

Draft
wants to merge 6 commits into
base: trunk
Choose a base branch
from
Draft
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
2 changes: 2 additions & 0 deletions lib/block-supports/block-style-variations.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ function gutenberg_render_block_style_variation_support_styles( $parsed_block )
}

$tree = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data();
$tree = WP_Theme_JSON_Resolver_Gutenberg::resolve_theme_file_uris( $tree );
$theme_json = $tree->get_raw_data();

// Only the first block style variation with data is supported.
Expand Down Expand Up @@ -172,6 +173,7 @@ function gutenberg_render_block_style_variation_support_styles( $parsed_block )
$styles_registry->register( $parsed_block['blockName'], array( 'name' => $variation_instance ) );

$variation_theme_json = new WP_Theme_JSON_Gutenberg( $config, 'blocks' );
$variation_theme_json = WP_Theme_JSON_Resolver_Gutenberg::resolve_theme_file_uris( $variation_theme_json );
$variation_styles = $variation_theme_json->get_stylesheet(
array( 'styles' ),
array( 'custom' ),
Expand Down
66 changes: 56 additions & 10 deletions lib/class-wp-theme-json-gutenberg.php
Original file line number Diff line number Diff line change
Expand Up @@ -2602,6 +2602,7 @@ protected static function get_setting_nodes( $theme_json, $selectors = array() )
* @param array $options An array of options to facilitate filtering style node generation
* The options currently supported are:
* - `include_block_style_variations` which includes CSS for block style variations.
* - `include_node_paths_only` which skips the selector generation.
* @return array An array of style nodes metadata.
*/
protected static function get_style_nodes( $theme_json, $selectors = array(), $options = array() ) {
Expand All @@ -2610,11 +2611,16 @@ protected static function get_style_nodes( $theme_json, $selectors = array(), $o
return $nodes;
}

$include_node_paths_only = $options['include_node_paths_only'] ?? false;
Copy link
Member Author

Choose a reason for hiding this comment

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

Needs tests


// Top-level.
$nodes[] = array(
'path' => array( 'styles' ),
'selector' => static::ROOT_BLOCK_SELECTOR,
$node = array(
'path' => array( 'styles' ),
);
if ( ! $include_node_paths_only ) {
$node['selector'] = static::ROOT_BLOCK_SELECTOR;
}
$nodes[] = $node;

if ( isset( $theme_json['styles']['elements'] ) ) {
foreach ( self::ELEMENTS as $element => $selector ) {
Expand All @@ -2623,20 +2629,25 @@ protected static function get_style_nodes( $theme_json, $selectors = array(), $o
}

// Handle element defaults.
$nodes[] = array(
'path' => array( 'styles', 'elements', $element ),
'selector' => static::ELEMENTS[ $element ],
$node = array(
'path' => array( 'styles', 'elements', $element ),
);
if ( ! $include_node_paths_only ) {
$node['selector'] = static::ELEMENTS[ $element ];
}
$nodes[] = $node;

// Handle any pseudo selectors for the element.
if ( isset( static::VALID_ELEMENT_PSEUDO_SELECTORS[ $element ] ) ) {
foreach ( static::VALID_ELEMENT_PSEUDO_SELECTORS[ $element ] as $pseudo_selector ) {

if ( isset( $theme_json['styles']['elements'][ $element ][ $pseudo_selector ] ) ) {
$nodes[] = array(
'path' => array( 'styles', 'elements', $element ),
'selector' => static::append_to_selector( static::ELEMENTS[ $element ], $pseudo_selector ),
$node = array(
'path' => array( 'styles', 'elements', $element ),
);
if ( ! $include_node_paths_only ) {
$node['selector'] = static::append_to_selector( static::ELEMENTS[ $element ], $pseudo_selector );
}
$nodes[] = $node;
}
}
}
Expand Down Expand Up @@ -2676,6 +2687,26 @@ public function get_styles_block_nodes() {
return static::get_block_nodes( $this->theme_json );
}

/**
* A public helper to get all style node paths in theme.json,
* including elements and block styles variations. This is useful
* when iterating over all style nodes to search for or replace any values.
*
* @since 6.8.0
*
* @return array An array of paths to style nodes in theme.json.
*/
public function get_styles_nodes_paths() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Needs tests

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The naming here with three successive plurals seems a bit clunky. What about get_style_node_paths?

This avoids each word within the name "modifying" those before it making it a little less awkward and easier to parse.

return static::get_style_nodes(
$this->theme_json,
array(),
array(
'include_node_paths_only' => true,
'include_block_style_variations' => true,
)
);
}

/**
* Returns a filtered declarations array if there is a separator block with only a background
* style defined in theme.json by adding a color attribute to reflect the changes in the front.
Expand Down Expand Up @@ -2752,6 +2783,21 @@ private static function get_block_nodes( $theme_json, $selectors = array(), $opt
$nodes[] = array(
'path' => $node_path,
);

if ( $include_variations && isset( $node['variations'] ) ) {
foreach ( $node['variations'] as $variation => $node ) {
$nodes[] = array(
'path' => array( 'styles', 'blocks', $name, 'variations', $variation ),
);
if ( isset( $node['blocks'] ) ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Check with @aaronrobertshaw about whether this is all correct 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

It appears to match the original code in the else statement. I'm missing context here though around the inclusion of include_node_paths_only. It looks like the variations part of the equation was mistakenly overlooked during that enhancement.

One thing I haven't grokked yet is where the paths get handled for element styles within block style variations. Were they covered?

Copy link
Member Author

Choose a reason for hiding this comment

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

One thing I haven't grokked yet is where the paths get handled for element styles within block style variations. Were they covered?

No not covered. They'll need to be added. I deliberately skipped over them, because I wasn't sure whether it was appropriate to overload get_block_nodes() in that way when they didn't previously deal with nested block styles or elements in block style variations.

TBD I guess.

It looks like the variations part of the equation was mistakenly overlooked during that enhancement.

That's what I thought - I moved it up to make this PR work, but I suppose it should be a separate PR.

Fortunately it's only used in one place for now WP_Theme_JSON:merge and only in the context of merging background images. So I don't think there's any urgency to it.

foreach ( $node['blocks'] as $variation_block_name => $node ) {
$nodes[] = array(
'path' => array( 'styles', 'blocks', $name, 'variations', $variation, 'blocks', $variation_block_name ),
);
}
}
}
}
} else {
$selector = null;
if ( isset( $selectors[ $name ]['selector'] ) ) {
Expand Down
52 changes: 17 additions & 35 deletions lib/class-wp-theme-json-resolver-gutenberg.php
Original file line number Diff line number Diff line change
Expand Up @@ -855,53 +855,35 @@ public static function get_resolved_theme_uris( $theme_json ) {
// Using the same file convention when registering web fonts. See: WP_Font_Face_Resolver:: to_theme_file_uri.
$placeholder = 'file:./';

// Top level styles.
$background_image_url = $theme_json_data['styles']['background']['backgroundImage']['url'] ?? null;
if (
isset( $background_image_url ) &&
is_string( $background_image_url ) &&
// Skip if the src doesn't start with the placeholder, as there's nothing to replace.
str_starts_with( $background_image_url, $placeholder ) ) {
/*
* Style values are merged at the leaf level, however
* some values provide exceptions, namely style values that are
* objects and represent unique definitions for the style.
*/
$style_nodes = $theme_json->get_styles_nodes_paths();
Copy link
Member Author

Choose a reason for hiding this comment

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

This change can be a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Anything that generally makes theme.json class related PRs easier is fine by me 😅

In this case though the PR isn't huge, so I don't mind reviewing as is.


foreach ( $style_nodes as $style_node ) {
$path = $style_node['path'];
$background_image_path = array_merge( $path, array( 'background', 'backgroundImage', 'url' ) );
$background_image_url = _wp_array_get( $theme_json_data, $background_image_path, null );
if (
isset( $background_image_url ) &&
is_string( $background_image_url ) &&
// Skip if the src doesn't start with the placeholder, as there's nothing to replace.
str_starts_with( $background_image_url, $placeholder ) ) {
$file_type = wp_check_filetype( $background_image_url );
$src_url = str_replace( $placeholder, '', $background_image_url );
$resolved_theme_uri = array(
'name' => $background_image_url,
'href' => sanitize_url( get_theme_file_uri( $src_url ) ),
'target' => 'styles.background.backgroundImage.url',
'target' => implode( '.', $background_image_path ),
);
if ( isset( $file_type['type'] ) ) {
$resolved_theme_uri['type'] = $file_type['type'];
}
$resolved_theme_uris[] = $resolved_theme_uri;
}

// Block styles.
if ( ! empty( $theme_json_data['styles']['blocks'] ) ) {
foreach ( $theme_json_data['styles']['blocks'] as $block_name => $block_styles ) {
if ( ! isset( $block_styles['background']['backgroundImage']['url'] ) ) {
continue;
}
$background_image_url = $block_styles['background']['backgroundImage']['url'] ?? null;
if (
isset( $background_image_url ) &&
is_string( $background_image_url ) &&
// Skip if the src doesn't start with the placeholder, as there's nothing to replace.
str_starts_with( $background_image_url, $placeholder ) ) {
$file_type = wp_check_filetype( $background_image_url );
$src_url = str_replace( $placeholder, '', $background_image_url );
$resolved_theme_uri = array(
'name' => $background_image_url,
'href' => sanitize_url( get_theme_file_uri( $src_url ) ),
'target' => "styles.blocks.{$block_name}.background.backgroundImage.url",
);
if ( isset( $file_type['type'] ) ) {
$resolved_theme_uri['type'] = $file_type['type'];
}
$resolved_theme_uris[] = $resolved_theme_uri;
}
}
}

return $resolved_theme_uris;
}

Expand Down
32 changes: 20 additions & 12 deletions packages/block-editor/src/hooks/block-style-variation.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import {
import { usePrivateStyleOverride } from './utils';
import { getValueFromObjectPath } from '../utils/object';
import { store as blockEditorStore } from '../store';
import { globalStylesDataKey } from '../store/private-keys';
import {
globalStylesDataKey,
globalStylesLinksDataKey,
} from '../store/private-keys';
import { unlock } from '../lock-unlock';

const VARIATION_PREFIX = 'is-style-';
Expand Down Expand Up @@ -86,7 +89,6 @@ export function __unstableBlockStyleVariationOverridesWithConfig( { config } ) {
[]
);
const { getBlockName } = useSelect( blockEditorStore );

const overridesWithConfig = useMemo( () => {
if ( ! overrides?.length ) {
return;
Expand Down Expand Up @@ -257,13 +259,17 @@ function useBlockStyleVariation( name, variation, clientId ) {
// if in the site editor. Otherwise fall back to whatever is in the
// editor settings and available in the post editor.
const { merged: mergedConfig } = useContext( GlobalStylesContext );
const { globalSettings, globalStyles } = useSelect( ( select ) => {
const settings = select( blockEditorStore ).getSettings();
return {
globalSettings: settings.__experimentalFeatures,
globalStyles: settings[ globalStylesDataKey ],
};
}, [] );
const { globalSettings, globalStyles, globalLinks } = useSelect(
( select ) => {
const settings = select( blockEditorStore ).getSettings();
return {
globalSettings: settings.__experimentalFeatures,
globalStyles: settings[ globalStylesDataKey ],
globalLinks: settings[ globalStylesLinksDataKey ],
};
},
[]
);

return useMemo( () => {
const variationStyles = getVariationStylesWithRefValues(
Expand All @@ -289,11 +295,13 @@ function useBlockStyleVariation( name, variation, clientId ) {
},
},
},
_links: mergedConfig?._links ?? globalLinks,
};
}, [
mergedConfig,
globalSettings,
globalStyles,
globalLinks,
variation,
clientId,
name,
Expand All @@ -310,7 +318,7 @@ function useBlockProps( { name, className, clientId } ) {
const variation = getVariationNameFromClass( className, registeredStyles );
const variationClass = `${ VARIATION_PREFIX }${ variation }-${ clientId }`;

const { settings, styles } = useBlockStyleVariation(
const { settings, styles, _links } = useBlockStyleVariation(
name,
variation,
clientId
Expand All @@ -321,7 +329,7 @@ function useBlockProps( { name, className, clientId } ) {
return;
}

const variationConfig = { settings, styles };
const variationConfig = { settings, styles, _links };
const blockSelectors = getBlockSelectors(
getBlockTypes(),
getBlockStyles,
Expand Down Expand Up @@ -349,7 +357,7 @@ function useBlockProps( { name, className, clientId } ) {
variationStyles: true,
}
);
}, [ variation, settings, styles, getBlockStyles, clientId ] );
}, [ variation, settings, styles, _links, getBlockStyles, clientId ] );

usePrivateStyleOverride( {
id: `variation-${ clientId }`,
Expand Down
Loading
Loading