Block gap: Check for splitOnAxis in the onChange callback (#39788)

* Check for splitOnAxis in the onChange callback and return a box value object only when the boxvalue control is activated. Rename vars for clarity Add some tests for getGapBoxControlValueFromStyle because we call it directly in flow.js (even though the tests for getGapCSSValue cover it indirectly) * Am I pretty enough for you?
WordPress · Mar 28, 2022 · 77e9bf4 · 77e9bf4
1 parent 6a4fd64
commit 77e9bf4
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 20 deletions.
diff --git a/packages/block-editor/src/hooks/gap.js b/packages/block-editor/src/hooks/gap.js
@@ -40,34 +40,36 @@ export function hasGapValue( props ) {
 }
 
 /**
- * Returns a BoxControl object value from a given blockGap style.
+ * Returns a BoxControl object value from a given blockGap style value.
  * The string check is for backwards compatibility before Gutenberg supported
  * split gap values (row and column) and the value was a string n + unit.
  *
- * @param {string? | Object?} rawBlockGapValue A style object.
- * @return {Object?}                           A value to pass to the BoxControl component.
+ * @param {string? | Object?} blockGapValue A block gap string or axial object value, e.g., '10px' or { top: '10px', left: '10px'}.
+ * @return {Object|null}                    A value to pass to the BoxControl component.
  */
-export function getGapValueFromStyle( rawBlockGapValue ) {
-	if ( ! rawBlockGapValue ) {
-		return rawBlockGapValue;
+export function getGapBoxControlValueFromStyle( blockGapValue ) {
+	if ( ! blockGapValue ) {
+		return null;
 	}
 
-	const isValueString = typeof rawBlockGapValue === 'string';
+	const isValueString = typeof blockGapValue === 'string';
 	return {
-		top: isValueString ? rawBlockGapValue : rawBlockGapValue?.top,
-		left: isValueString ? rawBlockGapValue : rawBlockGapValue?.left,
+		top: isValueString ? blockGapValue : blockGapValue?.top,
+		left: isValueString ? blockGapValue : blockGapValue?.left,
 	};
 }
 
 /**
  * Returns a CSS value for the `gap` property from a given blockGap style.
  *
- * @param {string? | Object?} blockGapValue A style object.
+ * @param {string? | Object?} blockGapValue A block gap string or axial object value, e.g., '10px' or { top: '10px', left: '10px'}.
  * @param {string?}           defaultValue  A default gap value.
  * @return {string|null}                    The concatenated gap value (row and column).
  */
 export function getGapCSSValue( blockGapValue, defaultValue = '0' ) {
-	const blockGapBoxControlValue = getGapValueFromStyle( blockGapValue );
+	const blockGapBoxControlValue = getGapBoxControlValueFromStyle(
+		blockGapValue
+	);
 	if ( ! blockGapBoxControlValue ) {
 		return null;
 	}
@@ -142,14 +144,22 @@ export function GapEdit( props ) {
 		return null;
 	}
 
+	const splitOnAxis =
+		sides && sides.some( ( side ) => AXIAL_SIDES.includes( side ) );
+
 	const onChange = ( next ) => {
+		let blockGap = next;
+
+		// If splitOnAxis activated we need to return a BoxControl object to the BoxControl component.
+		if ( !! next && splitOnAxis ) {
+			blockGap = { ...getGapBoxControlValueFromStyle( next ) };
+		}
+
 		const newStyle = {
 			...style,
 			spacing: {
 				...style?.spacing,
-				blockGap: {
-					...getGapValueFromStyle( next ),
-				},
+				blockGap,
 			},
 		};
 
@@ -171,9 +181,7 @@ export function GapEdit( props ) {
 		}
 	};
 
-	const splitOnAxis =
-		sides && sides.some( ( side ) => AXIAL_SIDES.includes( side ) );
-	const gapValue = getGapValueFromStyle( style?.spacing?.blockGap );
+	const gapValue = getGapBoxControlValueFromStyle( style?.spacing?.blockGap );
 
 	// The BoxControl component expects a full complement of side values.
 	// Gap row and column values translate to top/bottom and left/right respectively.

diff --git a/packages/block-editor/src/hooks/test/gap.js b/packages/block-editor/src/hooks/test/gap.js
@@ -1,9 +1,33 @@
 /**
  * Internal dependencies
  */
-import { getGapCSSValue } from '../gap';
+import { getGapCSSValue, getGapBoxControlValueFromStyle } from '../gap';
 
 describe( 'gap', () => {
+	describe( 'getGapBoxControlValueFromStyle()', () => {
+		it( 'should return `null` if argument is falsey', () => {
+			expect( getGapBoxControlValueFromStyle( undefined ) ).toBeNull();
+			expect( getGapBoxControlValueFromStyle( '' ) ).toBeNull();
+		} );
+		it( 'should return box control value from string', () => {
+			const expectedValue = {
+				top: '88rem',
+				left: '88rem',
+			};
+			expect( getGapBoxControlValueFromStyle( '88rem' ) ).toEqual(
+				expectedValue
+			);
+		} );
+		it( 'should return box control value from object', () => {
+			const blockGapValue = {
+				top: '222em',
+				left: '22px',
+			};
+			expect( getGapBoxControlValueFromStyle( blockGapValue ) ).toEqual( {
+				...blockGapValue,
+			} );
+		} );
+	} );
 	describe( 'getGapCSSValue()', () => {
 		it( 'should return `null` if argument is falsey', () => {
 			expect( getGapCSSValue( undefined ) ).toBeNull();

diff --git a/packages/block-editor/src/layouts/flow.js b/packages/block-editor/src/layouts/flow.js
@@ -14,7 +14,7 @@ import { Icon, positionCenter, stretchWide } from '@wordpress/icons';
  */
 import useSetting from '../components/use-setting';
 import { appendSelectors } from './utils';
-import { getGapValueFromStyle } from '../hooks/gap';
+import { getGapBoxControlValueFromStyle } from '../hooks/gap';
 
 export default {
 	name: 'default',
@@ -110,7 +110,7 @@ export default {
 		const { contentSize, wideSize } = layout;
 		const blockGapSupport = useSetting( 'spacing.blockGap' );
 		const hasBlockGapStylesSupport = blockGapSupport !== null;
-		const blockGapStyleValue = getGapValueFromStyle(
+		const blockGapStyleValue = getGapBoxControlValueFromStyle(
 			style?.spacing?.blockGap
 		);
 		const blockGapValue =