#58586 closed task (blessed) (fixed)
Block.json: Refactor and stabilize selectors API
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.3 | Priority: | normal |
| Severity: | normal | Version: | 6.3 |
| Component: | Editor | Keywords: | has-patch has-unit-tests gutenberg-merge |
| Focuses: | Cc: |
Description (last modified by )
This ticket tracks the backporting of PHP files for the following Gutenberg changes:
- โhttps://github.com/WordPress/gutenberg/pull/46496
- โhttps://github.com/WordPress/gutenberg/pull/46496
The selectors API is stabilized as part of this refactor.
cc @aaronrobertshaw
Change History (22)
This ticket was mentioned in โPR #4655 on โWordPress/wordpress-develop by โ@ramonopoly.
2 years ago
#1
โ@ramonopoly commented on โPR #4655:
2 years ago
#3
@ramonjd
A pretty rough start. I wasn't sure whether the following change to blocks.php had to be ported.
โ@ramonopoly commented on โPR #4655:
2 years ago
#4
Didn't edit unit tests yet. Failures are expected ๐
โ@aaronrobertshaw commented on โPR #4655:
2 years ago
#5
A pretty rough start. I wasn't sure whether the following change to blocks.php had to be ported.
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.
Didn't edit unit tests yet. Failures are expected ๐
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 image/block.json not being updated yet.
โ@ramonopoly commented on โPR #4655:
2 years ago
#6
The theme.json tests look to be failing due to the image/block.json not being updated yet.
๐ค I think we should revert that test until the block library package is backported. That should happen early next week.
Most of the test failures will relate to the โremoval of the "editor-only" selectors from the API.
I've fixed these for now.
I noticed that โhttps://github.com/WordPress/wordpress-develop/pull/4619 is backporting the duotone changes only, so it looks like we'll have to port over the rest of the changes from
- โhttps://github.com/WordPress/gutenberg/pull/49423
- โhttps://github.com/WordPress/gutenberg/pull/49393
I've also removed the following test from Tests_Theme_WpGetBlockCssSelector, which was failing and I couldn't work out if it was required since the API was stabilized or because there are dependencies in โhttps://github.com/WordPress/wordpress-develop/pull/4619.
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...
โ@ramonopoly commented on โPR #4655:
2 years ago
#7
I'm getting โ1 failure for Tests_Theme_wpThemeJson::test_get_stylesheet where the image block class is being output twice for duotone: .wp-block-image .wp-block-image img, .wp-block-image .wp-block-image .components-placeholder{filter: var(--wp--preset--duotone--custom-duotone);}.
Is that also dependent on block.json or the duotone backport changes @aaronrobertshaw ?
โ@aaronrobertshaw commented on โPR #4655:
2 years ago
#8
I'm getting โ1 failure
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.
โ@aaronrobertshaw commented on โPR #4655:
2 years ago
#9
I'm getting one failure
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.
-
src/wp-includes/class-wp-theme-json.php
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 b class WP_Theme_JSON {ย 2389 2389 2390 2390 // 3. Generate and append the rules that use the duotone selector. 2391 2391 if ( isset( $block_metadata['duotone'] ) && ! empty( $declarations_duotone ) ) { 2392 ย $selector_duotone = static::scope_selector( $block_metadata['selector'], $block_metadata['duotone'] ); 2393 ย $block_rules .= static::to_ruleset( $selector_duotone, $declarations_duotone ); ย 2392 $block_rules .= static::to_ruleset( $block_metadata['duotone'], $declarations_duotone ); 2394 2393 } 2395 2394 2396 2395 // 4. Generate Layout block gap styles.
โ@ramonopoly commented on โPR #4655:
2 years ago
#10
Thanks for doing the heavy thinking here @aaronrobertshaw I'll get these in tomorrow ๐
โ@ramonopoly commented on โPR #4655:
2 years ago
#11
@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 โhttps://github.com/WordPress/wordpress-develop/pull/4619?
โ@onemaggie commented on โPR #4655:
2 years ago
#12
@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
โ@aaronrobertshaw commented on โPR #4655:
2 years ago
#13
Suggesting that one (maybe this PR) gets merged first and then we rebase โhttps://github.com/WordPress/wordpress-develop/pull/4619?
Merging this PR first works for me, thanks ๐
I'm happy to help out if I can on the duotone updates as well.
โ@ramonopoly commented on โPR #4655:
2 years ago
#14
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? โhttps://github.com/WordPress/gutenberg/pull/50678 Not sure.
โ@aaronrobertshaw commented on โPR #4655:
2 years ago
#15
I think this one seems to have fallen through the cracks? โhttps://github.com/WordPress/gutenberg/pull/50678 Not sure.
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 โhttps://github.com/WordPress/wordpress-develop/pull/4619.
cc/ @MaggieCabrera
#16
@
2 years ago
- Type changed from enhancement to task (blessed)
Converting to a task as this can ship after beta 1 is released.
#17
@
2 years ago
- Owner set to isabel_brison
- Resolution set to fixed
- Status changed from new to closed
In 56058:
โ@isabel_brison commented on โPR #4655:
2 years ago
#18
committed in r56058 / โ8b9d8d8.
#19
@
2 years ago
@isabel_brison the multiline comment indention is wrong in src/wp-includes/block-supports/settings.php line no 99. โhttps://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-supports/settings.php#L99-L102
This ticket was mentioned in โPR #4803 on โWordPress/wordpress-develop by โ@isabel_brison.
2 years ago
#20
Trac ticket: https://core.trac.wordpress.org/ticket/58586
Fix comment indentation issue noted here
โ@isabel_brison commented on โPR #4803:
2 years ago
#22
Committed in r56146 / โ21c74f1.
This is a backport PR for WordPress 6.3 that includes the following PHP Gutenberg changes:
Trac ticket: https://core.trac.wordpress.org/ticket/58586
cc @aaronrobertshaw