From 141a7dc096d07e327a971ea9daa2d73c79b70cab Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 28 Mar 2022 10:11:48 +1100 Subject: [PATCH 1/2] 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) --- packages/block-editor/src/hooks/gap.js | 42 ++++++++++++--------- packages/block-editor/src/hooks/test/gap.js | 24 +++++++++++- packages/block-editor/src/layouts/flow.js | 4 +- 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/packages/block-editor/src/hooks/gap.js b/packages/block-editor/src/hooks/gap.js index 84a1bf5ce40024..d360d64331a379 100644 --- 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 index 10a6d02e8ed1d7..fd556bec497aed 100644 --- a/packages/block-editor/src/hooks/test/gap.js +++ b/packages/block-editor/src/hooks/test/gap.js @@ -1,9 +1,31 @@ /** * 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 index 65685334fbffff..f8251c33a3dba8 100644 --- 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 = From 5fe893e3bc7c5fed16bff4e8e71cb554e30de158 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 28 Mar 2022 11:24:24 +1100 Subject: [PATCH 2/2] Am I pretty enough for you? --- packages/block-editor/src/hooks/test/gap.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/hooks/test/gap.js b/packages/block-editor/src/hooks/test/gap.js index fd556bec497aed..908b793f25d881 100644 --- a/packages/block-editor/src/hooks/test/gap.js +++ b/packages/block-editor/src/hooks/test/gap.js @@ -14,7 +14,9 @@ describe( 'gap', () => { top: '88rem', left: '88rem', }; - expect( getGapBoxControlValueFromStyle( '88rem' ) ).toEqual( expectedValue ); + expect( getGapBoxControlValueFromStyle( '88rem' ) ).toEqual( + expectedValue + ); } ); it( 'should return box control value from object', () => { const blockGapValue = {