From 1f7a1beea27612669b94e5f2eb1a4f2cd361abe7 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Thu, 6 Jan 2022 14:21:14 +1100 Subject: [PATCH 01/10] Initial commit: splitting the gap value to allow for axial gap values --- packages/block-editor/src/hooks/gap.js | 66 ++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 10 deletions(-) diff --git a/packages/block-editor/src/hooks/gap.js b/packages/block-editor/src/hooks/gap.js index 13b41c606049c..13de75b980fd2 100644 --- a/packages/block-editor/src/hooks/gap.js +++ b/packages/block-editor/src/hooks/gap.js @@ -6,6 +6,7 @@ import { Platform } from '@wordpress/element'; import { getBlockSupport } from '@wordpress/blocks'; import { __experimentalUseCustomUnits as useCustomUnits, + __experimentalBoxControl as BoxControl, __experimentalUnitControl as UnitControl, } from '@wordpress/components'; @@ -14,7 +15,7 @@ import { */ import { __unstableUseBlockRef as useBlockRef } from '../components/block-list/use-block-props/use-block-refs'; import useSetting from '../components/use-setting'; -import { SPACING_SUPPORT_KEY } from './dimensions'; +import { AXIAL_SIDES, SPACING_SUPPORT_KEY, useCustomSides } from './dimensions'; import { cleanEmptyObject } from './utils'; /** @@ -82,6 +83,7 @@ export function GapEdit( props ) { const { clientId, attributes: { style }, + name: blockName, setAttributes, } = props; @@ -95,6 +97,10 @@ export function GapEdit( props ) { ], } ); + const sides = useCustomSides( blockName, 'blockGap' ); + const splitOnAxis = + sides && sides.some( ( side ) => AXIAL_SIDES.includes( side ) ); + const ref = useBlockRef( clientId ); if ( useIsGapDisabled( props ) ) { @@ -102,11 +108,15 @@ export function GapEdit( props ) { } const onChange = ( next ) => { + const row = next?.top ?? next; + const column = next?.left ?? next; + const newValue = row === column ? row : `${ row } ${ column }`; + const newStyle = { ...style, spacing: { ...style?.spacing, - blockGap: next, + blockGap: newValue, }, }; @@ -128,17 +138,53 @@ export function GapEdit( props ) { } }; + const blockGapValue = style?.spacing?.blockGap; + const boxValuesArray = blockGapValue + ? blockGapValue.split( ' ' ) + : [ undefined ]; + const boxValues = { + left: undefined, + top: undefined, + }; + + if ( boxValuesArray.length === 1 ) { + boxValues.left = boxValuesArray[ 0 ]; + boxValues.top = boxValuesArray[ 0 ]; + } + + if ( boxValuesArray.length === 2 ) { + boxValues.left = boxValuesArray[ 1 ]; + boxValues.top = boxValuesArray[ 0 ]; + } + + // The default combined value we'll take from row. + const defaultValue = boxValues.top; + return Platform.select( { web: ( <> - + { splitOnAxis ? ( + + ) : ( + + ) } ), native: null, From 9ff07348bd8292bfa2ba9f0e3f500daffd919ccd Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 25 Jan 2022 13:22:46 +1100 Subject: [PATCH 02/10] Checking for object. Now passing complete object to box control. --- packages/block-editor/src/hooks/gap.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/hooks/gap.js b/packages/block-editor/src/hooks/gap.js index 13de75b980fd2..ba46ea2990874 100644 --- a/packages/block-editor/src/hooks/gap.js +++ b/packages/block-editor/src/hooks/gap.js @@ -108,10 +108,12 @@ export function GapEdit( props ) { } const onChange = ( next ) => { - const row = next?.top ?? next; - const column = next?.left ?? next; - const newValue = row === column ? row : `${ row } ${ column }`; - + let newValue = next; + if ( typeof next === 'object' ) { + const row = next?.top; + const column = next?.left; + newValue = row === column ? row : `${ row } ${ column }`; + } const newStyle = { ...style, spacing: { @@ -145,16 +147,22 @@ export function GapEdit( props ) { 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. From 72021d80471d5db488710fee0d7d218281c62e78 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Thu, 10 Feb 2022 14:34:45 +1100 Subject: [PATCH 03/10] Providing a default value for split gap values, where one is `undefined`. --- packages/block-editor/src/hooks/gap.js | 4 ++-- packages/block-library/src/columns/block.json | 2 +- packages/block-library/src/navigation/block.json | 2 +- packages/block-library/src/social-links/block.json | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/hooks/gap.js b/packages/block-editor/src/hooks/gap.js index ba46ea2990874..b897a18520508 100644 --- a/packages/block-editor/src/hooks/gap.js +++ b/packages/block-editor/src/hooks/gap.js @@ -110,8 +110,8 @@ export function GapEdit( props ) { const onChange = ( next ) => { let newValue = next; if ( typeof next === 'object' ) { - const row = next?.top; - const column = next?.left; + const row = next?.top || 0; + const column = next?.left || 0; newValue = row === column ? row : `${ row } ${ column }`; } const newStyle = { diff --git a/packages/block-library/src/columns/block.json b/packages/block-library/src/columns/block.json index 77d5f7d745ce5..a38150da21ddb 100644 --- a/packages/block-library/src/columns/block.json +++ b/packages/block-library/src/columns/block.json @@ -28,7 +28,7 @@ } }, "spacing": { - "blockGap": true, + "blockGap": [ "vertical", "horizontal" ], "margin": [ "top", "bottom" ], "padding": true, "__experimentalDefaultControls": { diff --git a/packages/block-library/src/navigation/block.json b/packages/block-library/src/navigation/block.json index af877a768c1b1..0c9005c2e739d 100644 --- a/packages/block-library/src/navigation/block.json +++ b/packages/block-library/src/navigation/block.json @@ -95,7 +95,7 @@ } }, "spacing": { - "blockGap": true, + "blockGap": [ "vertical", "horizontal" ], "units": [ "px", "em", "rem", "vh", "vw" ], "__experimentalDefaultControls": { "blockGap": true diff --git a/packages/block-library/src/social-links/block.json b/packages/block-library/src/social-links/block.json index 07c7af0a27abe..671814b387a6f 100644 --- a/packages/block-library/src/social-links/block.json +++ b/packages/block-library/src/social-links/block.json @@ -56,7 +56,7 @@ } }, "spacing": { - "blockGap": true, + "blockGap": [ "vertical", "horizontal" ], "margin": [ "top", "bottom" ], "units": [ "px", "em", "rem", "vh", "vw" ], "__experimentalDefaultControls": { From de6da586e8d8bbaf67dc360a5647d581dccd3fcb Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 15 Feb 2022 13:13:56 +1100 Subject: [PATCH 04/10] 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 +- 8 files changed, 114 insertions(+), 47 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 fb0332f1070ab..6020faa72f787 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 > .alignright { float: right; margin-inline-start: 2em; margin-inline-end: 0; }"; $style .= "$selector > .aligncenter { margin-left: auto !important; margin-right: auto !important; }"; 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-block-start: 0; margin-block-end: 0; }"; $style .= "$selector > * + * { margin-block-start: $gap_style; margin-block-end: 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 b308875abb4a0..84a966ca0fdeb 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'; @@ -90,7 +91,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; @@ -113,8 +115,8 @@ export default {