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

Borders: Stabilize border block supports within block processing #66918

Merged
merged 8 commits into from
Nov 20, 2024
Merged

Borders: Stabilize border block supports within block processing #66918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5c954b8
Use stabilized border support key
aaronrobertshaw Nov 8, 2024
03f745d
Update stabilization approach to include border support
aaronrobertshaw Nov 8, 2024
0377da1
Add unit tests for border support stabilization
aaronrobertshaw Nov 8, 2024
50caa72
Update processBlockType tests for border stabilization
aaronrobertshaw Nov 12, 2024
32df888
Fix global styles unit test
aaronrobertshaw Nov 13, 2024
611de98
Make comments match between JS and PHP stabilizations
aaronrobertshaw Nov 13, 2024
f51cbc0
Add border support stabilization to existing backport log
aaronrobertshaw Nov 13, 2024
9c32c4a
Try improving comments around importance of order
aaronrobertshaw Nov 20, 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
1 change: 1 addition & 0 deletions backport-changelog/6.8/7069.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
https://github.com/WordPress/wordpress-develop/pull/7069

* https://github.com/WordPress/gutenberg/pull/63401
* https://github.com/WordPress/gutenberg/pull/66918
28 changes: 14 additions & 14 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' ) ) && ! array_key_exists( 'style', $block_type->attributes ) ) {
$block_type->attributes['style'] = array(
'type' => 'object',
);
Expand Down Expand Up @@ -52,7 +52,7 @@ 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, 'border', 'radius' )
) {
$border_radius = $block_attributes['style']['border']['radius'];

Expand All @@ -67,7 +67,7 @@ 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, 'border', 'style' )
) {
$border_block_styles['style'] = $block_attributes['style']['border']['style'];
}
Expand All @@ -76,7 +76,7 @@ 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, 'border', 'width' )
) {
$border_width = $block_attributes['style']['border']['width'];

Expand All @@ -91,7 +91,7 @@ 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, '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 +103,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,
);
$border_block_styles[ $side ] = $border_side_values;
}
Expand All @@ -129,9 +129,9 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
/**
* Checks whether the current block type supports the border feature requested.
*
* If the `__experimentalBorder` support flag is a boolean `true` all border
* If the `border` support flag is a boolean `true` all border
* support features are available. Otherwise, the specific feature's support
* flag nested under `experimentalBorder` must be enabled for the feature
* flag nested under `border` must be enabled for the feature
* to be opted into.
*
* @param WP_Block_Type $block_type Block type to check for support.
Expand All @@ -141,17 +141,17 @@ function gutenberg_apply_border_support( $block_type, $block_attributes ) {
* @return boolean Whether or not the feature is supported.
*/
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`.
// Check if all border support features have been opted into via `"border": 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'] ?? $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 );
// via nested flag under `border`.
return block_has_support( $block_type, array( 'border', $feature ), $default_value );
}

