Block.json: Refactor and stabilize selectors API #4655
Conversation
|
A pretty rough start. I wasn't sure whether the following change to blocks.php had to be ported. |
ramonjd
changed the title
|
Didn't edit unit tests yet. Failures are expected 😄 |
I can confirm, the changes referred to are expected to remain Gutenberg-only until 6.3 is the minimum supported version. That filter is there to ensure the selectors configuration from block.json metadata is honoured.
Most of the test failures will relate to the removal of the "editor-only" selectors from the API. The theme.json tests look to be failing due to the I'll have some bandwidth tomorrow to pick this up or help out. Thanks for getting this started @ramonjd 🙇 |
ramonjd
changed the title
ramonjd
force-pushed
the
update/block-supports-selectors-api
branch
from
413cd3e to
9fa4f93
Compare
🤔 I think we should revert that test until the block library package is backported (done). That should happen early next week.
I've fixed these for now. I noticed that #4619 is backporting the duotone changes only, so it looks like we'll have to port over the rest of the changes from
I've also removed the following test from public function test_get_duotone_selector_via_experimental_property() {
$block_type = self::register_test_block(
'test/experimental-duotone-selector',
null,
array(
'color' => array(
'__experimentalDuotone' => '.experimental-duotone',
),
)
);
$selector = wp_get_block_css_selector( $block_type, 'filters.duotone' );
$this->assertEquals( '.experimental-duotone', $selector );
}More to work to come on this one... |
|
I'm getting 1 failure for Is that also dependent on block.json or the duotone backport changes @aaronrobertshaw ? |
My expectation was that this test would be failing due to the image block.json needing to be updated. I'll go digging and see what I can find and see what else might need backporting. |
There's a duotone-related change that we'll need to include in this backport as well to get the tests passing. Unfortunately, I don't have permission to push to your fork or this PR branch. Instead, below is a small diff that you can apply. diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php
index 97f6d12511..92abe30f1c 100644
--- a/src/wp-includes/class-wp-theme-json.php
+++ b/src/wp-includes/class-wp-theme-json.php
@@ -2389,8 +2389,7 @@ class WP_Theme_JSON {
// 3. Generate and append the rules that use the duotone selector.
if ( isset( $block_metadata['duotone'] ) && ! empty( $declarations_duotone ) ) {
- $selector_duotone = static::scope_selector( $block_metadata['selector'], $block_metadata['duotone'] );
- $block_rules .= static::to_ruleset( $selector_duotone, $declarations_duotone );
+ $block_rules .= static::to_ruleset( $block_metadata['duotone'], $declarations_duotone );
}
// 4. Generate Layout block gap styles.
|
There was a problem hiding this comment.
I've gone over the related Gutenberg Block Selector API PRs.
- Block.json: Refactor and stabilize selectors API gutenberg#46496
- Selectors API: Fix for global styles hook, style variations, and duotone gutenberg#49393
- Selectors API: Make duotone selectors fallback and be scoped gutenberg#49423
- Update preset styles to use Selectors API gutenberg#49427
In addition to the change noted in my earlier comment, there are a few changes to the tests that require backporting still.
A patch for the proposed updates can be found below:
Proposed Updates
diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php
index 97f6d12511..92abe30f1c 100644
--- a/src/wp-includes/class-wp-theme-json.php
+++ b/src/wp-includes/class-wp-theme-json.php
@@ -2389,8 +2389,7 @@ class WP_Theme_JSON {
// 3. Generate and append the rules that use the duotone selector.
if ( isset( $block_metadata['duotone'] ) && ! empty( $declarations_duotone ) ) {
- $selector_duotone = static::scope_selector( $block_metadata['selector'], $block_metadata['duotone'] );
- $block_rules .= static::to_ruleset( $selector_duotone, $declarations_duotone );
+ $block_rules .= static::to_ruleset( $block_metadata['duotone'], $declarations_duotone );
}
// 4. Generate Layout block gap styles.
diff --git a/tests/phpunit/tests/theme/wpGetBlockCssSelector.php b/tests/phpunit/tests/theme/wpGetBlockCssSelector.php
index 1a58943fc7..ac7e7629fb 100644
--- a/tests/phpunit/tests/theme/wpGetBlockCssSelector.php
+++ b/tests/phpunit/tests/theme/wpGetBlockCssSelector.php
@@ -2,6 +2,8 @@
/**
* Tests wp_get_block_css_selector().
*
+ * @since 6.3.0
+ *
* @group themes
*
* @covers ::wp_get_block_css_selector
@@ -18,30 +20,26 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
public function tear_down() {
unregister_block_type( $this->test_block_name );
$this->test_block_name = null;
- set_current_screen( '' );
parent::tear_down();
}
- private function register_test_block( $name, $selectors = null, $supports = null, $editor_selectors = null ) {
+ private function register_test_block( $name, $selectors = null, $supports = null ) {
$this->test_block_name = $name;
return register_block_type(
$this->test_block_name,
array(
- 'api_version' => 2,
- 'attributes' => array(),
- 'selectors' => $selectors,
- 'editor_selectors' => $editor_selectors,
- 'supports' => $supports,
+ 'api_version' => 2,
+ 'attributes' => array(),
+ 'selectors' => $selectors,
+ 'supports' => $supports,
)
);
}
- private function set_screen_to_block_editor() {
- set_current_screen( 'edit-post' );
- get_current_screen()->is_block_editor( true );
- }
-
+ /**
+ * @ticket 58586
+ */
public function test_get_root_selector_via_selectors_api() {
$block_type = self::register_test_block(
'test/block-with-selectors',
@@ -52,6 +50,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.wp-custom-block-class', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_get_root_selector_via_experimental_property() {
$block_type = self::register_test_block(
'test/block-without-selectors',
@@ -63,6 +64,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.experimental-selector', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_default_root_selector_generation_for_core_block() {
$block_type = self::register_test_block(
'core/without-selectors-or-supports',
@@ -74,6 +78,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.wp-block-without-selectors-or-supports', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_default_root_selector_generation() {
$block_type = self::register_test_block(
'test/without-selectors-or-supports',
@@ -85,30 +92,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.wp-block-test-without-selectors-or-supports', $selector );
}
- public function test_get_duotone_selector_via_selectors_api() {
- $block_type = self::register_test_block(
- 'test/duotone-selector',
- array(
- 'filters' => array( 'duotone' => '.duotone-selector' ),
- ),
- null
- );
-
- $selector = wp_get_block_css_selector( $block_type, array( 'filters', 'duotone' ) );
- $this->assertEquals( '.duotone-selector', $selector );
- }
-
- public function test_no_duotone_selector_set() {
- $block_type = self::register_test_block(
- 'test/null-duotone-selector',
- null,
- null
- );
-
- $selector = wp_get_block_css_selector( $block_type, 'filters.duotone' );
- $this->assertEquals( null, $selector );
- }
-
+ /**
+ * @ticket 58586
+ */
public function test_get_feature_selector_via_selectors_api() {
$block_type = self::register_test_block(
'test/feature-selector',
@@ -120,6 +106,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.typography', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_get_feature_selector_via_selectors_api_shorthand_property() {
$block_type = self::register_test_block(
'test/shorthand-feature-selector',
@@ -131,6 +120,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.typography', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_no_feature_level_selector_via_selectors_api() {
$block_type = self::register_test_block(
'test/null-feature-selector',
@@ -142,6 +134,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( null, $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_fallback_feature_level_selector_via_selectors_api_to_generated_class() {
$block_type = self::register_test_block(
'test/fallback-feature-selector',
@@ -153,7 +148,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.wp-block-test-fallback-feature-selector', $selector );
}
-
+ /**
+ * @ticket 58586
+ */
public function test_fallback_feature_level_selector_via_selectors_api() {
$block_type = self::register_test_block(
'test/fallback-feature-selector',
@@ -165,6 +162,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.fallback-root-selector', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_get_feature_selector_via_experimental_property() {
$block_type = self::register_test_block(
'test/experimental-feature-selector',
@@ -180,6 +180,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.wp-block-test-experimental-feature-selector .experimental-typography', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_fallback_feature_selector_via_experimental_property() {
$block_type = self::register_test_block(
'test/fallback-feature-selector',
@@ -191,6 +194,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.wp-block-test-fallback-feature-selector', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_no_feature_selector_via_experimental_property() {
$block_type = self::register_test_block(
'test/null-experimental-feature-selector',
@@ -202,6 +208,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( null, $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_get_subfeature_selector_via_selectors_api() {
$block_type = self::register_test_block(
'test/subfeature-selector',
@@ -221,6 +230,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.root .typography .text-decoration', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_fallback_subfeature_selector_via_selectors_api() {
$block_type = self::register_test_block(
'test/subfeature-selector',
@@ -239,6 +251,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.root .typography', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_no_subfeature_level_selector_via_selectors_api() {
$block_type = self::register_test_block(
'test/null-subfeature-selector',
@@ -250,6 +265,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( null, $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_fallback_subfeature_selector_via_experimental_property() {
$block_type = self::register_test_block(
'test/fallback-subfeature-selector',
@@ -265,6 +283,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.wp-block-test-fallback-subfeature-selector', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_no_subfeature_selector_via_experimental_property() {
$block_type = self::register_test_block(
'test/null-experimental-subfeature-selector',
@@ -279,6 +300,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( null, $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_empty_target_returns_null() {
$block_type = self::register_test_block(
'test/null-experimental-subfeature-selector',
@@ -293,6 +317,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( null, $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_string_targets_for_features() {
$block_type = self::register_test_block(
'test/target-types-for-features',
@@ -307,6 +334,9 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$this->assertEquals( '.found', $selector );
}
+ /**
+ * @ticket 58586
+ */
public function test_string_targets_for_subfeatures() {
$block_type = self::register_test_block(
'test/target-types-for-features',
@@ -322,107 +352,5 @@ class Tests_Theme_WpGetBlockCssSelector extends WP_Theme_UnitTestCase {
$selector = wp_get_block_css_selector( $block_type, array( 'typography', 'fontSize' ) );
$this->assertEquals( '.found', $selector );
}
-
- public function test_editor_only_root_selector() {
- self::set_screen_to_block_editor();
-
- $block_type = self::register_test_block(
- 'test/editor-only-selectors',
- array( 'root' => '.wp-custom-block-class' ),
- null,
- array( 'root' => '.wp-custom-block-class' )
- );
-
- $selector = wp_get_block_css_selector( $block_type, 'root' );
- $this->assertEquals( '.wp-custom-block-class', $selector );
- }
-
- public function test_editor_only_duotone_selector() {
- self::set_screen_to_block_editor();
-
- $block_type = self::register_test_block(
- 'test/editor-duotone-selector',
- array(
- 'filters' => array( 'duotone' => '.duotone-selector' ),
- ),
- null,
- array(
- 'filters' => array( 'duotone' => '.duotone-selector' ),
- )
- );
-
- $selector = wp_get_block_css_selector( $block_type, 'filters.duotone' );
- $this->assertEquals( '.duotone-selector', $selector );
- }
-
- public function test_editor_only_feature_selector() {
- self::set_screen_to_block_editor();
-
- $block_type = self::register_test_block(
- 'test/editor-feature-selector',
- array( 'typography' => array( 'root' => '.typography' ) ),
- null,
- array( 'typography' => array( 'root' => '.typography' ) )
- );
-
- $selector = wp_get_block_css_selector( $block_type, 'typography' );
- $this->assertEquals( '.typography', $selector );
- }
-
- public function test_editor_only_feature_selector_shorthand() {
- self::set_screen_to_block_editor();
-
- $block_type = self::register_test_block(
- 'test/editor-feature-selector',
- array( 'typography' => '.typography' ),
- null,
- array( 'typography' => '.typography' )
- );
-
- $selector = wp_get_block_css_selector( $block_type, 'typography' );
- $this->assertEquals( '.typography', $selector );
- }
-
- public function test_editor_only_subfeature_selector() {
- self::set_screen_to_block_editor();
-
- $block_type = self::register_test_block(
- 'test/editor-subfeature-selector',
- array( 'typography' => array( 'fontSize' => '.font-size' ) ),
- null,
- array( 'typography' => array( 'fontSize' => '.font-size' ) )
- );
-
- $selector = wp_get_block_css_selector( $block_type, 'typography.fontSize' );
- $this->assertEquals( '.font-size', $selector );
- }
-
- public function test_non_editor_subfeature_does_not_fall_back_to_editor_only_feature_selector() {
- self::set_screen_to_block_editor();
-
- $block_type = self::register_test_block(
- 'test/editor-subfeature-selector',
- array( 'typography' => array( 'fontSize' => '.font-size' ) ),
- null,
- array( 'typography' => '.font-size' )
- );
-
- $selector = wp_get_block_css_selector( $block_type, 'typography.fontSize', true );
- $this->assertEquals( '.font-size', $selector );
- }
-
- public function test_unspecified_subfeature_falls_back_to_editor_only_feature_selector() {
- self::set_screen_to_block_editor();
-
- $block_type = self::register_test_block(
- 'test/editor-subfeature-selector',
- array( 'typography' => '.typography' ),
- null,
- array( 'typography' => '.typography' )
- );
-
- $selector = wp_get_block_css_selector( $block_type, 'typography.fontSize', true );
- $this->assertEquals( '.typography', $selector );
- }
}
The following is a breakdown per-commit.
✅ class-wp-theme-json-gutenberg.php - Backported
🚫 blocks.php - N/A - Changes are plugin-only
🚫 lib/load.php - N/A
❓️ phpunit/class-wp-theme-json-test.php - New tests haven't been backported yet although they require registered blocks using both the old experimental selector and the new selectors API. This block registration was handled in Gutenberg via the phpunit bootstrap. I’m not sure how best to approach it here for core though.
✅ phpunit/class-wp-get-block-css-selectors-test.php - Backported (updates in later PRs haven't been applied to backport though).
🚫 class-wp-duotone-gutenberg.php - N/A - To be backported in duotone-specific PR (see https://github.com/WordPress/wordpress-develop/pull/4619)
✅ class-wp-theme-json-gutenberg.php - Backported
🚫 blocks.php - N/A - The filter added here should be plugin only
✅ get-global-styles-and-settings.php - Backported
❓️ phpunit/class-wp-get-block-css-selectors-test.php - We need to backport the changes here to remove the editor-only settings tests and update the key duotone selectors are found under.
✅ phpunit/class-wp-theme-json-test.php - Already backported elsewhere
🚫 class-wp-duotone-gutenberg.php - N/A - To be backported in duotone-specific PR (see https://github.com/WordPress/wordpress-develop/pull/4619)
✅ class-wp-theme-json-gutenberg.php - Backported
✅ get-global-styles-and-settings.php - Backported
❓️ phpunit/class-wp-get-block-css-selectors-test.php - The duotone test removals in this PR aren't reflected in the backport
✅ phpunit/class-wp-theme-json-test.php - Typo correction backported
✅ block-support/settings.php - Backported in full
|
Thanks for doing the heavy thinking here @aaronrobertshaw I'll get these in tomorrow 👍 |
ramonjd
force-pushed
the
update/block-supports-selectors-api
branch
from
5530348 to
b728754
Compare
There was a problem hiding this comment.
Thank you for wrangling those updates @ramonjd 🙇
I've given this PR another once over and it covers all the updates from the relevant block selector PRs and is consistent with the current state of Gutenberg trunk.
The failing e2es look unrelated to me. So code wise I think this is pretty close. It would be good to get final approval from someone more familiar with applying core standards though.
ramonjd
changed the title
|
@MaggieCabrera @aaronrobertshaw Just checking, as there might be some overlap between this PR and Suggesting that one (maybe this PR) gets merged first and then we rebase #4619? |
I think that's the right order, yes |
ramonjd
force-pushed
the
update/block-supports-selectors-api
branch
from
b728754 to
0c09d6c
Compare
Merging this PR first works for me, thanks 🙂 I'm happy to help out if I can on the duotone updates as well. |
That'd be great, thank you! I think this one seems to have fallen through the cracks? WordPress/gutenberg#50678 Not sure. |
ramonjd
force-pushed
the
update/block-supports-selectors-api
branch
from
0c09d6c to
9fcf6d0
Compare
Thanks for flagging that @ramonjd. It does appear to have been missed. It is related to duotone filters rather than the block selectors API so I think belongs in #4619. cc/ @MaggieCabrera |
Reverting changes to Tests_Theme_wpThemeJson that will need to be reinstated after WordPress#4619 merges
…r the Tests_Theme_WpGetBlockCssSelector block tests)
tellthemachines
force-pushed
the
update/block-supports-selectors-api
branch
from
9fcf6d0 to
045d224
Compare
This is a backport PR for WordPress 6.3 that includes the following PHP Gutenberg changes:
Related (original code introduced in 6.2):
Trac ticket: https://core.trac.wordpress.org/ticket/58586
cc @aaronrobertshaw
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.