Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Block spacing: block-level axial gap block support #37736

Merged
merged 10 commits into from
Mar 22, 2022
Merged

Block spacing: block-level axial gap block support #37736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1f7a1be
Initial commit: splitting the gap value to allow for axial gap values
ramonjd Jan 6, 2022
9ff0734
Checking for object. Now passing complete object to box control.
ramonjd Jan 25, 2022
72021d8
Providing a default value for split gap values, where one is `undefin…
ramonjd Feb 10, 2022
de6da58
This is the first commit that switches the blockGap over to use to an…
ramonjd Feb 15, 2022
be909d7
margin-top should take a single value
ramonjd Feb 16, 2022
34b7f8c
null coalescing operator (??) is not present in PHP
ramonjd Feb 16, 2022
2191831
Checking for gap array items before using them.
ramonjd Feb 21, 2022
119333b
Using `--wp--style--block-gap` as the fallback for split values
ramonjd Mar 14, 2022
765ca94
Returning null instead of the argument in getGapCSSValue and updated …
ramonjd Mar 16, 2022
b618d4e
Rolling back using the gap CSS var as a fallback since the CSS var ca…
ramonjd Mar 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions lib/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = isset( $gap_value['top'] ) ? $gap_value['top'] : null;
}
$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; }";
Expand All @@ -91,11 +94,17 @@ 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_row = isset( $gap_value['top'] ) ? $gap_value['top'] : '0.5em';
$gap_column = isset( $gap_value['left'] ) ? $gap_value['left'] : '0.5em';
$gap_value = $gap_row === $gap_column ? $gap_row : $gap_row . ' ' . $gap_column;
}
$gap_style = $gap_value ? $gap_value : 'var( --wp--style--block-gap, 0.5em )';
$style .= "gap: $gap_style;";
} else {
$style .= 'gap: 0.5em;';
}

$style .= "flex-wrap: $flex_wrap;";
if ( 'horizontal' === $layout_orientation ) {
$style .= 'align-items: center;';
Expand Down Expand Up @@ -155,8 +164,15 @@ function gutenberg_render_layout_support_flag( $block_content, $block ) {
// Skip if gap value contains unsupported characters.
// Regex for CSS value borrowed from `safecss_filter_attr`, and used here
// because we only want to match against the value, not the CSS attribute.
$gap_value = preg_match( '%[\\\(&=}]|/\*%', $gap_value ) ? null : $gap_value;
$style = gutenberg_get_layout_style( ".$class_name", $used_layout, $has_block_gap_support, $gap_value );
if ( is_array( $gap_value ) ) {
foreach ( $gap_value as $key => $value ) {
$gap_value[ $key ] = preg_match( '%[\\\(&=}]|/\*%', $value ) ? null : $value;
}
} else {
$gap_value = preg_match( '%[\\\(&=}]|/\*%', $gap_value ) ? null : $gap_value;
}

$style = gutenberg_get_layout_style( ".$class_name", $used_layout, $has_block_gap_support, $gap_value );
// This assumes the hook only applies to blocks with a single wrapper.
// I think this is a reasonable limitation for that particular hook.
$content = preg_replace(
Expand Down
95 changes: 83 additions & 12 deletions packages/block-editor/src/hooks/gap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -14,14 +15,14 @@ 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';

/**
* 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 );
Expand All @@ -38,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;
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
}

const isValueString = typeof rawBlockGapValue === 'string';
return {
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|null} The concatenated gap value (row and column).
*/
export function getGapCSSValue( blockGapValue, defaultValue = '0' ) {
const blockGapBoxControlValue = getGapValueFromStyle( blockGapValue );
if ( ! blockGapBoxControlValue ) {
return null;
}

const row = blockGapBoxControlValue?.top || defaultValue;
const column = blockGapBoxControlValue?.left || defaultValue;

return row === column ? row : `${ row } ${ column }`;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I was wondering whether we should be returning the full definition so that we can isolate row and column values where only one is set, e.g.,

// row and column is set or is the same
return `gap: 10px 10px;`;

// only row is set
return  `row-gap: 10px;`;

// only column is set
return  `column-gap: 10px;`;

That way, if a block ever has to inherit either row/column we're not overwriting it.

Just typing out loud...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a nice feature, though if we do it here we should do it on the server-side too 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some further tinkering I probably need to put more thought into this.

I'm picturing a future where theme.json authors could add something like this.

"spacing": {
    "blockGap": {
         "row": "0.5em",
         "column": "2em"
     }
}

and blocks would then inherit or use these values as defaults.

Currently "blockGap" accept a string, which can be really anything, including the shorthand value of "0.5em 2em" so it might take some more cogitating to make things backwards compatible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we don't need it for this PR; better to work out the details as a future enhancement.


/**
* Resets the gap block support attribute. This can be used when disabling
* the gap support controls for a block via a progressive discovery panel.
Expand Down Expand Up @@ -82,6 +122,7 @@ export function GapEdit( props ) {
const {
clientId,
attributes: { style },
name: blockName,
setAttributes,
} = props;

Expand All @@ -94,7 +135,7 @@ export function GapEdit( props ) {
'vw',
],
} );

const sides = useCustomSides( blockName, 'blockGap' );
const ref = useBlockRef( clientId );

if ( useIsGapDisabled( props ) ) {
Expand All @@ -106,7 +147,9 @@ export function GapEdit( props ) {
...style,
spacing: {
...style?.spacing,
blockGap: next,
blockGap: {
...getGapValueFromStyle( next ),
},
},
};

Expand All @@ -128,17 +171,45 @@ export function GapEdit( props ) {
}
};

const splitOnAxis =
sides && sides.some( ( side ) => AXIAL_SIDES.includes( side ) );
const gapValue = getGapValueFromStyle( 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.
const boxControlGapValue = splitOnAxis
? {
...gapValue,
right: gapValue?.left,
bottom: gapValue?.top,
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
}
: gapValue?.top;

return Platform.select( {
web: (
<>
<UnitControl
label={ __( 'Block spacing' ) }
__unstableInputWidth="80px"
min={ 0 }
onChange={ onChange }
units={ units }
value={ style?.spacing?.blockGap }
/>
{ splitOnAxis ? (
<BoxControl
label={ __( 'Block spacing' ) }
min={ 0 }
onChange={ onChange }
units={ units }
sides={ sides }
values={ boxControlGapValue }
allowReset={ false }
splitOnAxis={ splitOnAxis }
/>
) : (
<UnitControl
label={ __( 'Block spacing' ) }
__unstableInputWidth="80px"
min={ 0 }
onChange={ onChange }
units={ units }
// Default to `row` for combined values.
value={ boxControlGapValue }
/>
) }
</>
),
native: null,
Expand Down
42 changes: 42 additions & 0 deletions packages/block-editor/src/hooks/test/gap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* Internal dependencies
*/
import { getGapCSSValue } from '../gap';

describe( 'gap', () => {
describe( 'getGapCSSValue()', () => {
it( 'should return `null` if argument is falsey', () => {
expect( getGapCSSValue( undefined ) ).toBeNull();
expect( getGapCSSValue( '' ) ).toBeNull();
} );

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'
);
} );
} );
} );
6 changes: 4 additions & 2 deletions packages/block-editor/src/layouts/flex.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand All @@ -113,8 +115,8 @@ export default {
<style>{ `
${ appendSelectors( selector ) } {
display: flex;
gap: ${ hasBlockGapStylesSupport ? blockGapValue : '0.5em' };
flex-wrap: ${ flexWrap };
gap: ${ hasBlockGapStylesSupport ? blockGapValue : '0.5em' };
${ orientation === 'horizontal' ? rowOrientation : columnOrientation }
}

Expand Down
6 changes: 5 additions & 1 deletion packages/block-editor/src/layouts/flow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { Icon, positionCenter, stretchWide } from '@wordpress/icons';
*/
import useSetting from '../components/use-setting';
import { appendSelectors } from './utils';
import { getGapValueFromStyle } from '../hooks/gap';

export default {
name: 'default',
Expand Down Expand Up @@ -109,8 +110,11 @@ export default {
const { contentSize, wideSize } = layout;
const blockGapSupport = useSetting( 'spacing.blockGap' );
const hasBlockGapStylesSupport = blockGapSupport !== null;
const blockGapStyleValue = getGapValueFromStyle(
style?.spacing?.blockGap
);
const blockGapValue =
style?.spacing?.blockGap ?? 'var( --wp--style--block-gap )';
blockGapStyleValue?.top ?? 'var( --wp--style--block-gap )';

let output =
!! contentSize || !! wideSize
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/social-links/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
}
},
"spacing": {
"blockGap": true,
"blockGap": [ "horizontal", "vertical" ],
"margin": [ "top", "bottom" ],
"units": [ "px", "em", "rem", "vh", "vw" ],
"__experimentalDefaultControls": {
Expand Down