Background image: ensure consistency with defaults and fix reset/remove functionality

backport-changelog/6.7/7137.md

@@ -1,3 +1,4 @@
 https://github.com/WordPress/wordpress-develop/pull/7137
 
 * https://github.com/WordPress/gutenberg/pull/64192
+* https://github.com/WordPress/gutenberg/pull/64328

lib/block-supports/background.php

@@ -62,7 +62,7 @@ function gutenberg_render_background_support( $block_content, $block ) {
 		$background_styles['backgroundSize'] = $background_styles['backgroundSize'] ?? 'cover';
 		// If the background size is set to `contain` and no position is set, set the position to `center`.
 		if ( 'contain' === $background_styles['backgroundSize'] && ! $background_styles['backgroundPosition'] ) {
-			$background_styles['backgroundPosition'] = 'center';
+			$background_styles['backgroundPosition'] = '50% 50%';
 		}
 	}
 

lib/class-wp-theme-json-gutenberg.php

@@ -2391,7 +2391,7 @@ protected static function compute_style_properties( $styles, $settings = array()
 					$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'] = 'center';
+						$styles['background']['backgroundPosition'] = '50% 50%';
 					}
 				}
 				$background_styles = gutenberg_style_engine_get_styles( array( 'background' => $styles['background'] ) );

packages/block-editor/src/components/global-styles/background-panel.js

