From 7f7ea9a4378f6315d538993ec99ce0627f4649aa Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 30 Jul 2024 22:16:12 +1000 Subject: [PATCH 01/11] - resolve refs before passing styles to style engine in wp_theme_json - add tests - adds tests Check for string types in background image so it will apply defaults for blocks. Update comments --- lib/class-wp-theme-json-gutenberg.php | 40 +++++++++------- .../test/use-global-styles-output.js | 16 +++++++ .../src/styles/background/index.ts | 26 +++++----- phpunit/class-wp-theme-json-test.php | 48 +++++++++++++++++-- 4 files changed, 95 insertions(+), 35 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index 20ea31090407b..0b3c144f543fb 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -2329,7 +2329,7 @@ protected static function flatten_tree( $tree, $prefix = '', $token = '--' ) { * ```php * array( * 'name' => 'property_name', - * 'value' => 'property_value, + * 'value' => 'property_value', * ) * ``` * @@ -2338,6 +2338,7 @@ protected static function flatten_tree( $tree, $prefix = '', $token = '--' ) { * @since 6.1.0 Added `$theme_json`, `$selector`, and `$use_root_padding` parameters. * @since 6.5.0 Output a `min-height: unset` rule when `aspect-ratio` is set. * @since 6.6.0 Passing current theme JSON settings to wp_get_typography_font_size_value(). Using style engine to correctly fetch background CSS values. + * @since 6.7.0 Allow ref resolution of background properties. * * @param array $styles Styles to process. * @param array $settings Theme settings. @@ -2381,23 +2382,26 @@ protected static function compute_style_properties( $styles, $settings = array() $root_variable_duplicates[] = substr( $css_property, $root_style_length ); } - // Processes background styles. - if ( 'background' === $value_path[0] && isset( $styles['background'] ) ) { - /* - * For user-uploaded images at the block level, assign defaults. - * Matches defaults applied in the editor and in block supports: background.php. - */ - if ( static::ROOT_BLOCK_SELECTOR !== $selector && ! empty( $styles['background']['backgroundImage']['id'] ) ) { - $styles['background']['backgroundSize'] = $styles['background']['backgroundSize'] ?? 'cover'; - // If the background size is set to `contain` and no position is set, set the position to `center`. - if ( 'contain' === $styles['background']['backgroundSize'] && empty( $styles['background']['backgroundPosition'] ) ) { - $styles['background']['backgroundPosition'] = '50% 50%'; - } + // Processes background image styles. + if ( 'background-image' === $css_property && ! empty( $value ) ) { + $background_styles = gutenberg_style_engine_get_styles( + array( 'background' => array( 'backgroundImage' => $value ) ) + ); + + $value = $background_styles['declarations'][ $css_property ]; + } + + if ( static::ROOT_BLOCK_SELECTOR !== $selector && ! empty( $styles['background']['backgroundImage']['id'] ) ) { + if ( 'background-size' === $css_property && empty( $value ) ) { + $value = 'cover'; + } + // If the background size is set to `contain` and no position is set, set the position to `center`. + if ( 'background-position' === $css_property && empty( $value ) && 'contain' === $styles['background']['backgroundSize'] ) { + $value = '50% 50%'; } - $background_styles = gutenberg_style_engine_get_styles( array( 'background' => $styles['background'] ) ); - $value = $background_styles['declarations'][ $css_property ] ?? $value; } + // 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 ) ) { @@ -2485,9 +2489,11 @@ protected static function get_property_value( $styles, $path, $theme_json = null */ if ( is_array( $value ) && isset( $value['ref'] ) ) { $value_path = explode( '.', $value['ref'] ); - $ref_value = _wp_array_get( $theme_json, $value_path ); + $ref_value = _wp_array_get( $theme_json, $value_path, null ); + // Background Image refs can refer to a string or an array containing a URL string. + $ref_value_url = $ref_value['url'] ?? null; // Only use the ref value if we find anything. - if ( ! empty( $ref_value ) && is_string( $ref_value ) ) { + if ( ! empty( $ref_value ) && ( is_string( $ref_value ) || is_string( $ref_value_url ) ) ) { $value = $ref_value; } diff --git a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js index 1b061f6921f2c..5022e8ba591db 100644 --- a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js +++ b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js @@ -1008,9 +1008,23 @@ describe( 'global styles renderer', () => { ref: 'styles.elements.h1.typography.letterSpacing', }, }, + background: { + backgroundImage: { + ref: 'styles.background.backgroundImage', + }, + backgroundSize: { + ref: 'styles.background.backgroundSize', + }, + }, }; const tree = { styles: { + background: { + backgroundImage: { + url: 'http://my-image.org/image.gif', + }, + backgroundSize: 'cover', + }, elements: { h1: { typography: { @@ -1026,6 +1040,8 @@ describe( 'global styles renderer', () => { ).toEqual( [ 'font-size: var(--wp--preset--font-size--xx-large)', 'letter-spacing: 2px', + "background-image: url( 'http://my-image.org/image.gif' )", + 'background-size: cover', ] ); } ); it( 'should set default values for block background styles', () => { diff --git a/packages/style-engine/src/styles/background/index.ts b/packages/style-engine/src/styles/background/index.ts index 211b97343d89c..b943032f9c441 100644 --- a/packages/style-engine/src/styles/background/index.ts +++ b/packages/style-engine/src/styles/background/index.ts @@ -8,6 +8,12 @@ const backgroundImage = { name: 'backgroundImage', generate: ( style: Style, options: StyleOptions ) => { const _backgroundImage = style?.background?.backgroundImage; + + /* + * The background image can be a string or an object. + * If the background image is a string, it could already contain a url() function, + * or have a linear-gradient value. + */ if ( typeof _backgroundImage === 'object' && _backgroundImage?.url ) { return [ { @@ -21,20 +27,12 @@ const backgroundImage = { ]; } - /* - * If the background image is a string, it could already contain a url() function, - * or have a linear-gradient value. - */ - if ( typeof _backgroundImage === 'string' ) { - return generateRule( - style, - options, - [ 'background', 'backgroundImage' ], - 'backgroundImage' - ); - } - - return []; + return generateRule( + style, + options, + [ 'background', 'backgroundImage' ], + 'backgroundImage' + ); }, }; diff --git a/phpunit/class-wp-theme-json-test.php b/phpunit/class-wp-theme-json-test.php index 3f11dd97a6688..2d33e2e10b360 100644 --- a/phpunit/class-wp-theme-json-test.php +++ b/phpunit/class-wp-theme-json-test.php @@ -4847,8 +4847,17 @@ public function test_get_block_background_image_styles() { array( 'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA, 'styles' => array( - 'blocks' => array( - 'core/group' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/top.png', + ), + 'backgroundSize' => 'contain', + 'backgroundRepeat' => 'repeat', + 'backgroundPosition' => '10% 20%', + 'backgroundAttachment' => 'scroll', + ), + 'blocks' => array( + 'core/group' => array( 'background' => array( 'backgroundImage' => "url('http://example.org/group.png')", 'backgroundRepeat' => 'no-repeat', @@ -4856,7 +4865,26 @@ public function test_get_block_background_image_styles() { 'backgroundAttachment' => 'fixed', ), ), - 'core/quote' => array( + 'core/post-content' => array( + 'background' => array( + 'backgroundImage' => array( + 'ref' => 'styles.background.backgroundImage', + ), + 'backgroundSize' => array( + 'ref' => 'styles.background.backgroundSize', + ), + 'backgroundRepeat' => array( + 'ref' => 'styles.background.backgroundRepeat', + ), + 'backgroundPosition' => array( + 'ref' => 'styles.background.backgroundPosition', + ), + 'backgroundAttachment' => array( + 'ref' => 'styles.background.backgroundAttachment', + ), + ), + ), + 'core/quote' => array( 'background' => array( 'backgroundImage' => array( 'url' => 'http://example.org/quote.png', @@ -4865,7 +4893,7 @@ public function test_get_block_background_image_styles() { 'backgroundSize' => 'contain', ), ), - 'core/verse' => array( + 'core/verse' => array( 'background' => array( 'backgroundImage' => array( 'url' => 'http://example.org/verse.png', @@ -4899,6 +4927,18 @@ public function test_get_block_background_image_styles() { ), ); + $post_content_node = array( + 'name' => 'core/post-content', + 'path' => array( 'styles', 'blocks', 'core/post-content' ), + 'selector' => '.wp-block-post-content', + 'selectors' => array( + 'root' => '.wp-block-post-content', + ), + ); + + $post_content_styles = ":root :where(.wp-block-post-content){background-image: url('http://example.org/top.png');background-position: 10% 20%;background-repeat: repeat;background-size: contain;background-attachment: scroll;}"; + $this->assertSameCSS( $post_content_styles, $theme_json->get_styles_for_block( $post_content_node ), 'Styles returned from "::get_styles_for_block()" with core/post-content background ref styles as string type do not match expectations' ); + $quote_styles = ":root :where(.wp-block-quote){background-image: url('http://example.org/quote.png');background-position: 50% 50%;background-size: contain;}"; $this->assertSameCSS( $quote_styles, $theme_json->get_styles_for_block( $quote_node ), 'Styles returned from "::get_styles_for_block()" with core/quote default background styles do not match expectations' ); From e359a75e52274c9babce2d9ce06c20eaf77138ba Mon Sep 17 00:00:00 2001 From: ramon Date: Thu, 1 Aug 2024 16:50:34 +1000 Subject: [PATCH 02/11] - consolidate all resolutions in the background panel (image and ref) - remove useGlobalStyleLinks completely - delete theme URI utils Remove string condition LInto --- backport-changelog/6.7/7137.md | 1 + .../global-styles/background-panel.js | 74 +++++++++++++------ .../src/components/global-styles/hooks.js | 5 -- .../src/components/global-styles/index.js | 1 - .../test/theme-file-uri-utils.js | 41 ---------- .../global-styles/theme-file-uri-utils.js | 18 ----- packages/block-editor/src/hooks/background.js | 9 +-- .../global-styles/background-panel.js | 3 - .../components/global-styles/screen-block.js | 3 - 9 files changed, 55 insertions(+), 100 deletions(-) delete mode 100644 packages/block-editor/src/components/global-styles/test/theme-file-uri-utils.js delete mode 100644 packages/block-editor/src/components/global-styles/theme-file-uri-utils.js diff --git a/backport-changelog/6.7/7137.md b/backport-changelog/6.7/7137.md index 1eba52ebaf508..00771b8bc6c21 100644 --- a/backport-changelog/6.7/7137.md +++ b/backport-changelog/6.7/7137.md @@ -1,4 +1,5 @@ https://github.com/WordPress/wordpress-develop/pull/7137 +* https://github.com/WordPress/gutenberg/pull/64128 * https://github.com/WordPress/gutenberg/pull/64192 * https://github.com/WordPress/gutenberg/pull/64328 diff --git a/packages/block-editor/src/components/global-styles/background-panel.js b/packages/block-editor/src/components/global-styles/background-panel.js index 14b37bb300765..1373c54764d15 100644 --- a/packages/block-editor/src/components/global-styles/background-panel.js +++ b/packages/block-editor/src/components/global-styles/background-panel.js @@ -34,6 +34,7 @@ import { useRef, useState, useEffect, + useMemo, } from '@wordpress/element'; import { useDispatch, useSelect } from '@wordpress/data'; import { focus } from '@wordpress/dom'; @@ -42,11 +43,15 @@ import { isBlobURL } from '@wordpress/blob'; /** * Internal dependencies */ -import { useToolsPanelDropdownMenuProps } from './utils'; +import { useToolsPanelDropdownMenuProps, getResolvedValue } from './utils'; import { setImmutably } from '../../utils/object'; import MediaReplaceFlow from '../media-replace-flow'; import { store as blockEditorStore } from '../../store'; -import { getResolvedThemeFilePath } from './theme-file-uri-utils'; + +import { + globalStylesDataKey, + globalStylesLinksDataKey, +} from '../../store/private-keys'; const IMAGE_BACKGROUND_TYPE = 'image'; const DEFAULT_CONTROLS = { @@ -270,7 +275,6 @@ function BackgroundImageControls( { onRemoveImage = noop, onResetImage = noop, displayInPanel, - themeFileURIs, defaultValues, } ) { const mediaUpload = useSelect( @@ -404,10 +408,7 @@ function BackgroundImageControls( { name={ @@ -449,7 +450,6 @@ function BackgroundSizeControls( { style, inheritedValue, defaultValues, - themeFileURIs, } ) { const sizeValue = style?.background?.backgroundSize || @@ -587,7 +587,7 @@ function BackgroundSizeControls( { @@ -697,8 +697,44 @@ export default function BackgroundPanel( { defaultControls = DEFAULT_CONTROLS, defaultValues = {}, headerLabel = __( 'Background image' ), - themeFileURIs, } ) { + /* + * Resolve any inherited "ref" pointers. + * Should the block editor need resolved, inherited values + * across all controls, this could be abstracted into a hook, + * e.g., useResolveGlobalStyle + */ + const { globalStyles, _links } = useSelect( ( select ) => { + const { getSettings } = select( blockEditorStore ); + const _settings = getSettings(); + return { + globalStyles: _settings[ globalStylesDataKey ], + _links: _settings[ globalStylesLinksDataKey ], + }; + }, [] ); + const resolvedInheritedValue = useMemo( () => { + const resolvedValues = { + background: {}, + }; + + if ( ! inheritedValue?.background ) { + return inheritedValue; + } + + Object.entries( inheritedValue?.background ).forEach( + ( [ key, backgroundValue ] ) => { + resolvedValues.background[ key ] = getResolvedValue( + backgroundValue, + { + styles: globalStyles, + _links, + } + ); + } + ); + return resolvedValues; + }, [ globalStyles, _links, inheritedValue ] ); + const resetAllFilter = useCallback( ( previousValue ) => { return { ...previousValue, @@ -710,11 +746,11 @@ export default function BackgroundPanel( { onChange( setImmutably( value, [ 'background' ], {} ) ); const { title, url } = value?.background?.backgroundImage || { - ...inheritedValue?.background?.backgroundImage, + ...resolvedInheritedValue?.background?.backgroundImage, }; const hasImageValue = hasBackgroundImageValue( value ) || - hasBackgroundImageValue( inheritedValue ); + hasBackgroundImageValue( resolvedInheritedValue ); const imageValue = value?.background?.backgroundImage || @@ -756,10 +792,7 @@ export default function BackgroundPanel( { @@ -767,8 +800,7 @@ export default function BackgroundPanel( { { setIsDropDownOpen( false ); @@ -784,8 +816,7 @@ export default function BackgroundPanel( { panelId={ panelId } style={ value } defaultValues={ defaultValues } - inheritedValue={ inheritedValue } - themeFileURIs={ themeFileURIs } + inheritedValue={ resolvedInheritedValue } /> @@ -793,8 +824,7 @@ export default function BackgroundPanel( { { setIsDropDownOpen( false ); diff --git a/packages/block-editor/src/components/global-styles/hooks.js b/packages/block-editor/src/components/global-styles/hooks.js index a1a4fc1a0a6ae..2be77aec18a2c 100644 --- a/packages/block-editor/src/components/global-styles/hooks.js +++ b/packages/block-editor/src/components/global-styles/hooks.js @@ -209,11 +209,6 @@ export function useGlobalStyle( return [ result, setStyle ]; } -export function useGlobalStyleLinks() { - const { merged: mergedConfig } = useContext( GlobalStylesContext ); - return mergedConfig?._links; -} - /** * React hook that overrides a global settings object with block and element specific settings. * diff --git a/packages/block-editor/src/components/global-styles/index.js b/packages/block-editor/src/components/global-styles/index.js index 062df0a5606e9..8096a48569f19 100644 --- a/packages/block-editor/src/components/global-styles/index.js +++ b/packages/block-editor/src/components/global-styles/index.js @@ -3,7 +3,6 @@ export { useGlobalSetting, useGlobalStyle, useSettingsForBlockElement, - useGlobalStyleLinks, } from './hooks'; export { getBlockCSSSelector } from './get-block-css-selector'; export { diff --git a/packages/block-editor/src/components/global-styles/test/theme-file-uri-utils.js b/packages/block-editor/src/components/global-styles/test/theme-file-uri-utils.js deleted file mode 100644 index e239bb0941ea9..0000000000000 --- a/packages/block-editor/src/components/global-styles/test/theme-file-uri-utils.js +++ /dev/null @@ -1,41 +0,0 @@ -/** - * Internal dependencies - */ -import { getResolvedThemeFilePath } from '../theme-file-uri-utils'; - -const themeFileURIs = [ - { - name: 'file:./assets/image.jpg', - href: 'https://wordpress.org/assets/image.jpg', - target: 'styles.background.backgroundImage.url', - }, - { - name: 'file:./assets/other/image.jpg', - href: 'https://wordpress.org/assets/other/image.jpg', - target: "styles.blocks.['core/group].background.backgroundImage.url", - }, -]; - -describe( 'getResolvedThemeFilePath()', () => { - it.each( [ - [ - 'file:./assets/image.jpg', - 'https://wordpress.org/assets/image.jpg', - 'Should return absolute URL if found in themeFileURIs', - ], - [ - 'file:./misc/image.jpg', - 'file:./misc/image.jpg', - 'Should return value if not found in themeFileURIs', - ], - [ - 'https://wordpress.org/assets/image.jpg', - 'https://wordpress.org/assets/image.jpg', - 'Should not match absolute URLs', - ], - ] )( 'Given file %s and return value %s: %s', ( file, returnedValue ) => { - expect( - getResolvedThemeFilePath( file, themeFileURIs ) === returnedValue - ).toBe( true ); - } ); -} ); diff --git a/packages/block-editor/src/components/global-styles/theme-file-uri-utils.js b/packages/block-editor/src/components/global-styles/theme-file-uri-utils.js deleted file mode 100644 index 96b3e2e4cb68b..0000000000000 --- a/packages/block-editor/src/components/global-styles/theme-file-uri-utils.js +++ /dev/null @@ -1,18 +0,0 @@ -/** - * Looks up a theme file URI based on a relative path. - * - * @param {string} file A relative path. - * @param {Array} themeFileURIs A collection of absolute theme file URIs and their corresponding file paths. - * @return {string?} A resolved theme file URI, if one is found in the themeFileURIs collection. - */ -export function getResolvedThemeFilePath( file, themeFileURIs = [] ) { - const uri = themeFileURIs.find( - ( themeFileUri ) => themeFileUri.name === file - ); - - if ( ! uri?.href ) { - return file; - } - - return uri?.href; -} diff --git a/packages/block-editor/src/hooks/background.js b/packages/block-editor/src/hooks/background.js index 0d38068cdefee..3755aecbcb9d0 100644 --- a/packages/block-editor/src/hooks/background.js +++ b/packages/block-editor/src/hooks/background.js @@ -16,10 +16,7 @@ import { useHasBackgroundPanel, hasBackgroundImageValue, } from '../components/global-styles/background-panel'; -import { - globalStylesDataKey, - globalStylesLinksDataKey, -} from '../store/private-keys'; +import { globalStylesDataKey } from '../store/private-keys'; export const BACKGROUND_SUPPORT_KEY = 'background'; @@ -136,14 +133,13 @@ export function BackgroundImagePanel( { setAttributes, settings, } ) { - const { style, inheritedValue, _links } = useSelect( + const { style, inheritedValue } = useSelect( ( select ) => { const { getBlockAttributes, getSettings } = select( blockEditorStore ); const _settings = getSettings(); return { style: getBlockAttributes( clientId )?.style, - _links: _settings[ globalStylesLinksDataKey ], /* * To ensure we pass down the right inherited values: * @TODO 1. Pass inherited value down to all block style controls, @@ -190,7 +186,6 @@ export function BackgroundImagePanel( { settings={ updatedSettings } onChange={ onChange } value={ style } - themeFileURIs={ _links?.[ 'wp:theme-file' ] } /> ); } diff --git a/packages/edit-site/src/components/global-styles/background-panel.js b/packages/edit-site/src/components/global-styles/background-panel.js index 24ab914bed8c5..e185079d8cee0 100644 --- a/packages/edit-site/src/components/global-styles/background-panel.js +++ b/packages/edit-site/src/components/global-styles/background-panel.js @@ -16,7 +16,6 @@ const BACKGROUND_DEFAULT_VALUES = { const { useGlobalStyle, useGlobalSetting, - useGlobalStyleLinks, BackgroundPanel: StylesBackgroundPanel, } = unlock( blockEditorPrivateApis ); @@ -42,7 +41,6 @@ export default function BackgroundPanel() { const [ inheritedStyle, setStyle ] = useGlobalStyle( '', undefined, 'all', { shouldDecodeEncode: false, } ); - const _links = useGlobalStyleLinks(); const [ settings ] = useGlobalSetting( '' ); return ( @@ -52,7 +50,6 @@ export default function BackgroundPanel() { onChange={ setStyle } settings={ settings } defaultValues={ BACKGROUND_DEFAULT_VALUES } - themeFileURIs={ _links?.[ 'wp:theme-file' ] } /> ); } diff --git a/packages/edit-site/src/components/global-styles/screen-block.js b/packages/edit-site/src/components/global-styles/screen-block.js index dee921f37f1e8..b1489167f2dc7 100644 --- a/packages/edit-site/src/components/global-styles/screen-block.js +++ b/packages/edit-site/src/components/global-styles/screen-block.js @@ -85,7 +85,6 @@ const { FiltersPanel: StylesFiltersPanel, ImageSettingsPanel, AdvancedPanel: StylesAdvancedPanel, - useGlobalStyleLinks, } = unlock( blockEditorPrivateApis ); function ScreenBlock( { name, variation } ) { @@ -105,7 +104,6 @@ function ScreenBlock( { name, variation } ) { const [ rawSettings, setSettings ] = useGlobalSetting( '', name ); const settings = useSettingsForBlockElement( rawSettings, name ); const blockType = getBlockType( name ); - const _links = useGlobalStyleLinks(); // Only allow `blockGap` support if serialization has not been skipped, to be sure global spacing can be applied. if ( @@ -272,7 +270,6 @@ function ScreenBlock( { name, variation } ) { onChange={ setStyle } settings={ settings } defaultValues={ BACKGROUND_BLOCK_DEFAULT_VALUES } - themeFileURIs={ _links?.[ 'wp:theme-file' ] } /> ) } { hasTypographyPanel && ( From 550098a6f3ab12e45f80d79297a00847444d6eab Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 12 Aug 2024 15:03:53 +1000 Subject: [PATCH 03/11] Reconcile global styles default value assignment an ref resolution after rebase. --- lib/class-wp-theme-json-gutenberg.php | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index 0b3c144f543fb..2c495daf464ee 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -2382,7 +2382,12 @@ protected static function compute_style_properties( $styles, $settings = array() $root_variable_duplicates[] = substr( $css_property, $root_style_length ); } - // Processes background image styles. + /* + * Processes background image styles. + * If the value is a URL, it will be converted to a CSS `url()` value. + * For uploaded image (images with a database ID), apply size and position defaults, + * equal to those applied in block supports in lib/background.php. + */ if ( 'background-image' === $css_property && ! empty( $value ) ) { $background_styles = gutenberg_style_engine_get_styles( array( 'background' => array( 'backgroundImage' => $value ) ) @@ -2390,18 +2395,16 @@ protected static function compute_style_properties( $styles, $settings = array() $value = $background_styles['declarations'][ $css_property ]; } - - if ( static::ROOT_BLOCK_SELECTOR !== $selector && ! empty( $styles['background']['backgroundImage']['id'] ) ) { - if ( 'background-size' === $css_property && empty( $value ) ) { + if ( empty( $value ) && static::ROOT_BLOCK_SELECTOR !== $selector && ! empty( $styles['background']['backgroundImage']['id'] ) ) { + if ( 'background-size' === $css_property ) { $value = 'cover'; } // If the background size is set to `contain` and no position is set, set the position to `center`. - if ( 'background-position' === $css_property && empty( $value ) && 'contain' === $styles['background']['backgroundSize'] ) { + if ( 'background-position' === $css_property && 'contain' === $styles['background']['backgroundSize'] ) { $value = '50% 50%'; } } - // 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 ) ) { @@ -2467,6 +2470,7 @@ protected static function compute_style_properties( $styles, $settings = array() * @since 5.8.0 * @since 5.9.0 Added support for values of array type, which are returned as is. * @since 6.1.0 Added the `$theme_json` parameter. + * @since 6.7.0 Added support for background image refs * * @param array $styles Styles subtree. * @param array $path Which property to process. @@ -2484,10 +2488,12 @@ protected static function get_property_value( $styles, $path, $theme_json = null /* * This converts references to a path to the value at that path - * where the values is an array with a "ref" key, pointing to a path. + * where the value is an array with a "ref" key, pointing to a path. * For example: { "ref": "style.color.background" } => "#fff". + * In the case of backgroundImage, if both a ref and a URL are present in the value, + * the URL takes precedence. */ - if ( is_array( $value ) && isset( $value['ref'] ) ) { + if ( is_array( $value ) && isset( $value['ref'] ) && ! isset( $value['url'] ) ) { $value_path = explode( '.', $value['ref'] ); $ref_value = _wp_array_get( $theme_json, $value_path, null ); // Background Image refs can refer to a string or an array containing a URL string. From bc8dcc3cdc982e2805a515bd2a0cfecc233dad4c Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 12 Aug 2024 15:37:28 +1000 Subject: [PATCH 04/11] Update tests and perform check for background size before use. --- lib/class-wp-theme-json-gutenberg.php | 5 +++-- phpunit/class-wp-theme-json-test.php | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index 2c495daf464ee..ee3e32a21b3ed 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -2400,8 +2400,9 @@ protected static function compute_style_properties( $styles, $settings = array() $value = 'cover'; } // If the background size is set to `contain` and no position is set, set the position to `center`. - if ( 'background-position' === $css_property && 'contain' === $styles['background']['backgroundSize'] ) { - $value = '50% 50%'; + if ( 'background-position' === $css_property ) { + $background_size = $styles['background']['backgroundSize'] ?? null; + $value = 'contain' === $background_size ? '50% 50%' : null; } } diff --git a/phpunit/class-wp-theme-json-test.php b/phpunit/class-wp-theme-json-test.php index 2d33e2e10b360..7500663f313e8 100644 --- a/phpunit/class-wp-theme-json-test.php +++ b/phpunit/class-wp-theme-json-test.php @@ -4898,6 +4898,8 @@ public function test_get_block_background_image_styles() { 'backgroundImage' => array( 'url' => 'http://example.org/verse.png', 'id' => 123, + // Merged theme.json and global styles will retain the "ref" value. + 'ref' => 'styles.background.backgroundImage', ), ), ), From fb65340b89c3f8bcd6aa254f2f8b164b96367e9c Mon Sep 17 00:00:00 2001 From: ramon Date: Mon, 12 Aug 2024 17:24:23 +1000 Subject: [PATCH 05/11] Fixes a but where a ref value will be resolved even where a url value exists. This will occur when base and user configs are merged. Update tests and perform check for background size before use. Update tests and perform check for background size before use. --- lib/class-wp-theme-json-gutenberg.php | 8 +- .../components/global-styles/test/utils.js | 14 +++ .../src/components/global-styles/utils.js | 12 ++ phpunit/class-wp-theme-json-test.php | 109 ++++++++++-------- 4 files changed, 95 insertions(+), 48 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index ee3e32a21b3ed..ce96a3a1afdfd 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -2492,9 +2492,13 @@ protected static function get_property_value( $styles, $path, $theme_json = null * where the value is an array with a "ref" key, pointing to a path. * For example: { "ref": "style.color.background" } => "#fff". * In the case of backgroundImage, if both a ref and a URL are present in the value, - * the URL takes precedence. + * the URL takes precedence and the ref is ignored. */ - if ( is_array( $value ) && isset( $value['ref'] ) && ! isset( $value['url'] ) ) { + if ( is_array( $value ) && isset( $value['ref'] ) ) { + if ( isset( $value['url'] ) ) { + unset( $value['ref'] ); + return $value; + } $value_path = explode( '.', $value['ref'] ); $ref_value = _wp_array_get( $theme_json, $value_path, null ); // Background Image refs can refer to a string or an array containing a URL string. diff --git a/packages/block-editor/src/components/global-styles/test/utils.js b/packages/block-editor/src/components/global-styles/test/utils.js index 08ed1f18e8040..7da09ba0572aa 100644 --- a/packages/block-editor/src/components/global-styles/test/utils.js +++ b/packages/block-editor/src/components/global-styles/test/utils.js @@ -444,6 +444,20 @@ describe( 'editor utils', () => { { url: 'file:./assets/image.jpg' }, themeJson, ], + /* + * Merged theme.json and global styles retain the "ref" value, + * even though the URL is provided in the global styles. + */ + [ + { + ref: 'styles.background.backgroundImage', + url: 'https://wordpress.org/assets/image.jpg', + }, + { + url: 'https://wordpress.org/assets/image.jpg', + }, + themeJson, + ], [ { ref: 'styles.blocks.core/group.background.backgroundImage', diff --git a/packages/block-editor/src/components/global-styles/utils.js b/packages/block-editor/src/components/global-styles/utils.js index 4cd93357b081b..1f6388d472fea 100644 --- a/packages/block-editor/src/components/global-styles/utils.js +++ b/packages/block-editor/src/components/global-styles/utils.js @@ -562,7 +562,19 @@ export function getResolvedRefValue( ruleValue, tree ) { return ruleValue; } + /* + * This converts references to a path to the value at that path + * where the value is an array with a "ref" key, pointing to a path. + * For example: { "ref": "style.color.background" } => "#fff". + * In the case of backgroundImage, if both a ref and a URL are present in the value, + * the URL takes precedence and the ref is ignored. + */ if ( typeof ruleValue !== 'string' && ruleValue?.ref ) { + if ( 'string' === typeof ruleValue?.url ) { + delete ruleValue.ref; + return ruleValue; + } + const refPath = ruleValue.ref.split( '.' ); const resolvedRuleValue = getCSSValueFromRawStyle( getValueFromObjectPath( tree, refPath ) diff --git a/phpunit/class-wp-theme-json-test.php b/phpunit/class-wp-theme-json-test.php index 7500663f313e8..0231c3b3e0579 100644 --- a/phpunit/class-wp-theme-json-test.php +++ b/phpunit/class-wp-theme-json-test.php @@ -4847,17 +4847,8 @@ public function test_get_block_background_image_styles() { array( 'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA, 'styles' => array( - 'background' => array( - 'backgroundImage' => array( - 'url' => 'http://example.org/top.png', - ), - 'backgroundSize' => 'contain', - 'backgroundRepeat' => 'repeat', - 'backgroundPosition' => '10% 20%', - 'backgroundAttachment' => 'scroll', - ), - 'blocks' => array( - 'core/group' => array( + 'blocks' => array( + 'core/group' => array( 'background' => array( 'backgroundImage' => "url('http://example.org/group.png')", 'backgroundRepeat' => 'no-repeat', @@ -4865,26 +4856,7 @@ public function test_get_block_background_image_styles() { 'backgroundAttachment' => 'fixed', ), ), - 'core/post-content' => array( - 'background' => array( - 'backgroundImage' => array( - 'ref' => 'styles.background.backgroundImage', - ), - 'backgroundSize' => array( - 'ref' => 'styles.background.backgroundSize', - ), - 'backgroundRepeat' => array( - 'ref' => 'styles.background.backgroundRepeat', - ), - 'backgroundPosition' => array( - 'ref' => 'styles.background.backgroundPosition', - ), - 'backgroundAttachment' => array( - 'ref' => 'styles.background.backgroundAttachment', - ), - ), - ), - 'core/quote' => array( + 'core/quote' => array( 'background' => array( 'backgroundImage' => array( 'url' => 'http://example.org/quote.png', @@ -4893,13 +4865,11 @@ public function test_get_block_background_image_styles() { 'backgroundSize' => 'contain', ), ), - 'core/verse' => array( + 'core/verse' => array( 'background' => array( 'backgroundImage' => array( 'url' => 'http://example.org/verse.png', 'id' => 123, - // Merged theme.json and global styles will retain the "ref" value. - 'ref' => 'styles.background.backgroundImage', ), ), ), @@ -4929,18 +4899,6 @@ public function test_get_block_background_image_styles() { ), ); - $post_content_node = array( - 'name' => 'core/post-content', - 'path' => array( 'styles', 'blocks', 'core/post-content' ), - 'selector' => '.wp-block-post-content', - 'selectors' => array( - 'root' => '.wp-block-post-content', - ), - ); - - $post_content_styles = ":root :where(.wp-block-post-content){background-image: url('http://example.org/top.png');background-position: 10% 20%;background-repeat: repeat;background-size: contain;background-attachment: scroll;}"; - $this->assertSameCSS( $post_content_styles, $theme_json->get_styles_for_block( $post_content_node ), 'Styles returned from "::get_styles_for_block()" with core/post-content background ref styles as string type do not match expectations' ); - $quote_styles = ":root :where(.wp-block-quote){background-image: url('http://example.org/quote.png');background-position: 50% 50%;background-size: contain;}"; $this->assertSameCSS( $quote_styles, $theme_json->get_styles_for_block( $quote_node ), 'Styles returned from "::get_styles_for_block()" with core/quote default background styles do not match expectations' ); @@ -4957,6 +4915,65 @@ public function test_get_block_background_image_styles() { $this->assertSameCSS( $verse_styles, $theme_json->get_styles_for_block( $verse_node ), 'Styles returned from "::get_styles_for_block()" with default core/verse background styles as string type do not match expectations' ); } + /** + * Testing background dynamic properties in theme.json. + */ + public function test_get_resolved_background_image_styles() { + $theme_json = new WP_Theme_JSON_Gutenberg( + array( + 'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA, + 'styles' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/top.png', + ), + 'backgroundSize' => 'contain', + 'backgroundRepeat' => 'repeat', + 'backgroundPosition' => '10% 20%', + 'backgroundAttachment' => 'scroll', + ), + 'blocks' => array( + 'core/group' => array( + 'background' => array( + 'backgroundImage' => array( + /* + * Merged theme.json and global styles retain the "ref" value, + * even though the URL is provided in the global styles. + */ + 'id' => 123, + 'ref' => 'styles.background.backgroundImage', + 'url' => 'http://example.org/group.png', + ), + ), + ), + 'core/post-content' => array( + 'background' => array( + 'backgroundImage' => array( + 'ref' => 'styles.background.backgroundImage', + ), + 'backgroundSize' => array( + 'ref' => 'styles.background.backgroundSize', + ), + 'backgroundRepeat' => array( + 'ref' => 'styles.background.backgroundRepeat', + ), + 'backgroundPosition' => array( + 'ref' => 'styles.background.backgroundPosition', + ), + 'backgroundAttachment' => array( + 'ref' => 'styles.background.backgroundAttachment', + ), + ), + ), + ), + ), + ) + ); + + $expected = "html{min-height: calc(100% - var(--wp-admin--admin-bar--height, 0px));}body{background-image: url('http://example.org/top.png');background-position: 10% 20%;background-repeat: repeat;background-size: contain;background-attachment: scroll;}:root :where(.wp-block-group){background-image: url('http://example.org/group.png');background-size: cover;}:root :where(.wp-block-post-content){background-image: url('http://example.org/top.png');background-position: 10% 20%;background-repeat: repeat;background-size: contain;background-attachment: scroll;}"; + $this->assertSameCSS( $expected, $theme_json->get_stylesheet( array( 'styles' ), null, array( 'skip_root_layout_styles' => true ) ) ); + } + /** * Tests that base custom CSS is generated correctly. */ From cb34d729909539e3d6ff4e069963861c097675ae Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Aug 2024 15:53:11 +1000 Subject: [PATCH 06/11] The commit tries to ensure that backgroundImage style objects are not merged, but replaced when merged theme.json configs. So, for example, an incoming config such as a saved user config should overwrite the background styles of the base theme.json, where it exists. --- lib/class-wp-theme-json-gutenberg.php | 27 ++++- .../components/global-styles/test/utils.js | 14 --- .../src/components/global-styles/utils.js | 7 -- .../global-styles-provider/index.js | 19 ++- phpunit/class-wp-theme-json-test.php | 109 +++++++++++++++++- 5 files changed, 141 insertions(+), 35 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index ce96a3a1afdfd..e86a3295f3ef4 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -2491,14 +2491,8 @@ protected static function get_property_value( $styles, $path, $theme_json = null * This converts references to a path to the value at that path * where the value is an array with a "ref" key, pointing to a path. * For example: { "ref": "style.color.background" } => "#fff". - * In the case of backgroundImage, if both a ref and a URL are present in the value, - * the URL takes precedence and the ref is ignored. */ if ( is_array( $value ) && isset( $value['ref'] ) ) { - if ( isset( $value['url'] ) ) { - unset( $value['ref'] ); - return $value; - } $value_path = explode( '.', $value['ref'] ); $ref_value = _wp_array_get( $theme_json, $value_path, null ); // Background Image refs can refer to a string or an array containing a URL string. @@ -3264,6 +3258,27 @@ public function merge( $incoming ) { } } } + + $blocks_metadata = static::get_blocks_metadata(); + $style_nodes = static::get_style_nodes( + $incoming_data, + $blocks_metadata, + array( + 'include_block_style_variations' => true, + ) + ); + foreach ( $style_nodes as $style_node ) { + $path = $style_node['path']; + /* + * Background image styles should be replaced, not merged, + * as they themselves are specific object definitions for the style. + */ + $background_image_path = array_merge( $path, static::PROPERTIES_METADATA['background-image'] ); + $content = _wp_array_get( $incoming_data, $background_image_path, null ); + if ( isset( $content ) ) { + _wp_array_set( $this->theme_json, $background_image_path, $content ); + } + } } /** diff --git a/packages/block-editor/src/components/global-styles/test/utils.js b/packages/block-editor/src/components/global-styles/test/utils.js index 7da09ba0572aa..08ed1f18e8040 100644 --- a/packages/block-editor/src/components/global-styles/test/utils.js +++ b/packages/block-editor/src/components/global-styles/test/utils.js @@ -444,20 +444,6 @@ describe( 'editor utils', () => { { url: 'file:./assets/image.jpg' }, themeJson, ], - /* - * Merged theme.json and global styles retain the "ref" value, - * even though the URL is provided in the global styles. - */ - [ - { - ref: 'styles.background.backgroundImage', - url: 'https://wordpress.org/assets/image.jpg', - }, - { - url: 'https://wordpress.org/assets/image.jpg', - }, - themeJson, - ], [ { ref: 'styles.blocks.core/group.background.backgroundImage', diff --git a/packages/block-editor/src/components/global-styles/utils.js b/packages/block-editor/src/components/global-styles/utils.js index 1f6388d472fea..0847375e94dbe 100644 --- a/packages/block-editor/src/components/global-styles/utils.js +++ b/packages/block-editor/src/components/global-styles/utils.js @@ -566,15 +566,8 @@ export function getResolvedRefValue( ruleValue, tree ) { * This converts references to a path to the value at that path * where the value is an array with a "ref" key, pointing to a path. * For example: { "ref": "style.color.background" } => "#fff". - * In the case of backgroundImage, if both a ref and a URL are present in the value, - * the URL takes precedence and the ref is ignored. */ if ( typeof ruleValue !== 'string' && ruleValue?.ref ) { - if ( 'string' === typeof ruleValue?.url ) { - delete ruleValue.ref; - return ruleValue; - } - const refPath = ruleValue.ref.split( '.' ); const resolvedRuleValue = getCSSValueFromRawStyle( getValueFromObjectPath( tree, refPath ) diff --git a/packages/editor/src/components/global-styles-provider/index.js b/packages/editor/src/components/global-styles-provider/index.js index 8ac292fb2ce0b..8426593d8f5f5 100644 --- a/packages/editor/src/components/global-styles-provider/index.js +++ b/packages/editor/src/components/global-styles-provider/index.js @@ -23,10 +23,23 @@ const { GlobalStylesContext, cleanEmptyObject } = unlock( export function mergeBaseAndUserConfigs( base, user ) { return deepmerge( base, user, { - // We only pass as arrays the presets, - // in which case we want the new array of values - // to override the old array (no merging). + /* + * We only pass as arrays the presets, + * in which case we want the new array of values + * to override the old array (no merging). + */ isMergeableObject: isPlainObject, + /* + * Exceptions to the above rule. + * Background images should be replaced, not merged, + * as they themselves are specific object definitions for the style. + */ + customMerge: ( key ) => { + if ( key === 'backgroundImage' ) { + return ( baseConfig, userConfig ) => userConfig; + } + return undefined; + }, } ); } diff --git a/phpunit/class-wp-theme-json-test.php b/phpunit/class-wp-theme-json-test.php index 0231c3b3e0579..2d921a0b7304c 100644 --- a/phpunit/class-wp-theme-json-test.php +++ b/phpunit/class-wp-theme-json-test.php @@ -2294,6 +2294,110 @@ public function test_merge_incoming_data_presets_use_default_names() { $this->assertSameSetsWithIndex( $expected, $actual ); } + public function test_merge_incoming_styles() { + $theme_json = new WP_Theme_JSON_Gutenberg( + array( + 'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA, + 'styles' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/quote.png', + ), + 'backgroundSize' => 'cover', + ), + 'typography' => array( + 'fontSize' => '10px', + ), + 'core/group' => array( + 'background' => array( + 'backgroundImage' => array( + 'ref' => 'styles.blocks.core/verse.background.backgroundImage', + ), + 'backgroundAttachment' => 'fixed', + ), + ), + 'core/quote' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/quote.png', + ), + 'backgroundAttachment' => array( + 'ref' => 'styles.blocks.core/group.background.backgroundAttachment', + ), + ), + ), + 'core/verse' => array( + 'background' => array( + 'backgroundImage' => "url(''http://example.org/verse.png')", + ), + ), + ), + ) + ); + + $update_background_image_styles = array( + 'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA, + 'styles' => array( + 'background' => array( + 'backgroundSize' => 'contain', + ), + 'typography' => array( + 'fontSize' => '12px', + ), + 'blocks' => array( + 'core/group' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/group.png', + ), + ), + ), + 'core/verse' => array( + 'background' => array( + 'backgroundImage' => array( + 'ref' => 'styles.blocks.core/group.background.backgroundImage', + ), + ), + ), + ), + ), + ); + $expected = array( + 'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA, + 'styles' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/quote.png', + ), + 'backgroundSize' => 'contain', + ), + 'typography' => array( + 'fontSize' => '12px', + ), + 'blocks' => array( + 'core/group' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/group.png', + ), + ), + ), + 'core/verse' => array( + 'background' => array( + 'backgroundImage' => array( + 'ref' => 'styles.blocks.core/group.background.backgroundImage', + ), + ), + ), + ), + ), + ); + $theme_json->merge( new WP_Theme_JSON_Gutenberg( $update_background_image_styles ) ); + $actual = $theme_json->get_raw_data(); + + $this->assertEqualSetsWithIndex( $expected, $actual ); + } + public function test_remove_insecure_properties_removes_unsafe_styles() { $actual = WP_Theme_JSON_Gutenberg::remove_insecure_properties( array( @@ -4936,12 +5040,7 @@ public function test_get_resolved_background_image_styles() { 'core/group' => array( 'background' => array( 'backgroundImage' => array( - /* - * Merged theme.json and global styles retain the "ref" value, - * even though the URL is provided in the global styles. - */ 'id' => 123, - 'ref' => 'styles.background.backgroundImage', 'url' => 'http://example.org/group.png', ), ), From 87bb56b6b7a0cb128e538afc611dc74b784b35c4 Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Aug 2024 17:21:50 +1000 Subject: [PATCH 07/11] Fix bug where the style node list was being filtered, add comment. --- lib/class-wp-theme-json-gutenberg.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index e86a3295f3ef4..1b06e761c41c2 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -3259,14 +3259,12 @@ public function merge( $incoming ) { } } - $blocks_metadata = static::get_blocks_metadata(); - $style_nodes = static::get_style_nodes( - $incoming_data, - $blocks_metadata, - array( - 'include_block_style_variations' => true, - ) - ); + /* + * 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 = static::get_styles_block_nodes(); foreach ( $style_nodes as $style_node ) { $path = $style_node['path']; /* From aa7be71a713cd011fad23a6f7a6a87b75c7941ee Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 13 Aug 2024 17:17:45 +0700 Subject: [PATCH 08/11] Fix unit test for merging background image styles --- phpunit/class-wp-theme-json-test.php | 49 ++++++++++++++++++---------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/phpunit/class-wp-theme-json-test.php b/phpunit/class-wp-theme-json-test.php index 2d921a0b7304c..f8fa9511b1b82 100644 --- a/phpunit/class-wp-theme-json-test.php +++ b/phpunit/class-wp-theme-json-test.php @@ -2308,27 +2308,29 @@ public function test_merge_incoming_styles() { 'typography' => array( 'fontSize' => '10px', ), - 'core/group' => array( - 'background' => array( - 'backgroundImage' => array( - 'ref' => 'styles.blocks.core/verse.background.backgroundImage', + 'blocks' => array( + 'core/group' => array( + 'background' => array( + 'backgroundImage' => array( + 'ref' => 'styles.blocks.core/verse.background.backgroundImage', + ), + 'backgroundAttachment' => 'fixed', ), - 'backgroundAttachment' => 'fixed', ), - ), - 'core/quote' => array( - 'background' => array( - 'backgroundImage' => array( - 'url' => 'http://example.org/quote.png', - ), - 'backgroundAttachment' => array( - 'ref' => 'styles.blocks.core/group.background.backgroundAttachment', + 'core/quote' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/quote.png', + ), + 'backgroundAttachment' => array( + 'ref' => 'styles.blocks.core/group.background.backgroundAttachment', + ), ), ), - ), - 'core/verse' => array( - 'background' => array( - 'backgroundImage' => "url(''http://example.org/verse.png')", + 'core/verse' => array( + 'background' => array( + 'backgroundImage' => "url('http://example.org/verse.png')", + ), ), ), ), @@ -2377,9 +2379,20 @@ public function test_merge_incoming_styles() { 'blocks' => array( 'core/group' => array( 'background' => array( - 'backgroundImage' => array( + 'backgroundImage' => array( 'url' => 'http://example.org/group.png', ), + 'backgroundAttachment' => 'fixed', + ), + ), + 'core/quote' => array( + 'background' => array( + 'backgroundImage' => array( + 'url' => 'http://example.org/quote.png', + ), + 'backgroundAttachment' => array( + 'ref' => 'styles.blocks.core/group.background.backgroundAttachment', + ), ), ), 'core/verse' => array( From 70ee62209248e906117bff24146e29ef74bada44 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 13 Aug 2024 17:34:23 +0700 Subject: [PATCH 09/11] Tweak wording for readability --- lib/class-wp-theme-json-gutenberg.php | 4 ++-- packages/block-editor/src/components/global-styles/utils.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index 1b06e761c41c2..d2c9370f1c5bb 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -2488,8 +2488,8 @@ protected static function get_property_value( $styles, $path, $theme_json = null } /* - * This converts references to a path to the value at that path - * where the value is an array with a "ref" key, pointing to a path. + * Where the current value is an array with a 'ref' key pointing + * to a path, this converts that path into the value at that path. * For example: { "ref": "style.color.background" } => "#fff". */ if ( is_array( $value ) && isset( $value['ref'] ) ) { diff --git a/packages/block-editor/src/components/global-styles/utils.js b/packages/block-editor/src/components/global-styles/utils.js index 0847375e94dbe..59799c9032c67 100644 --- a/packages/block-editor/src/components/global-styles/utils.js +++ b/packages/block-editor/src/components/global-styles/utils.js @@ -563,8 +563,8 @@ export function getResolvedRefValue( ruleValue, tree ) { } /* - * This converts references to a path to the value at that path - * where the value is an array with a "ref" key, pointing to a path. + * Where the rule value is an object with a 'ref' property pointing + * to a path, this converts that path into the value at that path. * For example: { "ref": "style.color.background" } => "#fff". */ if ( typeof ruleValue !== 'string' && ruleValue?.ref ) { From 2099c31cf27540406446c89059312ec473a3d3f5 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Tue, 13 Aug 2024 17:35:49 +0700 Subject: [PATCH 10/11] Tweak comment --- lib/class-wp-theme-json-gutenberg.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index d2c9370f1c5bb..756ef06c80aa8 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -2385,8 +2385,8 @@ protected static function compute_style_properties( $styles, $settings = array() /* * Processes background image styles. * If the value is a URL, it will be converted to a CSS `url()` value. - * For uploaded image (images with a database ID), apply size and position defaults, - * equal to those applied in block supports in lib/background.php. + * For an uploaded image (images with a database ID), apply size and position + * defaults equal to those applied in block supports in lib/background.php. */ if ( 'background-image' === $css_property && ! empty( $value ) ) { $background_styles = gutenberg_style_engine_get_styles( From f21a3e992dea715ceac1251b59576fd34f1a79b2 Mon Sep 17 00:00:00 2001 From: ramon Date: Wed, 14 Aug 2024 14:18:12 +1000 Subject: [PATCH 11/11] Reduce the scope of the merge PHP unit test - let's just test background styles as general styles are tested above in test_merge_incoming_data --- phpunit/class-wp-theme-json-test.php | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/phpunit/class-wp-theme-json-test.php b/phpunit/class-wp-theme-json-test.php index f8fa9511b1b82..10bb47b87fba8 100644 --- a/phpunit/class-wp-theme-json-test.php +++ b/phpunit/class-wp-theme-json-test.php @@ -2294,7 +2294,7 @@ public function test_merge_incoming_data_presets_use_default_names() { $this->assertSameSetsWithIndex( $expected, $actual ); } - public function test_merge_incoming_styles() { + public function test_merge_incoming_background_styles() { $theme_json = new WP_Theme_JSON_Gutenberg( array( 'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA, @@ -2305,9 +2305,6 @@ public function test_merge_incoming_styles() { ), 'backgroundSize' => 'cover', ), - 'typography' => array( - 'fontSize' => '10px', - ), 'blocks' => array( 'core/group' => array( 'background' => array( @@ -2327,11 +2324,6 @@ public function test_merge_incoming_styles() { ), ), ), - 'core/verse' => array( - 'background' => array( - 'backgroundImage' => "url('http://example.org/verse.png')", - ), - ), ), ), ) @@ -2343,9 +2335,6 @@ public function test_merge_incoming_styles() { 'background' => array( 'backgroundSize' => 'contain', ), - 'typography' => array( - 'fontSize' => '12px', - ), 'blocks' => array( 'core/group' => array( 'background' => array( @@ -2354,6 +2343,11 @@ public function test_merge_incoming_styles() { ), ), ), + 'core/quote' => array( + 'background' => array( + 'backgroundAttachment' => 'fixed', + ), + ), 'core/verse' => array( 'background' => array( 'backgroundImage' => array( @@ -2373,9 +2367,6 @@ public function test_merge_incoming_styles() { ), 'backgroundSize' => 'contain', ), - 'typography' => array( - 'fontSize' => '12px', - ), 'blocks' => array( 'core/group' => array( 'background' => array( @@ -2390,9 +2381,7 @@ public function test_merge_incoming_styles() { 'backgroundImage' => array( 'url' => 'http://example.org/quote.png', ), - 'backgroundAttachment' => array( - 'ref' => 'styles.blocks.core/group.background.backgroundAttachment', - ), + 'backgroundAttachment' => 'fixed', ), ), 'core/verse' => array(