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.

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

Block Support: Add border radius support #25791

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
89 changes: 89 additions & 0 deletions lib/block-supports/border.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php
/**
* Border block support flag.
*
* @package gutenberg
*/

/**
* Registers the style attribute used by the border feature if needed for block types that
* support borders.
*
* @param WP_Block_Type $block_type Block Type.
*/
function gutenberg_register_border_support( $block_type ) {
// Determine border related features supported.
// Border width, style etc can be added in the future.
$has_border_radius_support = gutenberg_has_border_support( $block_type, 'radius' );

// Setup attributes and styles within that if needed.
if ( ! $block_type->attributes ) {
$block_type->attributes = array();
}

if ( $has_border_radius_support && ! array_key_exists( 'style', $block_type->attributes ) ) {
$block_type->attributes['style'] = array(
'type' => 'object',
);
}
}

/**
* Adds CSS classes and inline styles for border styles to the incoming
* attributes array. This will be applied to the block markup in the front-end.
*
* @param WP_Block_type $block_type Block type.
* @param array $block_attributes Block attributes.
*
* @return array Border CSS classes and inline styles.
*/
function gutenberg_apply_border_support( $block_type, $block_attributes ) {
// Arrays used to ease addition of further border related features in future.
$styles = array();

// Border Radius.
if ( gutenberg_has_border_support( $block_type, 'radius' ) ) {
if ( isset( $block_attributes['style']['border']['radius'] ) ) {
$border_radius = intval( $block_attributes['style']['border']['radius'] );
$styles[] = sprintf( 'border-radius: %dpx;', $border_radius );
}
}

// Border width, style etc can be added here.

// Collect classes and styles.
$attributes = array();

if ( ! empty( $styles ) ) {
$attributes['style'] = implode( ' ', $styles );
}

return $attributes;
}

/**
* Checks whether the current block type supports the feature requested.
*
* @param WP_Block_Type $block_type Block type to check for support.
* @param string $feature Name of the feature to check support for.
* @param mixed $default Fallback value for feature support, defaults to false.
*
* @return boolean Whether or not the feature is supported.
*/
function gutenberg_has_border_support( $block_type, $feature, $default = false ) {
$block_support = false;
if ( property_exists( $block_type, 'supports' ) ) {
$block_support = gutenberg_experimental_get( $block_type->supports, array( '__experimentalBorder' ), $default );
}

return true === $block_support || ( is_array( $block_support ) && gutenberg_experimental_get( $block_support, array( $feature ), false ) );
}