@@ -268,8 +268,10 @@ function BackgroundImageControls( {
 	style,
 	inheritedValue,
 	onRemoveImage = noop,
+	onResetImage = noop,
 	displayInPanel,
 	themeFileURIs,
+	defaultValues,
 } ) {
 	const mediaUpload = useSelect(
 		( select ) => select( blockEditorStore ).getSettings().mediaUpload,
@@ -319,12 +321,8 @@ function BackgroundImageControls( {
 		}
 
 		const sizeValue =
-			style?.background?.backgroundSize ||
-			inheritedValue?.background?.backgroundSize;
-		const positionValue =
-			style?.background?.backgroundPosition ||
-			inheritedValue?.background?.backgroundPosition;
-
+			style?.background?.backgroundSize || defaultValues?.backgroundSize;
+		const positionValue = style?.background?.backgroundPosition;
 		onChange(
 			setImmutably( style, [ 'background' ], {
 				...style?.background,
@@ -335,6 +333,12 @@ function BackgroundImageControls( {
 					title: media.title || undefined,
 				},
 				backgroundPosition:
+					/*
+					 * A background image uploaded and set in the editor receives a default background position of '50% 0',
+					 * when the background image size is the equivalent of "Tile".
+					 * This is to increase the chance that the image's focus point is visible.
+					 * This is in-editor only to assist with the user experience.
+					 */
 					! positionValue && ( 'auto' === sizeValue || ! sizeValue )
 						? '50% 0'
 						: positionValue,
@@ -372,7 +376,9 @@ function BackgroundImageControls( {
 
 	const onRemove = () =>
 		onChange(
-			setImmutably( style, [ 'background', 'backgroundImage' ], 'none' )
+			setImmutably( style, [ 'background' ], {
+				backgroundImage: 'none',
+			} )
 		);
 	const canRemove = ! hasValue && hasBackgroundImageValue( inheritedValue );
 	const imgLabel =
@@ -413,6 +419,7 @@ function BackgroundImageControls( {
 						onClick={ () => {
 							closeAndFocus();
 							onRemove();
+							onRemoveImage();
 						} }
 					>
 						{ __( 'Remove' ) }
@@ -422,7 +429,7 @@ function BackgroundImageControls( {
 					<MenuItem
 						onClick={ () => {
 							closeAndFocus();
-							onRemoveImage();
+							onResetImage();
 						} }
 					>
 						{ __( 'Reset ' ) }
@@ -453,9 +460,7 @@ function BackgroundSizeControls( {
 	const imageValue =
 		style?.background?.backgroundImage?.url ||
 		inheritedValue?.background?.backgroundImage?.url;
-	const isUploadedImage =
-		style?.background?.backgroundImage?.id ||
-		inheritedValue?.background?.backgroundImage?.id;
+	const isUploadedImage = style?.background?.backgroundImage?.id;
 	const positionValue =
 		style?.background?.backgroundPosition ||
 		inheritedValue?.background?.backgroundPosition;
@@ -469,11 +474,19 @@ function BackgroundSizeControls( {
 	 * Block-level controls may have different defaults to root-level controls.
 	 * A falsy value is treated by default as `auto` (Tile).
 	 */
-	const currentValueForToggle =
+	let currentValueForToggle =
 		! sizeValue && isUploadedImage
 			? defaultValues?.backgroundSize
 			: sizeValue || 'auto';
-
+	/*
+	 * The incoming value could be a value + unit, e.g. '20px'.
+	 * In this case set the value to 'tile'.
+	 */
+	currentValueForToggle = ! [ 'cover', 'contain', 'auto' ].includes(
+		currentValueForToggle
+	)
+		? 'auto'
+		: currentValueForToggle;
 	/*
 	 * 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
@@ -510,6 +523,7 @@ function BackgroundSizeControls( {
 			 * receives a default background position of '50% 0',
 			 * when the toggle switches to "Tile". This is to increase the chance that
 			 * the image's focus point is visible.
+			 * This is in-editor only to assist with the user experience.
 			 */
 			if ( !! style?.background?.backgroundImage?.id ) {
 				nextPosition = '50% 0';
@@ -562,24 +576,27 @@ function BackgroundSizeControls( {
 			)
 		);
 
+	// Set a default background position for non-site-wide, uploaded images with a size of 'contain'.
+	const backgroundPositionValue =
+		! positionValue && isUploadedImage && 'contain' === sizeValue
+			? defaultValues?.backgroundPosition
+			: positionValue;
+
 	return (
-		<VStack spacing={ 4 } className="single-column">
+		<VStack spacing={ 3 } className="single-column">
 			<FocalPointPicker
 				__next40pxDefaultSize
 				__nextHasNoMarginBottom
 				label={ __( 'Focal point' ) }
 				url={ getResolvedThemeFilePath( imageValue, themeFileURIs ) }
-				value={ backgroundPositionToCoords( positionValue ) }
+				value={ backgroundPositionToCoords( backgroundPositionValue ) }
 				onChange={ updateBackgroundPosition }
 			/>
 			<ToggleControl
 				__nextHasNoMarginBottom
 				label={ __( 'Fixed background' ) }
 				checked={ attachmentValue === 'fixed' }
 				onChange={ toggleScrollWithPage }
-				help={ __(
-					'Whether your image should scroll with the page or stay fixed in place.'
-				) }
 			/>
 			<ToggleGroupControl
 				__nextHasNoMarginBottom
@@ -660,7 +677,7 @@ function BackgroundToolsPanel( {
 	return (
 		<VStack
 			as={ ToolsPanel }
-			spacing={ 4 }
+			spacing={ 2 }
 			label={ headerLabel }
 			resetAll={ resetAll }
 			panelId={ panelId }
@@ -700,8 +717,13 @@ export default function BackgroundPanel( {
 		hasBackgroundImageValue( value ) ||
 		hasBackgroundImageValue( inheritedValue );
 
+	const imageValue =
+		value?.background?.backgroundImage ||
+		inheritedValue?.background?.backgroundImage;
+
 	const shouldShowBackgroundImageControls =
 		hasImageValue &&
+		'none' !== imageValue &&
 		( settings?.background?.backgroundSize ||
 			settings?.background?.backgroundPosition ||
 			settings?.background?.backgroundRepeat );
@@ -725,7 +747,7 @@ export default function BackgroundPanel( {
 				) }
 			>
 				<ToolsPanelItem
-					hasValue={ () => hasImageValue }
+					hasValue={ () => !! value?.background }
 					label={ __( 'Image' ) }
 					onDeselect={ resetBackground }
 					isShownByDefault={ defaultControls.backgroundImage }
@@ -749,10 +771,14 @@ export default function BackgroundPanel( {
 									inheritedValue={ inheritedValue }
 									themeFileURIs={ themeFileURIs }
 									displayInPanel
-									onRemoveImage={ () => {
+									onResetImage={ () => {
 										setIsDropDownOpen( false );
 										resetBackground();
 									} }
+									onRemoveImage={ () =>
+										setIsDropDownOpen( false )
+									}
+									defaultValues={ defaultValues }
 								/>
 								<BackgroundSizeControls
 									onChange={ onChange }
@@ -770,6 +796,12 @@ export default function BackgroundPanel( {
 							style={ value }
 							inheritedValue={ inheritedValue }
 							themeFileURIs={ themeFileURIs }
+							defaultValues={ defaultValues }
+							onResetImage={ () => {
+								setIsDropDownOpen( false );
+								resetBackground();
+							} }
+							onRemoveImage={ () => setIsDropDownOpen( false ) }
 						/>
 					) }
 				</ToolsPanelItem>

packages/block-editor/src/components/global-styles/test/use-global-styles-output.js

@@ -1061,7 +1061,7 @@ describe( 'global styles renderer', () => {
 				)
 			).toEqual( [
 				"background-image: url( 'https://wordpress.org/assets/image.jpg' )",
-				'background-position: center',
+				'background-position: 50% 50%',
 				'background-size: contain',
 			] );
 		} );

packages/block-editor/src/hooks/background.js

@@ -23,9 +23,10 @@ import {
 
 export const BACKGROUND_SUPPORT_KEY = 'background';
 
-// Initial control values where no block style is set.
-const BACKGROUND_DEFAULT_VALUES = {
+// Initial control values.
+export const BACKGROUND_BLOCK_DEFAULT_VALUES = {
 	backgroundSize: 'cover',
+	backgroundPosition: '50% 50%', // used only when backgroundSize is 'contain'.
 };
 
 /**
@@ -55,31 +56,28 @@ export function hasBackgroundSupport( blockName, feature = 'any' ) {
 }
 
 export function setBackgroundStyleDefaults( backgroundStyle ) {
-	if ( ! backgroundStyle ) {
+	if ( ! backgroundStyle || ! backgroundStyle?.backgroundImage?.url ) {
 		return;
 	}
 
-	const backgroundImage = backgroundStyle?.backgroundImage;
 	let backgroundStylesWithDefaults;
 
 	// Set block background defaults.
-	if ( !! backgroundImage?.url ) {
-		if ( ! backgroundStyle?.backgroundSize ) {
-			backgroundStylesWithDefaults = {
-				backgroundSize: 'cover',
-			};
-		}
-
-		if (
-			'contain' === backgroundStyle?.backgroundSize &&
-			! backgroundStyle?.backgroundPosition
-		) {
-			backgroundStylesWithDefaults = {
-				backgroundPosition: 'center',
-			};
-		}
+	if ( ! backgroundStyle?.backgroundSize ) {
+		backgroundStylesWithDefaults = {
+			backgroundSize: BACKGROUND_BLOCK_DEFAULT_VALUES.backgroundSize,
+		};
 	}
 
+	if (
+		'contain' === backgroundStyle?.backgroundSize &&
+		! backgroundStyle?.backgroundPosition
+	) {
+		backgroundStylesWithDefaults = {
+			backgroundPosition:
+				BACKGROUND_BLOCK_DEFAULT_VALUES.backgroundPosition,
+		};
+	}
 	return backgroundStylesWithDefaults;
 }
 
@@ -147,6 +145,7 @@ export function BackgroundImagePanel( {
 				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,
 				 *   See: packages/block-editor/src/hooks/style.js
 				 * @TODO 2. Add support for block style variations,
@@ -187,7 +186,7 @@ export function BackgroundImagePanel( {
 			inheritedValue={ inheritedValue }
 			as={ BackgroundInspectorControl }
 			panelId={ clientId }
-			defaultValues={ BACKGROUND_DEFAULT_VALUES }
+			defaultValues={ BACKGROUND_BLOCK_DEFAULT_VALUES }
 			settings={ updatedSettings }
 			onChange={ onChange }
 			value={ style }

packages/block-editor/src/hooks/test/background.js

@@ -0,0 +1,60 @@
+/**
+ * Internal dependencies
+ */
+import {
+	setBackgroundStyleDefaults,
+	BACKGROUND_BLOCK_DEFAULT_VALUES,
+} from '../background';
+
+describe( 'background', () => {
+	describe( 'setBackgroundStyleDefaults', () => {
+		const backgroundStyles = {
+			backgroundImage: { id: 123, url: 'image.png' },
+		};
+		const backgroundStylesContain = {
+			backgroundImage: { id: 123, url: 'image.png' },
+			backgroundSize: 'contain',
+		};
+		const backgroundStylesNoURL = { backgroundImage: { id: 123 } };
+		it.each( [
+			[
+				'return background size default',
+				backgroundStyles,
+				{
+					backgroundSize:
+						BACKGROUND_BLOCK_DEFAULT_VALUES.backgroundSize,
+				},
+			],
+			[ 'return early if no styles are passed', undefined, undefined ],
+			[
+				'return early if images has no id',
+				backgroundStylesNoURL,
+				undefined,
+			],
+			[
+				'return early if images has no URL',
+				backgroundStylesNoURL,
+				undefined,
+			],
+			[
+				'return background position default',
+				backgroundStylesContain,
+				{
+					backgroundPosition:
+						BACKGROUND_BLOCK_DEFAULT_VALUES.backgroundPosition,
+				},
+			],
+			[
+				'not apply background position value if one already exists in styles',
+				{
+					...backgroundStylesContain,
+					backgroundPosition: 'center',
+				},
+				undefined,
+			],
+		] )( 'should %s', ( message, styles, expected ) => {
+			const result = setBackgroundStyleDefaults( styles );
+			expect( result ).toEqual( expected );
+		} );
+	} );
+} );

packages/edit-site/src/components/global-styles/screen-block.js

@@ -25,9 +25,10 @@ import {
 	VariationsPanel,
 } from './variations/variations-panel';
 
-// Initial control values where no block style is set.
+// Initial control values.
 const BACKGROUND_BLOCK_DEFAULT_VALUES = {
 	backgroundSize: 'cover',
+	backgroundPosition: '50% 50%', // used only when backgroundSize is 'contain'.
 };
 
 function applyFallbackStyle( border ) {