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

Backport min-height block support feature #3940

Closed
wants to merge 13 commits into from
Closed

Backport min-height block support feature #3940

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a668ff3
Backport min-height block support feature
andrewserong Jan 30, 2023
c2bb6cd
Fix failing tests
andrewserong Jan 30, 2023
c3be880
Fix linting issue
andrewserong Jan 30, 2023
df4d498
Add missing line
andrewserong Jan 30, 2023
e79718f
Add full stop to comment
andrewserong Jan 31, 2023
6b1b594
Simplify tests
andrewserong Jan 31, 2023
19fa05e
Fix linting issue
andrewserong Jan 31, 2023
432be91
Empty commit to rerun tests
andrewserong Jan 31, 2023
69e218f
Update data provider docblock.
andrewserong Feb 1, 2023
adb55ca
Renames test file for coding standard
hellofromtonya Feb 1, 2023
cf79639
Rename test class to coding standards
hellofromtonya Feb 1, 2023
811495e
Minor formatting in tests
hellofromtonya Feb 1, 2023
12cebb4
Capitalize 1st letter of function name
hellofromtonya Feb 1, 2023
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
27 changes: 21 additions & 6 deletions src/wp-includes/block-supports/dimensions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ function wp_register_dimensions_support( $block_type ) {
return;
}

$has_dimensions_support = block_has_support( $block_type, array( '__experimentalDimensions' ), false );
// Future block supports such as height & width will be added here.
$has_dimensions_support = block_has_support( $block_type, array( 'dimensions' ), false );
Copy link
Contributor

Choose a reason for hiding this comment

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

Results from a wp.org search:

So I'm wondering:

  • What are the backwards-compatibility risks for users who may have a custom plugin or theme using it?
  • How was '__experimentalDimensions' previously exposed and used?
  • What is the mitigation plan for '__experimentalDimensions' to upgrade blocks using this setting?

Copy link
Contributor

Choose a reason for hiding this comment

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

One way to mitigate:

  • Check it here with an ||.
  • Throw a deprecation notice to alert to use the new 'dimensions' instead.

Copy link
Contributor Author

@andrewserong andrewserong Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for raising this one @hellofromtonya — this is a pretty interesting case for stabilisation in that the support technically existed prior to this PR (with the name __experimentalDimensions) however it was a no-op, in that it didn't actually expose any controls or styles. So even if a plugin had opted in to using it (which is pretty unlikely), there would have been no behaviour from it. Because min-height is a new feature, and the dimensions controls were never exposed or promoted anywhere, I don't think we need to add any handling to deal with the former __experimentalDimensions name — it appears it was mostly there as a placeholder in code for future features.

What do you think?

Copy link

Choose a reason for hiding this comment

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

If it was a no-op, I think we're good to leave it as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @andrewserong and @ntsekouras for the explanation.

tl;dr
No BC concern.

Let me summarize the reasoning:

  • Yes, this __experimentalDimensions setting existed before 6.2.
  • It did nothing. No functionality was attached to it. (think of it as placeholder for the future, ie today)
  • If a developer is using it, there is no effect to them for removing it.

I agree then. It's not a BC concern to remove it. Thank you!


