From 7de1819c1e0f33b252ae046e470f53614901cc83 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 15 Feb 2022 13:13:56 +1100 Subject: [PATCH] This is the first commit that switches the blockGap over to use to an object. We're using the `top` and `left` properties of the BoxControl next value to represent row and column values. Reverting some block.json changes. Ensuring that we cater for blockGap string values. Reverting some block.json changes. Ensuring that we cater for blockGap string values. Adding tests. --- lib/block-supports/layout.php | 6 ++ packages/block-editor/src/hooks/gap.js | 96 +++++++++++-------- packages/block-editor/src/hooks/test/gap.js | 41 ++++++++ packages/block-editor/src/layouts/flex.js | 6 +- packages/block-editor/src/layouts/flow.js | 6 +- packages/block-library/src/columns/block.json | 2 +- .../block-library/src/navigation/block.json | 2 +- .../block-library/src/social-links/block.json | 2 +- .../global-styles/dimensions-panel.js | 41 +++++--- 9 files changed, 143 insertions(+), 59 deletions(-) create mode 100644 packages/block-editor/src/hooks/test/gap.js diff --git a/lib/block-supports/layout.php b/lib/block-supports/layout.php index f6e96b95e06b1..bb7d49101ae98 100644 --- a/lib/block-supports/layout.php +++ b/lib/block-supports/layout.php @@ -66,6 +66,9 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support $style .= "$selector .alignleft { float: left; margin-right: 2em; margin-left: 0; }"; $style .= "$selector .alignright { float: right; margin-left: 2em; margin-right: 0; }"; if ( $has_block_gap_support ) { + if ( is_array( $gap_value ) ) { + $gap_value = $gap_value['left'] === $gap_value['top'] ? $gap_value['top'] : $gap_value['top'] . ' ' . $gap_value['left']; + } $gap_style = $gap_value ? $gap_value : 'var( --wp--style--block-gap )'; $style .= "$selector > * { margin-top: 0; margin-bottom: 0; }"; $style .= "$selector > * + * { margin-top: $gap_style; margin-bottom: 0; }"; @@ -91,6 +94,9 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support $style = "$selector {"; $style .= 'display: flex;'; if ( $has_block_gap_support ) { + if ( is_array( $gap_value ) ) { + $gap_value = $gap_value['left'] === $gap_value['top'] ? $gap_value['top'] : $gap_value['top'] . ' ' . $gap_value['left']; + } $gap_style = $gap_value ? $gap_value : 'var( --wp--style--block-gap, 0.5em )'; $style .= "gap: $gap_style;"; } else { diff --git a/packages/block-editor/src/hooks/gap.js b/packages/block-editor/src/hooks/gap.js index b897a18520508..869f4f62d04a6 100644 --- a/packages/block-editor/src/hooks/gap.js +++ b/packages/block-editor/src/hooks/gap.js @@ -22,7 +22,7 @@ import { cleanEmptyObject } from './utils'; * Determines if there is gap support. * * @param {string|Object} blockType Block name or Block Type object. - * @return {boolean} Whether there is support. + * @return {boolean} Whether there is support. */ export function hasGapSupport( blockType ) { const support = getBlockSupport( blockType, SPACING_SUPPORT_KEY ); @@ -39,6 +39,45 @@ export function hasGapValue( props ) { return props.attributes.style?.spacing?.blockGap !== undefined; } +/** + * Returns a BoxControl object value from a given blockGap style. + * 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. + */ +export function getGapValueFromStyle( rawBlockGapValue ) { + if ( ! rawBlockGapValue ) { + return rawBlockGapValue; + } + + const isValueString = typeof rawBlockGapValue === 'string'; + return cleanEmptyObject( { + top: isValueString ? rawBlockGapValue : rawBlockGapValue?.top, + left: isValueString ? rawBlockGapValue : rawBlockGapValue?.left, + } ); +} + +/** + * Returns a CSS value for the `gap` property from a given blockGap style. + * + * @param {string? | Object?} blockGapValue A style object. + * @param {string?} defaultValue A default gap value. + * @return {string?} The concatenated gap value (row and column). + */ +export function getGapCSSValue( blockGapValue, defaultValue = '0' ) { + const blockGapBoxControlValue = getGapValueFromStyle( blockGapValue ); + if ( ! blockGapBoxControlValue ) { + return blockGapBoxControlValue; + } + + const row = blockGapBoxControlValue?.top || defaultValue; + const column = blockGapBoxControlValue?.left || defaultValue; + + return row === column ? row : `${ row } ${ column }`; +} + /** * Resets the gap block support attribute. This can be used when disabling * the gap support controls for a block via a progressive discovery panel. @@ -96,11 +135,7 @@ export function GapEdit( props ) { 'vw', ], } ); - const sides = useCustomSides( blockName, 'blockGap' ); - const splitOnAxis = - sides && sides.some( ( side ) => AXIAL_SIDES.includes( side ) ); - const ref = useBlockRef( clientId ); if ( useIsGapDisabled( props ) ) { @@ -108,17 +143,13 @@ export function GapEdit( props ) { } const onChange = ( next ) => { - let newValue = next; - if ( typeof next === 'object' ) { - const row = next?.top || 0; - const column = next?.left || 0; - newValue = row === column ? row : `${ row } ${ column }`; - } const newStyle = { ...style, spacing: { ...style?.spacing, - blockGap: newValue, + blockGap: { + ...getGapValueFromStyle( next ), + }, }, }; @@ -140,33 +171,16 @@ export function GapEdit( props ) { } }; - const blockGapValue = style?.spacing?.blockGap; - const boxValuesArray = blockGapValue - ? blockGapValue.split( ' ' ) - : [ undefined ]; - const boxValues = { - left: undefined, - top: undefined, - bottom: undefined, - right: undefined, - }; - - if ( boxValuesArray.length === 1 ) { - boxValues.left = boxValuesArray[ 0 ]; - boxValues.right = boxValuesArray[ 0 ]; - boxValues.top = boxValuesArray[ 0 ]; - boxValues.bottom = boxValuesArray[ 0 ]; - } - - if ( boxValuesArray.length === 2 ) { - boxValues.left = boxValuesArray[ 1 ]; - boxValues.right = boxValuesArray[ 1 ]; - boxValues.top = boxValuesArray[ 0 ]; - boxValues.bottom = boxValuesArray[ 0 ]; - } - - // The default combined value we'll take from row. - const defaultValue = boxValues.top; + const splitOnAxis = + sides && sides.some( ( side ) => AXIAL_SIDES.includes( side ) ); + const gapValue = getGapValueFromStyle( style?.spacing?.blockGap ); + const boxControlGapValue = splitOnAxis + ? { + ...gapValue, + right: gapValue?.left, + bottom: gapValue?.top, + } + : gapValue?.top; return Platform.select( { web: ( @@ -178,7 +192,7 @@ export function GapEdit( props ) { onChange={ onChange } units={ units } sides={ sides } - values={ boxValues } + values={ boxControlGapValue } allowReset={ false } splitOnAxis={ splitOnAxis } /> @@ -190,7 +204,7 @@ export function GapEdit( props ) { onChange={ onChange } units={ units } // Default to `row` for combined values. - value={ defaultValue } + value={ boxControlGapValue } /> ) } diff --git a/packages/block-editor/src/hooks/test/gap.js b/packages/block-editor/src/hooks/test/gap.js new file mode 100644 index 0000000000000..98276d0a74bab --- /dev/null +++ b/packages/block-editor/src/hooks/test/gap.js @@ -0,0 +1,41 @@ +/** + * Internal dependencies + */ +import { getGapCSSValue } from '../gap'; + +describe( 'gap', () => { + describe( 'getGapCSSValue()', () => { + it( 'should return argument if argument is falsey', () => { + expect( getGapCSSValue( undefined ) ).toBeUndefined(); + } ); + + it( 'should return single value for gap if argument is valid string', () => { + expect( getGapCSSValue( '88rem' ) ).toEqual( '88rem' ); + } ); + + it( 'should return single value for gap if row and column are the same', () => { + const blockGapValue = { + top: '88rem', + left: '88rem', + }; + expect( getGapCSSValue( blockGapValue ) ).toEqual( '88rem' ); + } ); + + it( 'should return shorthand value for gap if row and column are different', () => { + const blockGapValue = { + top: '88px', + left: '88rem', + }; + expect( getGapCSSValue( blockGapValue ) ).toEqual( '88px 88rem' ); + } ); + + it( 'should return default value if a top or left is missing', () => { + const blockGapValue = { + top: '88px', + }; + expect( getGapCSSValue( blockGapValue, '1px' ) ).toEqual( + '88px 1px' + ); + } ); + } ); +} ); diff --git a/packages/block-editor/src/layouts/flex.js b/packages/block-editor/src/layouts/flex.js index 092da1e036a10..986789f8726c3 100644 --- a/packages/block-editor/src/layouts/flex.js +++ b/packages/block-editor/src/layouts/flex.js @@ -16,6 +16,7 @@ import { Button, ToggleControl, Flex, FlexItem } from '@wordpress/components'; * Internal dependencies */ import { appendSelectors } from './utils'; +import { getGapCSSValue } from '../hooks/gap'; import useSetting from '../components/use-setting'; import { BlockControls, JustifyContentControl } from '../components'; @@ -89,7 +90,8 @@ export default { const blockGapSupport = useSetting( 'spacing.blockGap' ); const hasBlockGapStylesSupport = blockGapSupport !== null; const blockGapValue = - style?.spacing?.blockGap ?? 'var( --wp--style--block-gap, 0.5em )'; + getGapCSSValue( style?.spacing?.blockGap, '0.5em' ) ?? + 'var( --wp--style--block-gap, 0.5em )'; const justifyContent = justifyContentMap[ layout.justifyContent ] || justifyContentMap.left; @@ -112,8 +114,8 @@ export default {