This commit moves the logic that sets the default background CSS prop…

…erty values from the style engine to a useBlockProps hook. (WordPress#60008) The style engine can now generate "pure" styles, with return values reflecting input values, and not be opinionated or return feature-specific mutations. Co-authored-by: ramonjd <[email protected]> Co-authored-by: andrewserong <[email protected]>
carstingaxion · Mar 27, 2024 · f4f0ea2 · f4f0ea2
1 parent 35d9515
commit f4f0ea2
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 139 deletions.
diff --git a/packages/block-editor/src/hooks/background.js b/packages/block-editor/src/hooks/background.js
@@ -103,6 +103,46 @@ export function hasBackgroundSupport( blockName, feature = 'any' ) {
 	return !! support?.[ feature ];
 }
 
+function useBlockProps( { name, style } ) {
+	if (
+		! hasBackgroundSupport( name ) ||
+		! style?.background?.backgroundImage
+	) {
+		return;
+	}
+
+	const backgroundImage = style?.background?.backgroundImage;
+	let props;
+
+	// Set block background defaults.
+	if ( backgroundImage?.source === 'file' && !! backgroundImage?.url ) {
+		if ( ! style?.background?.backgroundSize ) {
+			props = {
+				style: {
+					backgroundSize: 'cover',
+				},
+			};
+		}
+
+		if (
+			'contain' === style?.background?.backgroundSize &&
+			! style?.background?.backgroundPosition
+		) {
+			props = {
+				style: {
+					backgroundPosition: 'center',
+				},
+			};
+		}
+	}
+
+	if ( ! props ) {
+		return;
+	}
+
+	return props;
+}
+
 /**
  * Resets the background image block support attributes. This can be used when disabling
  * the background image controls for a block via a `ToolsPanel`.
@@ -425,11 +465,10 @@ function BackgroundSizePanelItem( {
 	// If the current value is `cover` and the repeat value is `undefined`, then
 	// the toggle should be unchecked as the default state. Otherwise, the toggle
 	// should reflect the current repeat value.
-	const repeatCheckedValue =
+	const repeatCheckedValue = ! (
 		repeatValue === 'no-repeat' ||
 		( currentValueForToggle === 'cover' && repeatValue === undefined )
-			? false
-			: true;
+	);
 
 	const hasValue = hasBackgroundSizeValue( style );
 
@@ -602,3 +641,9 @@ export function BackgroundImagePanel( props ) {
 		</InspectorControls>
 	);
 }
+
+export default {
+	useBlockProps,
+	attributeKeys: [ 'style' ],
+	hasSupport: hasBackgroundSupport,
+};
diff --git a/packages/block-editor/src/hooks/index.js b/packages/block-editor/src/hooks/index.js
@@ -8,6 +8,7 @@ import {
 } from './utils';
 import './compat';
 import align from './align';
+import background from './background';
 import './lock';
 import anchor from './anchor';
 import ariaLabel from './aria-label';
@@ -47,6 +48,7 @@ createBlockEditFilter(
 );
 createBlockListBlockFilter( [
 	align,
+	background,
 	style,
 	color,
 	dimensions,

diff --git a/packages/style-engine/src/styles/background/index.ts b/packages/style-engine/src/styles/background/index.ts
@@ -1,58 +1,44 @@
 /**
  * Internal dependencies
  */
-import type { GeneratedCSSRule, Style, StyleOptions } from '../../types';
+import type { Style, StyleOptions } from '../../types';
 import { generateRule, safeDecodeURI } from '../utils';
 
 const backgroundImage = {
 	name: 'backgroundImage',
 	generate: ( style: Style, options: StyleOptions ) => {
 		const _backgroundImage = style?.background?.backgroundImage;
-		const _backgroundSize = style?.background?.backgroundSize;
-
-		const styleRules: GeneratedCSSRule[] = [];
-
-		if ( ! _backgroundImage ) {
-			return styleRules;
-		}
-
-		/*
-		 * If the background image is a string, it could already contain a url() function,
-		 * or have a linear-gradient value.
-		 */
-		if ( typeof _backgroundImage === 'string' ) {
-			styleRules.push( {
-				selector: options.selector,
-				key: 'backgroundImage',
-				value: _backgroundImage,
-			} );
-		}
-
 		if (
 			typeof _backgroundImage === 'object' &&
 			_backgroundImage?.source === 'file' &&
 			_backgroundImage?.url
 		) {
-			styleRules.push( {
-				selector: options.selector,
-				key: 'backgroundImage',
-				// Passed `url` may already be encoded. To prevent double encoding, decodeURI is executed to revert to the original string.
-				value: `url( '${ encodeURI(
-					safeDecodeURI( _backgroundImage.url )
-				) }' )`,
-			} );
+			return [
+				{
+					selector: options.selector,
+					key: 'backgroundImage',
+					// Passed `url` may already be encoded. To prevent double encoding, decodeURI is executed to revert to the original string.
+					value: `url( '${ encodeURI(
+						safeDecodeURI( _backgroundImage.url )
+					) }' )`,
+				},
+			];
 		}
 
-		// If no background size is set, but an image is, default to cover.
-		if ( _backgroundSize === undefined ) {
-			styleRules.push( {
-				selector: options.selector,
-				key: 'backgroundSize',
-				value: 'cover',
-			} );
+		/*
+		 * 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 styleRules;
+		return [];
 	},
 };
 
@@ -83,37 +69,12 @@ const backgroundRepeat = {
 const backgroundSize = {
 	name: 'backgroundSize',
 	generate: ( style: Style, options: StyleOptions ) => {
-		const _backgroundSize = style?.background?.backgroundSize;
-		const _backgroundPosition = style?.background?.backgroundPosition;
-
-		const styleRules: GeneratedCSSRule[] = [];
-
-		if ( _backgroundSize === undefined ) {
-			return styleRules;
-		}
-
-		styleRules.push(
-			...generateRule(
-				style,
-				options,
-				[ 'background', 'backgroundSize' ],
-				'backgroundSize'
-			)
+		return generateRule(
+			style,
+			options,
+			[ 'background', 'backgroundSize' ],
+			'backgroundSize'
 		);
-
-		// If background size is set to contain, but no position is set, default to center.
-		if (
-			_backgroundSize === 'contain' &&
-			_backgroundPosition === undefined
-		) {
-			styleRules.push( {
-				selector: options.selector,
-				key: 'backgroundPosition',
-				value: 'center',
-			} );
-		}
-
-		return styleRules;
 	},
 };
 

diff --git a/packages/style-engine/src/test/index.js b/packages/style-engine/src/test/index.js
@@ -430,35 +430,6 @@ describe( 'getCSSRules', () => {
 		] );
 	} );
 
-	it( 'should output fallback cover background size when no size is provided', () => {
-		expect(
-			getCSSRules(
-				{
-					background: {
-						backgroundImage: {
-							source: 'file',
-							url: 'https://example.com/image.jpg',
-						},
-					},
-				},
-				{
-					selector: '.some-selector',
-				}
-			)
-		).toEqual( [
-			{
-				selector: '.some-selector',
-				key: 'backgroundImage',
-				value: "url( 'https://example.com/image.jpg' )",
-			},
-			{
-				selector: '.some-selector',
-				key: 'backgroundSize',
-				value: 'cover',
-			},
-		] );
-	} );
-
 	it( 'should output background image value when that value is a string', () => {
 		expect(
 			getCSSRules(
@@ -478,46 +449,6 @@ describe( 'getCSSRules', () => {
 				key: 'backgroundImage',
 				value: "linear-gradient(to bottom,rgb(255 255 0 / 50%),rgb(0 0 255 / 50%), url('https://example.com/image.jpg')",
 			},
-			{
-				selector: '.some-selector',
-				key: 'backgroundSize',
-				value: 'cover',
-			},
-		] );
-	} );
-
-	it( 'should output fallback center position for contain background size', () => {
-		expect(
-			getCSSRules(
-				{
-					background: {
-						backgroundImage: {
-							source: 'file',
-							url: 'https://example.com/image.jpg',
-						},
-						backgroundSize: 'contain',
-					},
-				},
-				{
-					selector: '.some-selector',
-				}
-			)
-		).toEqual( [
-			{
-				selector: '.some-selector',
-				key: 'backgroundImage',
-				value: "url( 'https://example.com/image.jpg' )",
-			},
-			{
-				selector: '.some-selector',
-				key: 'backgroundSize',
-				value: 'contain',
-			},
-			{
-				selector: '.some-selector',
-				key: 'backgroundPosition',
-				value: 'center',
-			},
 		] );
 	} );
 } );