if ( $has_dimensions_support ) {
$block_type->attributes['style'] = array(
Expand All @@ -44,23 +43,39 @@ function wp_register_dimensions_support( $block_type ) {
* This will be applied to the block markup in the front-end.
*
* @since 5.9.0
* @since 6.2.0 Added `minHeight` support.
* @access private
*
* @param WP_Block_Type $block_type Block Type.
* @param array $block_attributes Block attributes.
* @return array Block dimensions CSS classes and inline styles.
*/
function wp_apply_dimensions_support( $block_type, $block_attributes ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
if ( wp_should_skip_block_supports_serialization( $block_type, '__experimentalDimensions' ) ) {
if ( wp_should_skip_block_supports_serialization( $block_type, 'dimensions' ) ) {
hellofromtonya marked this conversation as resolved.
Show resolved Hide resolved
return array();
}

$styles = array();
$attributes = array();

// Height support to be added in near future.
// Width support to be added in near future.

return empty( $styles ) ? array() : array( 'style' => implode( ' ', $styles ) );
$has_min_height_support = block_has_support( $block_type, array( 'dimensions', 'minHeight' ), false );
$block_styles = isset( $block_attributes['style'] ) ? $block_attributes['style'] : null;

if ( ! $block_styles ) {
return $attributes;
}

$skip_min_height = wp_should_skip_block_supports_serialization( $block_type, 'dimensions', 'minHeight' );
$dimensions_block_styles = array();
$dimensions_block_styles['minHeight'] = $has_min_height_support && ! $skip_min_height ? _wp_array_get( $block_styles, array( 'dimensions', 'minHeight' ), null ) : null;
$styles = wp_style_engine_get_styles( array( 'dimensions' => $dimensions_block_styles ) );

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

return $attributes;
}

// Register the block support.
Expand Down
14 changes: 12 additions & 2 deletions src/wp-includes/class-wp-theme-json.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class WP_Theme_JSON {
* @since 6.1.0 Added the `border-*-color`, `border-*-width`, `border-*-style`,
* `--wp--style--root--padding-*`, and `box-shadow` properties,
* removed the `--wp--style--block-gap` property.
* @since 6.2.0 Added `outline-*` properties.
* @since 6.2.0 Added `outline-*`, and `min-height` properties.
*
* @var array
*/
Expand Down Expand Up @@ -231,6 +231,7 @@ class WP_Theme_JSON {
'margin-right' => array( 'spacing', 'margin', 'right' ),
'margin-bottom' => array( 'spacing', 'margin', 'bottom' ),
'margin-left' => array( 'spacing', 'margin', 'left' ),
'min-height' => array( 'dimensions', 'minHeight' ),
'outline-color' => array( 'outline', 'color' ),
'outline-offset' => array( 'outline', 'offset' ),
'outline-style' => array( 'outline', 'style' ),
Expand Down Expand Up @@ -293,6 +294,7 @@ class WP_Theme_JSON {
* and `typography`, and renamed others according to the new schema.
* @since 6.0.0 Added `color.defaultDuotone`.
* @since 6.1.0 Added `layout.definitions` and `useRootPaddingAwareAlignments`.
* @since 6.2.0 Added `dimensions.minHeight`.
* @var array
*/
const VALID_SETTINGS = array(
Expand All @@ -319,6 +321,9 @@ class WP_Theme_JSON {
'text' => null,
),
'custom' => null,
'dimensions' => array(
'minHeight' => null,
),
'layout' => array(
'contentSize' => null,
'definitions' => null,
Expand Down Expand Up @@ -358,7 +363,7 @@ class WP_Theme_JSON {
* @since 6.1.0 Added new side properties for `border`,
* added new property `shadow`,
* updated `blockGap` to be allowed at any level.
* @since 6.2.0 Added `outline` properties.
* @since 6.2.0 Added `outline`, and `minHeight` properties.
*
* @var array
*/
Expand All @@ -378,6 +383,9 @@ class WP_Theme_JSON {
'gradient' => null,
'text' => null,
),
'dimensions' => array(
'minHeight' => null,
),
'filter' => array(
'duotone' => null,
),
Expand Down Expand Up @@ -490,6 +498,7 @@ public static function get_element_class_name( $element ) {
* Options that settings.appearanceTools enables.
*
* @since 6.0.0
* @since 6.2.0 Added `dimensions.minHeight`.
* @var array
*/
const APPEARANCE_TOOLS_OPT_INS = array(
Expand All @@ -498,6 +507,7 @@ public static function get_element_class_name( $element ) {
array( 'border', 'style' ),
array( 'border', 'width' ),
array( 'color', 'link' ),
array( 'dimensions', 'minHeight' ),
array( 'spacing', 'blockGap' ),
array( 'spacing', 'margin' ),
array( 'spacing', 'padding' ),
Expand Down
11 changes: 11 additions & 0 deletions src/wp-includes/style-engine/class-wp-style-engine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ final class WP_Style_Engine {
),
),
),
'dimensions' => array(
'minHeight' => array(
'property_keys' => array(
'default' => 'min-height',
),
'path' => array( 'dimensions', 'minHeight' ),
'css_vars' => array(
'spacing' => '--wp--preset--spacing--$slug',
),
),
),
'spacing' => array(
'padding' => array(
'property_keys' => array(
Expand Down
103 changes: 103 additions & 0 deletions tests/phpunit/tests/block-supports/wpApplyDimensionsSupport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<?php

/**
* @group block-supports
*
* @covers ::wp_apply_dimensions_support
*/
class Tests_Block_Supports_wpApplyDimensionsSupport extends WP_UnitTestCase {
/**
* @var string|null
*/
private $test_block_name;

public function set_up() {
parent::set_up();
$this->test_block_name = null;
}

public function tear_down() {
unregister_block_type( $this->test_block_name );
$this->test_block_name = null;
parent::tear_down();
}

/**
* Tests that minimum height block support works as expected.
*
* @ticket 57582
*
* @covers ::wp_apply_dimensions_support
*
* @dataProvider data_minimum_height_block_support
*
* @param string $block_name The test block name to register.
* @param mixed $dimensions The dimensions block support settings.
* @param mixed $expected The expected results.
*/
public function test_minimum_height_block_support( $block_name, $dimensions, $expected ) {
$this->test_block_name = $block_name;
register_block_type(
$this->test_block_name,
array(
'api_version' => 2,
'attributes' => array(
'style' => array(
'type' => 'object',
),
),
'supports' => array(
'dimensions' => $dimensions,
),
)
);
$registry = WP_Block_Type_Registry::get_instance();
$block_type = $registry->get_registered( $this->test_block_name );
$block_attrs = array(
'style' => array(
'dimensions' => array(
'minHeight' => '50vh',
),
),
);

$actual = wp_apply_dimensions_support( $block_type, $block_attrs );

$this->assertSame( $expected, $actual );
}

/**
* Data provider.
*
* @return array
*/
public function data_minimum_height_block_support() {
return array(
'style is applied' => array(
'block_name' => 'test/dimensions-block-supports',
'dimensions' => array(
'minHeight' => true,
),
'expected' => array(
'style' => 'min-height:50vh;',
),
),
'style output is skipped when serialization is skipped' => array(
'block_name' => 'test/dimensions-with-skipped-serialization-block-supports',
'dimensions' => array(
'minHeight' => true,
'__experimentalSkipSerialization' => true,
),
'expected' => array(),
),
'style output is skipped when individual feature serialization is skipped' => array(
'block_name' => 'test/min-height-with-individual-skipped-serialization-block-supports',
'dimensions' => array(
'minHeight' => true,
'__experimentalSkipSerialization' => array( 'minHeight' ),
),
'expected' => array(),
),
);
}
}
15 changes: 15 additions & 0 deletions tests/phpunit/tests/style-engine/styleEngine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,21 @@ public function data_wp_style_engine_get_styles() {
),
),

'inline_valid_dimensions_style' => array(
'block_styles' => array(
'dimensions' => array(
'minHeight' => '50vh',
),
),
'options' => null,
'expected_output' => array(
'css' => 'min-height:50vh;',
'declarations' => array(
'min-height' => '50vh',
),
),
),

'inline_valid_typography_style' => array(
'block_styles' => array(
'typography' => array(
Expand Down
6 changes: 6 additions & 0 deletions tests/phpunit/tests/theme/wpThemeJson.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ public function test_get_settings_appearance_true_opts_in() {
'color' => array(
'link' => true,
),
'dimensions' => array(
'minHeight' => true,
),
'spacing' => array(
'blockGap' => false,
'margin' => true,
Expand All @@ -265,6 +268,9 @@ public function test_get_settings_appearance_true_opts_in() {
'color' => array(
'link' => true,
),
'dimensions' => array(
'minHeight' => true,
),
'spacing' => array(
'blockGap' => false,
'margin' => true,
Expand Down