// Register the block support.
Expand Down
8 changes: 4 additions & 4 deletions lib/class-wp-theme-json-gutenberg.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,10 +615,10 @@ class WP_Theme_JSON_Gutenberg {
* @var string[]
*/
const BLOCK_SUPPORT_FEATURE_LEVEL_SELECTORS = array(
'__experimentalBorder' => 'border',
'color' => 'color',
'spacing' => 'spacing',
'typography' => 'typography',
'border' => 'border',
'color' => 'color',
'spacing' => 'spacing',
'typography' => 'typography',
);

/**
Expand Down
92 changes: 73 additions & 19 deletions lib/compat/wordpress-6.8/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,94 @@
*/

/**
* Filters the block type arguments during registration to stabilize experimental block supports.
* Filters the block type arguments during registration to stabilize
* experimental block supports.
*
* This is a temporary compatibility shim as the approach in core is for this to be handled
* within the WP_Block_Type class rather than requiring a filter.
* This is a temporary compatibility shim as the approach in core is for this
* to be handled within the WP_Block_Type class rather than requiring a filter.
*
* @param array $args Array of arguments for registering a block type.
* @return array Array of arguments for registering a block type.
*/
function gutenberg_stabilize_experimental_block_supports( $args ) {
if ( empty( $args['supports']['typography'] ) ) {
if ( empty( $args['supports'] ) ) {
return $args;
}

$experimental_typography_supports_to_stable = array(
'__experimentalFontFamily' => 'fontFamily',
'__experimentalFontStyle' => 'fontStyle',
'__experimentalFontWeight' => 'fontWeight',
'__experimentalLetterSpacing' => 'letterSpacing',
'__experimentalTextDecoration' => 'textDecoration',
'__experimentalTextTransform' => 'textTransform',
$experimental_to_stable_keys = array(
'typography' => array(
'__experimentalFontFamily' => 'fontFamily',
'__experimentalFontStyle' => 'fontStyle',
'__experimentalFontWeight' => 'fontWeight',
'__experimentalLetterSpacing' => 'letterSpacing',
'__experimentalTextDecoration' => 'textDecoration',
'__experimentalTextTransform' => 'textTransform',
),
'__experimentalBorder' => 'border',
);

$current_typography_supports = $args['supports']['typography'];
$stable_typography_supports = array();
$updated_supports = array();
foreach ( $args['supports'] as $support => $config ) {
// Add the support's config as is when it's not in need of stabilization.
if ( empty( $experimental_to_stable_keys[ $support ] ) ) {
$updated_supports[ $support ] = $config;
continue;
}

// Stabilize the support's key if needed e.g. __experimentalBorder => border.
if ( is_string( $experimental_to_stable_keys[ $support ] ) ) {
$stabilized_key = $experimental_to_stable_keys[ $support ];

// If there is no stabilized key present, use the experimental config as is.
if ( ! array_key_exists( $stabilized_key, $args['supports'] ) ) {
$updated_supports[ $stabilized_key ] = $config;
continue;
}

/*
* Determine the order of keys, so the last defined can be preferred.
*
* The reason for preferring the last defined key is that after filters
* are applied, the last inserted key is likely the most up-to-date value.
* We cannot determine with certainty which value was "last modified" so
* the insertion order is the best guess. The extreme edge case of multiple
* filters tweaking the same support property will become less over time as
* extenders migrate existing blocks and plugins to stable keys.
*/
$key_positions = array_flip( array_keys( $args['supports'] ) );
$experimental_index = $key_positions[ $support ] ?? -1;
$stabilized_index = $key_positions[ $stabilized_key ] ?? -1;
$experimental_first = $experimental_index < $stabilized_index;

// Update support config, prefer the last defined value.
if ( is_array( $config ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not at all a blocker for now, but in the JS implementation we check for an object, so there'd never be a match against the align support, where an array is a valid value. Here, there theoretically could be a match.

In practice it's not an issue because we don't have anything in $experimental_to_stable_keys that references supports where an array is a valid value, but just thought I'd mention it for the (very) unlikely event that at some distant point in the future we create an experimental support that does accept an array and then want to stabilize it further down the track. In that case, this condition might need to be updated.

For now, though, I think it's fine to leave as-is. And please excuse my verbose comment here 😄

Copy link
Contributor Author

@aaronrobertshaw aaronrobertshaw Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Appreciate you raising this one. It has vaguely been on my radar but I was struggling to come up with a real-word case where it should happen.

The align support is stable, so wouldn't be in the $experimental_to_stable_keys array, as you note. In that case, the guard clause at the beginning of the loop should just continue on, while using the align config as is.

In the end, rather than waste too much time on coming up with hypotheticals to test, I decided to leave this until it was a problem.

Does that allay your concern at all? What else am I missing?

Copy link
Contributor Author

@aaronrobertshaw aaronrobertshaw Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I meant to also add that in recent times, there's been a decided effort to not land block supports at all unless they were stable. To prevent being in the situation like we are with some typography and border supports. That's been the case for entire block support features like shadow or subfeatures like textColumns and minHeight.

If the approach to not landing experimental block supports continues, there shouldn't be a scenario with a new experimental block support that needs stabilizing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely, what you've described captures my thinking, too, so you're not missing anything here!

In the end, rather than waste too much time on coming up with hypotheticals to test, I decided to leave this until it was a problem.

That's where I was coming from, too. I was mostly thinking out loud, which is to say, flagging something that could theoretically be an issue at some distant point in the future. And it's at that distant point (if we're adding an experimental support that uses arrays) where I think it could be addressed, rather than now 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

If the approach to not landing experimental block supports continues, there shouldn't be a scenario with a new experimental block support that needs stabilizing.

Indeed — my vote is to avoid using the __experimental prefix for things in the future, 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.

👍 Having it on the record here will also flag it for our future selves or others, if it is really needed in the end.

$updated_supports[ $stabilized_key ] = $experimental_first
? array_merge( $config, $args['supports'][ $stabilized_key ] )
: array_merge( $args['supports'][ $stabilized_key ], $config );
} else {
$updated_supports[ $stabilized_key ] = $experimental_first
? $args['supports'][ $stabilized_key ]
: $config;
}

continue;
}

foreach ( $current_typography_supports as $key => $value ) {
if ( array_key_exists( $key, $experimental_typography_supports_to_stable ) ) {
$stable_typography_supports[ $experimental_typography_supports_to_stable[ $key ] ] = $value;
} else {
$stable_typography_supports[ $key ] = $value;
// Stabilize individual support feature keys e.g. __experimentalFontFamily => fontFamily.
if ( is_array( $experimental_to_stable_keys[ $support ] ) ) {
$stable_support_config = array();
foreach ( $config as $key => $value ) {
if ( array_key_exists( $key, $experimental_to_stable_keys[ $support ] ) ) {
$stable_support_config[ $experimental_to_stable_keys[ $support ][ $key ] ] = $value;
} else {
$stable_support_config[ $key ] = $value;
}
}
$updated_supports[ $support ] = $stable_support_config;
}
}

$args['supports']['typography'] = $stable_typography_supports;
$args['supports'] = $updated_supports;

return $args;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/global-styles/test/use-global-styles-output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ describe( 'global styles renderer', () => {

it( 'should return block selectors data with old experimental selectors', () => {
const imageSupports = {
__experimentalBorder: {
border: {
radius: true,
__experimentalSelector: 'img, .crop-area',
},
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/global-styles/use-global-styles-output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const ELEMENT_CLASS_NAMES = {
// List of block support features that can have their related styles
// generated under their own feature level selector rather than the block's.
const BLOCK_SUPPORT_FEATURE_LEVEL_SELECTORS = {
__experimentalBorder: 'border',
border: 'border',
color: 'color',
spacing: 'spacing',
typography: 'typography',
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.fontFamily';
Expand Down
49 changes: 26 additions & 23 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 Expand Up @@ -298,11 +298,14 @@ export const __EXPERIMENTAL_PATHS_WITH_OVERRIDE = {
'spacing.spacingSizes': true,
};

export const TYPOGRAPHY_SUPPORTS_EXPERIMENTAL_TO_STABLE = {
__experimentalFontFamily: 'fontFamily',
__experimentalFontStyle: 'fontStyle',
__experimentalFontWeight: 'fontWeight',
__experimentalLetterSpacing: 'letterSpacing',
__experimentalTextDecoration: 'textDecoration',
__experimentalTextTransform: 'textTransform',
export const EXPERIMENTAL_TO_STABLE_KEYS = {
typography: {
__experimentalFontFamily: 'fontFamily',
__experimentalFontStyle: 'fontStyle',
__experimentalFontWeight: 'fontWeight',
__experimentalLetterSpacing: 'letterSpacing',
__experimentalTextDecoration: 'textDecoration',
__experimentalTextTransform: 'textTransform',
},
__experimentalBorder: 'border',
};
Loading
Loading