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

Stabilize the __experimentalBorder support #65968

Closed
wants to merge 8 commits into from
Closed

Stabilize the __experimentalBorder support #65968

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cace01d
Update PHP block support code in lib/block-supports/border.php to use…
benazeer-ben Oct 9, 2024
344e635
Update JS implementations to use the non __experimental prefix
benazeer-ben Oct 9, 2024
5277b53
PHPCS FIX
benazeer-ben Oct 11, 2024
aae3411
Merge branch 'trunk' of https://github.com/benazeer-ben/gutenberg int…
benazeer-ben Oct 28, 2024
97a8dd1
Review feedback changes
benazeer-ben Oct 29, 2024
f9dbbfc
PHPCS fixex
benazeer-ben Oct 30, 2024
849a6d3
PHPCS fixes
benazeer-ben Oct 30, 2024
5f2cd17
Backport changelog
benazeer-ben Oct 30, 2024
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
25 changes: 15 additions & 10 deletions lib/block-supports/border.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function gutenberg_register_border_support( $block_type ) {
$block_type->attributes = array();
}

if ( block_has_support( $block_type, array( '__experimentalBorder' ) ) && ! array_key_exists( 'style', $block_type->attributes ) ) {
if ( block_has_support( $block_type, array( 'border', '__experimentalBorder' ) ) && ! array_key_exists( 'style', $block_type->attributes ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't tested this, but I think this needs to have two block_has_supports checks (one for border and one for __experimentalBorder) as an array passed to this function is treated as a path, rather than an OR. I see there's a similar check down in line 158 that looks correct to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestions @andrewserong

I have updated this case in the recent commit.

$block_type->attributes['style'] = array(
'type' => 'object',
);
Expand Down Expand Up @@ -52,7 +52,8 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
if (
gutenberg_has_border_feature_support( $block_type, 'radius' ) &&
isset( $block_attributes['style']['border']['radius'] ) &&
! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'radius' )
( ! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'radius' ) ||
! wp_should_skip_block_supports_serialization( $block_type, 'border', 'radius' ) )
) {
$border_radius = $block_attributes['style']['border']['radius'];

Expand All @@ -67,7 +68,8 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
if (
gutenberg_has_border_feature_support( $block_type, 'style' ) &&
isset( $block_attributes['style']['border']['style'] ) &&
! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'style' )
( ! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'style' ) ||
! wp_should_skip_block_supports_serialization( $block_type, 'border', 'style' ) )
) {
$border_block_styles['style'] = $block_attributes['style']['border']['style'];
}
Expand All @@ -76,7 +78,8 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
if (
$has_border_width_support &&
isset( $block_attributes['style']['border']['width'] ) &&
! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'width' )
( ! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'width' ) ||
! wp_should_skip_block_supports_serialization( $block_type, 'border', 'width' ) )
) {
$border_width = $block_attributes['style']['border']['width'];

Expand All @@ -91,7 +94,8 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
// Border color.
if (
$has_border_color_support &&
! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'color' )
( ! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'color' ) ||
! wp_should_skip_block_supports_serialization( $block_type, 'border', 'color' ) )
) {
$preset_border_color = array_key_exists( 'borderColor', $block_attributes ) ? "var:preset|color|{$block_attributes['borderColor']}" : null;
$custom_border_color = $block_attributes['style']['border']['color'] ?? null;
Expand All @@ -103,9 +107,9 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
foreach ( array( 'top', 'right', 'bottom', 'left' ) as $side ) {
$border = $block_attributes['style']['border'][ $side ] ?? null;
$border_side_values = array(
'width' => isset( $border['width'] ) && ! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'width' ) ? $border['width'] : null,
'color' => isset( $border['color'] ) && ! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'color' ) ? $border['color'] : null,
'style' => isset( $border['style'] ) && ! wp_should_skip_block_supports_serialization( $block_type, '__experimentalBorder', 'style' ) ? $border['style'] : null,
'width' => isset( $border['width'] ) && ! wp_should_skip_block_supports_serialization( $block_type, 'border', 'width' ) ? $border['width'] : null,
'color' => isset( $border['color'] ) && ! wp_should_skip_block_supports_serialization( $block_type, 'border', 'color' ) ? $border['color'] : null,
'style' => isset( $border['style'] ) && ! wp_should_skip_block_supports_serialization( $block_type, 'border', 'style' ) ? $border['style'] : null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to handle the skip serialization for the __experimentalBorder case, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to make sure that, now I have added the skip serialization for the __experimentalBorder case too.

);
$border_block_styles[ $side ] = $border_side_values;
}
Expand Down Expand Up @@ -143,15 +147,16 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
function gutenberg_has_border_feature_support( $block_type, $feature, $default_value = false ) {
// Check if all border support features have been opted into via `"__experimentalBorder": true`.
if ( $block_type instanceof WP_Block_Type ) {
$block_type_supports_border = $block_type->supports['__experimentalBorder'] ?? $default_value;
$block_type_supports_border = $block_type->supports['border'] ?? $block_type->supports['__experimentalBorder'] ?? $default_value;
if ( true === $block_type_supports_border ) {
return true;
}
}

// Check if the specific feature has been opted into individually
// via nested flag under `__experimentalBorder`.
return block_has_support( $block_type, array( '__experimentalBorder', $feature ), $default_value );
return block_has_support( $block_type, array( '__experimentalBorder', $feature ), $default_value )
|| block_has_support( $block_type, array( 'border', $feature ), $default_value );
}