// Register the block support.
WP_Block_Supports::get_instance()->register(
'border',
array(
'register_attribute' => 'gutenberg_register_border_support',
'apply' => 'gutenberg_apply_border_support',
)
);
8 changes: 8 additions & 0 deletions lib/class-wp-theme-json.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class WP_Theme_JSON {
'--wp--style--color--link',
'background',
'backgroundColor',
'border',
Copy link
Member

Choose a reason for hiding this comment

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

The values on this array need to be the name of the property as they appear in the PROPERTIES_METADATA, so this should have been borderRadius. There's a couple of other things I'd like to share:

  • these styles are attached to the :root element, so I'm not sure borderRadius makes a lot of sense here
  • the global styles sidebar doesn't have a corresponding UI control for the user to tweak this property ― neither for the global or the group block.

In #28320 I'm removing the support for borderRadius for the root element but could also just rename the property to fix the issue.

@aaronrobertshaw would you be available to create a follow-up PR that adds the radius control to the global styles sidebar if the block supports it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nosolosw thank you for the explanation. I agree, a border-radius value on :root doesn't make a lot of sense.

It was suggested via feedback on other border radius related PRs that there should be a "global" border radius value that themes could use or supply to achieve a more consistent visual result. I mention this as having the border radius as a CSS variable on the :root selector would help achieve this.

In #27991 I've used a similar approach adding a CSS variable for the border radius so that inner elements could adjust the value to keep uniform spacing between their border and their parent's.

It would be great to get your thoughts on that PR if you can spare the time.

@aaronrobertshaw would you be available to create a follow-up PR that adds the radius control to the global styles sidebar if the block supports it?

I can do however I had been advised to avoid adding any further sidebar controls until #27331 lands. Complicating matters and the UI, there are also requests to include support for other border properties such as width and style. I realise these aren't in place at the moment, just mentioning as they seemed to have contributed to the decision to hold off adding extra controls.

Again, I'm happy to do the follow-up if you can confirm I'm ok to do so.

Copy link
Member

Choose a reason for hiding this comment

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

I've left my comments on #27991 and #27664 (comment)

I can do however I had been advised to avoid adding any further sidebar controls until #27331 lands. Complicating matters and the UI, there are also requests to include support for other border properties such as width and style. I realise these aren't in place at the moment, just mentioning as they seemed to have contributed to the decision to hold off adding extra controls.

Can you fill me up a bit on this?

Would it work if we add the UI control for the GS sidebar but we keep the customRadius to false? That way we have the support built-in, themes can use it via theme.json but users won't see the controls. Same for the other border properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a PR up adding the border radius controls to the global styles sidebar #28346

Can you fill me up a bit on this?

Some extra background on past discussions regarding controls for border related support can be found in #21540 (comment).

I'm happy to change approach. Whatever we need to get the best result.

'color',
'fontFamily',
'fontSize',
Expand Down Expand Up @@ -114,6 +115,9 @@ class WP_Theme_JSON {
),
),
'settings' => array(
'border' => array(
'customRadius' => null,
),
'color' => array(
'custom' => null,
'customGradient' => null,
Expand Down Expand Up @@ -284,6 +288,10 @@ class WP_Theme_JSON {
'value' => array( 'color', 'background' ),
'support' => array( 'color' ),
),
'borderRadius' => array(
'value' => array( 'border', 'radius' ),
'support' => array( '__experimentalBorder' ),
),
'color' => array(
'value' => array( 'color', 'text' ),
'support' => array( 'color' ),
Expand Down
3 changes: 3 additions & 0 deletions lib/experimental-default-theme.json
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,9 @@
"spacing": {
"customPadding": false,
"units": [ "px", "em", "rem", "vh", "vw" ]
},
"border": {
"customRadius": true
}
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,4 @@ function gutenberg_is_experiment_enabled( $name ) {
require __DIR__ . '/block-supports/align.php';
require __DIR__ . '/block-supports/typography.php';
require __DIR__ . '/block-supports/custom-classname.php';
require __DIR__ . '/block-supports/border.php';
79 changes: 79 additions & 0 deletions packages/block-editor/src/hooks/border-radius.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/**
* WordPress dependencies
*/
import { getBlockSupport } from '@wordpress/blocks';
import { RangeControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import useEditorFeature from '../components/use-editor-feature';
import { BORDER_SUPPORT_KEY } from './border';
import { cleanEmptyObject } from './utils';

const MIN_BORDER_RADIUS_VALUE = 0;
const MAX_BORDER_RADIUS_VALUE = 50;

/**
* Inspector control panel containing the border radius related configuration.
*
* @param {Object} props Block properties.
* @return {WPElement} Border radius edit element.
*/
export function BorderRadiusEdit( props ) {
const {
attributes: { style },
setAttributes,
} = props;

if ( useIsBorderRadiusDisabled( props ) ) {
return null;
}

const onChange = ( newRadius ) => {
const newStyle = {
...style,
border: {
...style?.border,
radius: newRadius,
},
};

setAttributes( { style: cleanEmptyObject( newStyle ) } );
};

return (
<RangeControl
value={ style?.border?.radius }
label={ __( 'Border radius' ) }
min={ MIN_BORDER_RADIUS_VALUE }
max={ MAX_BORDER_RADIUS_VALUE }
initialPosition={ 0 }
allowReset
onChange={ onChange }
/>
);
}

/**
* Determines if there is border radius support.
*
* @param {string|Object} blockType Block name or Block Type object.
* @return {boolean} Whether there is support.
*/
export function hasBorderRadiusSupport( blockType ) {
const support = getBlockSupport( blockType, BORDER_SUPPORT_KEY );
return true === support || ( support && !! support.radius );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return true === support || ( support && !! support.radius );
return !! ( true === support || support?.radius );

}

/**
* Custom hook that checks if border radius settings have been disabled.
*
* @param {string} name The name of the block.
* @return {boolean} Whether border radius setting is disabled.
*/
export function useIsBorderRadiusDisabled( { name: blockName } = {} ) {
const isDisabled = ! useEditorFeature( 'border.customRadius' );
return ! hasBorderRadiusSupport( blockName ) || isDisabled;
}
67 changes: 67 additions & 0 deletions packages/block-editor/src/hooks/border.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* WordPress dependencies
*/
import { getBlockSupport } from '@wordpress/blocks';
import { PanelBody } from '@wordpress/components';
import { Platform } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import InspectorControls from '../components/inspector-controls';
import { BorderRadiusEdit, useIsBorderRadiusDisabled } from './border-radius';

export const BORDER_SUPPORT_KEY = '__experimentalBorder';

export function BorderPanel( props ) {
const isDisabled = useIsBorderDisabled( props );
const isSupported = hasBorderSupport( props.name );

if ( isDisabled || ! isSupported ) {
return null;
}

return (
<InspectorControls>
<PanelBody title={ __( 'Border settings' ) }>
<BorderRadiusEdit { ...props } />
</PanelBody>
</InspectorControls>
);
}

/**
* Determine whether there is block support for borders.
*
* @param {string} blockName Block name.
* @return {boolean} Whether there is support.
*/
export function hasBorderSupport( blockName ) {
if ( Platform.OS !== 'web' ) {
return false;
}

const support = getBlockSupport( blockName, BORDER_SUPPORT_KEY );

// Further border properties to be added in future iterations.
// e.g. support && ( support.radius || support.width || support.style )
return true === support || ( support && support.radius );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return true === support || ( support && support.radius );
return true === support || support?.radius;

If you care about the return type being boolean:

Suggested change
return true === support || ( support && support.radius );
return !! ( true === support || support?.radius );

}

/**
* Determines whether there is any block support for borders e.g. border radius,
* style, width etc.
*
* @param {Object} props Block properties.
* @return {boolean} If border support is completely disabled.
*/
const useIsBorderDisabled = ( props = {} ) => {
// Further border properties to be added in future iterations.
// e.g. const configs = [
// useIsBorderRadiusDisabled( props ),
// useIsBorderWidthDisabled( props ),
// ];
const configs = [ useIsBorderRadiusDisabled( props ) ];
return configs.filter( Boolean ).length === configs.length;
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const configs = [ useIsBorderRadiusDisabled( props ) ];
return configs.filter( Boolean ).length === configs.length;
return [ useIsBorderRadiusDisabled( props ) ].every( Boolean );

};
3 changes: 3 additions & 0 deletions packages/block-editor/src/hooks/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ import { createHigherOrderComponent } from '@wordpress/compose';
/**
* Internal dependencies
*/
import { BORDER_SUPPORT_KEY, BorderPanel } from './border';
import { COLOR_SUPPORT_KEY, ColorEdit } from './color';
import { TypographyPanel, TYPOGRAPHY_SUPPORT_KEYS } from './typography';
import { SPACING_SUPPORT_KEY, PaddingEdit } from './padding';
import SpacingPanelControl from '../components/spacing-panel-control';

const styleSupportKeys = [
...TYPOGRAPHY_SUPPORT_KEYS,
BORDER_SUPPORT_KEY,
COLOR_SUPPORT_KEY,
SPACING_SUPPORT_KEY,
];
Expand Down Expand Up @@ -156,6 +158,7 @@ export const withBlockControls = createHigherOrderComponent(

return [
<TypographyPanel key="typography" { ...props } />,
<BorderPanel key="border" { ...props } />,
<ColorEdit key="colors" { ...props } />,
<BlockEdit key="edit" { ...props } />,
hasSpacingSupport && (
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/hooks/test/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ describe( 'getInlineStyles', () => {
getInlineStyles( {
color: { text: 'red', background: 'black' },
typography: { lineHeight: 1.5, fontSize: 10 },
border: { radius: 10 },
} )
).toEqual( {
backgroundColor: 'black',
borderRadius: 10,
color: 'red',
lineHeight: 1.5,
fontSize: 10,
Expand Down
4 changes: 4 additions & 0 deletions packages/blocks/src/api/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ export const __EXPERIMENTAL_STYLE_PROPERTY = {
value: [ 'color', 'background' ],
support: [ 'color' ],
},
borderRadius: {
value: [ 'border', 'radius' ],
support: [ '__experimentalBorder', 'radius' ],
},
color: {
value: [ 'color', 'text' ],
support: [ 'color' ],
Expand Down