-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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.
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
Reduce layout rule specificity and add compound classname #4623
Reduce layout rule specificity and add compound classname #4623
Conversation
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.
Changes look good, and match Gutenberg.
I'm seeing the wp-block-group-is-layout-constrained
classes on group blocks and the reduced specificity for margin
:where(body .is-layout-constrained) > :first-child:first-child {
margin-block-start: 0;
}
Just checking whether I should see any CSS rules attached to wp-block-group-is-layout-constrained
as per the description in WordPress/gutenberg#47952
So far I can't
@@ -466,7 +466,7 @@ public function test_get_stylesheet_renders_enabled_protected_properties() { | |||
) | |||
); | |||
|
|||
$expected = 'body { margin: 0; }.wp-site-blocks > .alignleft { float: left; margin-right: 2em; }.wp-site-blocks > .alignright { float: right; margin-left: 2em; }.wp-site-blocks > .aligncenter { justify-content: center; margin-left: auto; margin-right: auto; }.wp-site-blocks > * { margin-block-start: 0; margin-block-end: 0; }.wp-site-blocks > * + * { margin-block-start: 1em; }body { --wp--style--block-gap: 1em; }'; | |||
$expected = 'body { margin: 0; }.wp-site-blocks > .alignleft { float: left; margin-right: 2em; }.wp-site-blocks > .alignright { float: right; margin-left: 2em; }.wp-site-blocks > .aligncenter { justify-content: center; margin-left: auto; margin-right: auto; }:where(.wp-site-blocks) > * { margin-block-start: 1em; margin-block-end: 0; }:where(.wp-site-blocks) > :first-child:first-child { margin-block-start: 0; }:where(.wp-site-blocks) > :last-child:last-child { margin-block-end: 0; }body { --wp--style--block-gap: 1em; }'; |
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.
I think the test function annotation just needs a @ticket 58548
@@ -3863,7 +3863,7 @@ public function test_wp_filter_content_tags_does_not_lazy_load_first_image_in_bl | |||
$_wp_current_template_content = '<!-- wp:post-content /-->'; | |||
|
|||
$html = get_the_block_template_html(); | |||
$this->assertSame( '<div class="wp-site-blocks"><div class="entry-content wp-block-post-content is-layout-flow">' . $expected_content . '</div></div>', $html ); | |||
$this->assertSame( '<div class="wp-site-blocks"><div class="entry-content wp-block-post-content is-layout-flow wp-block-post-content-is-layout-flow">' . $expected_content . '</div></div>', $html ); |
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.
Same here @ticket 58548
@@ -1317,7 +1317,7 @@ protected function get_layout_styles( $block_metadata ) { | |||
$spacing_rule['selector'] | |||
); | |||
} else { | |||
$format = static::ROOT_BLOCK_SELECTOR === $selector ? '%s .%s%s' : '%s.%s%s'; | |||
$format = static::ROOT_BLOCK_SELECTOR === $selector ? ':where(%s .%s) %s' : '%s-%s%s'; |
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.
Would something like @since 6.3.0 Reduces specificity of layout margin rules.
be accurate for the get_layout_styles
annotation?
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.
Yes, thanks!
@@ -474,6 +474,10 @@ function wp_render_layout_support_flag( $block_content, $block ) { | |||
} | |||
} | |||
|
|||
// Add combined layout and block classname for global styles to hook onto. | |||
$block_name = explode( '/', $block['blockName'] ); |
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.
For wp_render_layout_support_flag
annotation:
@since 6.3.0 Adds compound class to layout wrapper for global spacing styles.
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.
Aha, thanks!
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.
This is testing well for me, and is consistent with the compound classname approach that's been in Gutenberg for a while now 👍
✅ Compound classname is output at the individual block level
✅ Block-level spacing rules applied in global styles are attached to the compound classname
✅ Reduced specificity rules appear to be output correctly at the root and block-level
LGTM! ✨
Trac ticket: https://core.trac.wordpress.org/ticket/58548
This PR backports the PHP changes from WordPress/gutenberg#47858 and
WordPress/gutenberg#47952.
To test the changes, build PR branch locally. and make sure a block theme such as Twenty Twenty Three is enabled. Until the npm packages are updated, these changes will only be testable on the front end.
wp-block-group-is-layout-constrained
class, in addition to the previously existing classes.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.