// Register the block support.
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/hooks/border.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
import { store as blockEditorStore } from '../store';
import { __ } from '@wordpress/i18n';

export const BORDER_SUPPORT_KEY = '__experimentalBorder';
export const BORDER_SUPPORT_KEY = 'border';
export const SHADOW_SUPPORT_KEY = 'shadow';

const getColorByProperty = ( colors, property, value ) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/hooks/supports.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Platform } from '@wordpress/element';

const ALIGN_SUPPORT_KEY = 'align';
const ALIGN_WIDE_SUPPORT_KEY = 'alignWide';
const BORDER_SUPPORT_KEY = '__experimentalBorder';
const BORDER_SUPPORT_KEY = 'border';
const COLOR_SUPPORT_KEY = 'color';
const CUSTOM_CLASS_NAME_SUPPORT_KEY = 'customClassName';
const FONT_FAMILY_SUPPORT_KEY = 'typography.__experimentalFontFamily';
Expand Down
32 changes: 16 additions & 16 deletions packages/blocks/src/api/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ export const __EXPERIMENTAL_STYLE_PROPERTY = {
},
borderColor: {
value: [ 'border', 'color' ],
support: [ '__experimentalBorder', 'color' ],
support: [ 'border', 'color' ],
useEngine: true,
},
borderRadius: {
value: [ 'border', 'radius' ],
support: [ '__experimentalBorder', 'radius' ],
support: [ 'border', 'radius' ],
properties: {
borderTopLeftRadius: 'topLeft',
borderTopRightRadius: 'topRight',
Expand All @@ -74,72 +74,72 @@ export const __EXPERIMENTAL_STYLE_PROPERTY = {
},
borderStyle: {
value: [ 'border', 'style' ],
support: [ '__experimentalBorder', 'style' ],
support: [ 'border', 'style' ],
useEngine: true,
},
borderWidth: {
value: [ 'border', 'width' ],
support: [ '__experimentalBorder', 'width' ],
support: [ 'border', 'width' ],
useEngine: true,
},
borderTopColor: {
value: [ 'border', 'top', 'color' ],
support: [ '__experimentalBorder', 'color' ],
support: [ 'border', 'color' ],
useEngine: true,
},
borderTopStyle: {
value: [ 'border', 'top', 'style' ],
support: [ '__experimentalBorder', 'style' ],
support: [ 'border', 'style' ],
useEngine: true,
},
borderTopWidth: {
value: [ 'border', 'top', 'width' ],
support: [ '__experimentalBorder', 'width' ],
support: [ 'border', 'width' ],
useEngine: true,
},
borderRightColor: {
value: [ 'border', 'right', 'color' ],
support: [ '__experimentalBorder', 'color' ],
support: [ 'border', 'color' ],
useEngine: true,
},
borderRightStyle: {
value: [ 'border', 'right', 'style' ],
support: [ '__experimentalBorder', 'style' ],
support: [ 'border', 'style' ],
useEngine: true,
},
borderRightWidth: {
value: [ 'border', 'right', 'width' ],
support: [ '__experimentalBorder', 'width' ],
support: [ 'border', 'width' ],
useEngine: true,
},
borderBottomColor: {
value: [ 'border', 'bottom', 'color' ],
support: [ '__experimentalBorder', 'color' ],
support: [ 'border', 'color' ],
useEngine: true,
},
borderBottomStyle: {
value: [ 'border', 'bottom', 'style' ],
support: [ '__experimentalBorder', 'style' ],
support: [ 'border', 'style' ],
useEngine: true,
},
borderBottomWidth: {
value: [ 'border', 'bottom', 'width' ],
support: [ '__experimentalBorder', 'width' ],
support: [ 'border', 'width' ],
useEngine: true,
},
borderLeftColor: {
value: [ 'border', 'left', 'color' ],
support: [ '__experimentalBorder', 'color' ],
support: [ 'border', 'color' ],
useEngine: true,
},
borderLeftStyle: {
value: [ 'border', 'left', 'style' ],
support: [ '__experimentalBorder', 'style' ],
support: [ 'border', 'style' ],
useEngine: true,
},
borderLeftWidth: {
value: [ 'border', 'left', 'width' ],
support: [ '__experimentalBorder', 'width' ],
support: [ 'border', 'width' ],
useEngine: true,
},
color: {
Expand Down
60 changes: 39 additions & 21 deletions packages/blocks/src/store/process-block-type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,22 @@ function mergeBlockVariations(

return result;
}
function handleExperimentalBorder( rawSupports ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll also need to handle the transformation for Typography at some point (which I worked on in #63401). Would it be worth renaming this to stabilizeSupports so that we have one function call, and we can add other stabilized supports to the body of the function in follow-ups?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function name has been updated as you mentioned above.

if ( ! rawSupports ) {
return rawSupports;
}

const supports = { ...rawSupports };

if (
supports?.__experimentalBorder &&
typeof supports.__experimentalBorder === 'object'
) {
supports.border = supports.__experimentalBorder;
}

return supports;
}
/**
* Takes the unprocessed block type settings, merges them with block type metadata
* and applies all the existing filters for the registered block type.
Expand Down Expand Up @@ -101,13 +116,15 @@ export const processBlockType =
: []
),
};
blockType.supports = handleExperimentalBorder( blockType.supports );

const settings = applyFilters(
'blocks.registerBlockType',
blockType,
name,
null
);
blockType.supports = handleExperimentalBorder( blockType.supports );

if (
settings.description &&
Expand All @@ -119,29 +136,30 @@ export const processBlockType =
}

if ( settings.deprecated ) {
settings.deprecated = settings.deprecated.map( ( deprecation ) =>
Object.fromEntries(
Object.entries(
// Only keep valid deprecation keys.
applyFilters(
'blocks.registerBlockType',
// Merge deprecation keys with pre-filter settings
// so that filters that depend on specific keys being
// present don't fail.
{
// Omit deprecation keys here so that deprecations
// can opt out of specific keys like "supports".
...omit( blockType, DEPRECATED_ENTRY_KEYS ),
...deprecation,
},
blockType.name,
deprecation
)
).filter( ( [ key ] ) =>
settings.deprecated = settings.deprecated.map( ( deprecation ) => {
deprecation.supports = handleExperimentalBorder(
deprecation.supports
);

const filteredDeprecation = applyFilters(
'blocks.registerBlockType',
{
...omit( blockType, DEPRECATED_ENTRY_KEYS ),
...deprecation,
},
blockType.name,
deprecation
);
filteredDeprecation.supports = handleExperimentalBorder(
filteredDeprecation.supports
);

return Object.fromEntries(
Object.entries( filteredDeprecation ).filter( ( [ key ] ) =>
DEPRECATED_ENTRY_KEYS.includes( key )
)
)
);
);
} );
}

if ( ! isPlainObject( settings ) ) {
Expand Down
Loading