Backport min-height block support feature #3940
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| <?php | ||
|
There was a problem hiding this comment. The file and the class need renaming for Core's Test coding standards and consistency.
Will push a commit shortly. There was a problem hiding this comment. Whoops forgot to share the naming convention: For the file: For the test: See handbook here https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#test-classes for more information. Note: In Gutenberg, more work is needed in the linting before its tests can be converted into Core's coding standards. There was a problem hiding this comment. @hellofromtonya According to the handbook, it says the test class should be in this format:
What I'm not sure about though, is whether
There was a problem hiding this comment. @costdev The other test in the group has it as There was a problem hiding this comment. Regarding There was a problem hiding this comment. Oh wait I do see what you mean though about capitalizing the first letter of the function's name. Yup that needs changing. Good catch. Will do that in the commit. |
||
|
|
||
| /** | ||
| * @group block-supports | ||
| * | ||
| * @covers ::wp_apply_dimensions_support | ||
| */ | ||
|
|
||
| class WP_Block_Supports_Dimensions_Test 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; | ||
hellofromtonya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| parent::tear_down(); | ||
| } | ||
|
|
||
| public function test_dimensions_style_is_applied() { | ||
hellofromtonya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| $this->test_block_name = 'test/dimensions-style-is-applied'; | ||
| register_block_type( | ||
| $this->test_block_name, | ||
| array( | ||
| 'api_version' => 2, | ||
| 'attributes' => array( | ||
| 'style' => array( | ||
| 'type' => 'object', | ||
| ), | ||
| ), | ||
| 'supports' => array( | ||
| 'dimensions' => array( | ||
| 'minHeight' => true, | ||
| ), | ||
| ), | ||
| ) | ||
| ); | ||
| $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 ); | ||
| $expected = array( | ||
| 'style' => 'min-height:50vh;', | ||
| ); | ||
|
|
||
| $this->assertSame( $expected, $actual ); | ||
| } | ||
|
|
||
| public function test_dimensions_with_skipped_serialization_block_supports() { | ||
| $this->test_block_name = 'test/dimensions-with-skipped-serialization-block-supports'; | ||
| register_block_type( | ||
| $this->test_block_name, | ||
| array( | ||
| 'api_version' => 2, | ||
| 'attributes' => array( | ||
| 'style' => array( | ||
| 'type' => 'object', | ||
| ), | ||
| ), | ||
| 'supports' => array( | ||
| 'dimensions' => array( | ||
| 'minHeight' => true, | ||
| '__experimentalSkipSerialization' => true, | ||
| ), | ||
| ), | ||
| ) | ||
| ); | ||
| $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 ); | ||
| $expected = array(); | ||
|
|
||
| $this->assertSame( $expected, $actual ); | ||
| } | ||
|
|
||
| public function test_min_height_with_individual_skipped_serialization_block_supports() { | ||
| $this->test_block_name = 'test/min-height-with-individual-skipped-serialization-block-supports'; | ||
| register_block_type( | ||
| $this->test_block_name, | ||
| array( | ||
| 'api_version' => 2, | ||
| 'attributes' => array( | ||
| 'style' => array( | ||
| 'type' => 'object', | ||
| ), | ||
| ), | ||
| 'supports' => array( | ||
| 'dimensions' => array( | ||
| 'minHeight' => true, | ||
| '__experimentalSkipSerialization' => array( 'minHeight' ), | ||
| ), | ||
| ), | ||
| ) | ||
| ); | ||
| $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 ); | ||
|
|
||
| /* | ||
| * There is currently only one dimensions property available, | ||
| * so the expected result is an empty array. This test exists | ||
| * so that as new properties are added, this test can be expanded | ||
| * to check that skipping individual serialization works as expected. | ||
| */ | ||
| $expected = array(); | ||
|
|
||
| $this->assertSame( $expected, $actual ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
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:
__experimentalDimensionshttps://wpdirectory.net/search/01GR1JTT957N4BM1FAZAHPCQ1C__experimentalDimensionshttps://wpdirectory.net/search/01GR1JX1E9HM3BQQQ0GJ457B68So I'm wondering:
'__experimentalDimensions'previously exposed and used?'__experimentalDimensions'to upgrade blocks using this setting?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One way to mitigate:
||.'dimensions'instead.There was a problem hiding this comment.
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. Becausemin-heightis 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__experimentalDimensionsname — it appears it was mostly there as a placeholder in code for future features.What do you think?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
__experimentalDimensionssetting existed before 6.2.I agree then. It's not a BC concern to remove it. Thank you!