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.

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

Try reducing specificity of global styles block selector. #57841

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
0e3ee28
Try reducing specificity of global styles block selector.
tellthemachines Jan 15, 2024
a3ebd94
Apply in editor
tellthemachines Jan 15, 2024
9bb710a
Remove obsolete override
tellthemachines Jan 15, 2024
5741fd2
zero specificity for pesky button elements
tellthemachines Jan 17, 2024
3ffa0b7
Lower specificity of feature and compound selectors.
tellthemachines Jan 19, 2024
0265d3a
Reduce specificity of layout selectors
tellthemachines Jan 19, 2024
da01237
Lower specificity of base layout styles
tellthemachines Jan 22, 2024
c818e92
Fix JS unit tests
aaronrobertshaw Jan 23, 2024
872c324
Fix copy and paste error
aaronrobertshaw Jan 23, 2024
e7a7b79
Update layout support tests
aaronrobertshaw Jan 24, 2024
0c049a1
Change style loading order
tellthemachines Jan 24, 2024
0fee1af
Make sure global styles only load in admin
tellthemachines Jan 25, 2024
20dadf2
Remove redundant filter and duplicate global styles.
tellthemachines Jan 31, 2024
e5e19e7
Fix style loading order better.
tellthemachines Feb 2, 2024
b070ef2
Reduce specificity of Cover styles
tellthemachines Feb 5, 2024
b3ff00f
Reinstate theme.json test updates
aaronrobertshaw Feb 6, 2024
9916e9d
Reduce specificity of margin and other styles in core block theme files
tellthemachines Feb 6, 2024
8ae139c
Fix invalid selector
tellthemachines Feb 12, 2024
405c6f9
bump specificity of layout styles back up
tellthemachines Feb 20, 2024
a5c48bd
Bump up specificity of layout rule in global styles
tellthemachines Feb 20, 2024
3817e5e
Fix theme json unit tests again
aaronrobertshaw Feb 20, 2024
2fd08c2
Fix global styles tests
aaronrobertshaw Feb 20, 2024
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
14 changes: 7 additions & 7 deletions lib/block-supports/layout.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ function gutenberg_get_layout_definitions() {
),
'spacingStyles' => array(
array(
'selector' => ' > :first-child:first-child',
'selector' => ' > :first-child',
'rules' => array(
'margin-block-start' => '0',
),
),
array(
'selector' => ' > :last-child:last-child',
'selector' => ' > :last-child',
'rules' => array(
'margin-block-end' => '0',
),
Expand Down Expand Up @@ -112,13 +112,13 @@ function gutenberg_get_layout_definitions() {
),
'spacingStyles' => array(
array(
'selector' => ' > :first-child:first-child',
'selector' => ' > :first-child',
'rules' => array(
'margin-block-start' => '0',
),
),
array(
'selector' => ' > :last-child:last-child',
'selector' => ' > :last-child',
'rules' => array(
'margin-block-end' => '0',
),
Expand Down Expand Up @@ -248,7 +248,7 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
),
),
array(
'selector' => "$selector$selector > * + *",
'selector' => "$selector > * + *",
'declarations' => array(
'margin-block-start' => $gap_value,
'margin-block-end' => '0',
Expand Down Expand Up @@ -357,7 +357,7 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
),
),
array(
'selector' => "$selector$selector > * + *",
'selector' => "$selector > * + *",
'declarations' => array(
'margin-block-start' => $gap_value,
'margin-block-end' => '0',
Expand Down Expand Up @@ -716,7 +716,7 @@ function gutenberg_render_layout_support_flag( $block_content, $block ) {
$has_block_gap_support = isset( $block_gap );

$style = gutenberg_get_layout_style(
".$container_class.$container_class",
".$container_class",
$used_layout,
$has_block_gap_support,
$gap_value,
Expand Down
12 changes: 6 additions & 6 deletions lib/class-wp-theme-json-gutenberg.php
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ class WP_Theme_JSON_Gutenberg {
* @var string[]
*/
const ELEMENTS = array(
'link' => 'a:where(:not(.wp-element-button))', // The `where` is needed to lower the specificity.
'link' => 'a:not(.wp-element-button)',
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a few other places the elements selectors might need updating as well as here if we are actually wanting to increase this selectors specificity from 0-0-1 to 0-1-1 e.g. the elements block support and the global styles output constant.

Given the spirit of this PR is to reduce specificity can we omit this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I might be a bit turned around jumping between PRs, I take it this change was because at least in the theme.json class element selectors are getting wrapped in :where?

As these selectors may be referenced elsewhere, is there a benefit to keeping the a:where(:not(.wp-element-button)) selector for links so it is consistent with the elements block support?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure! Theoretically it would be nice if the element selectors didn't make any specificity assumptions, and we just manipulated the specificity as needed when using them in different contexts.

Relatedly, I think I found a bug in the link selector on this branch: the blocks-specific link selector no longer has the :not bit attached. Looking into it now!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh turns out that's only recently been fixed on trunk, so rebasing should fix it. Might not be worth it if I'm going to split this into multiple PRs though.

Copy link
Contributor

Choose a reason for hiding this comment

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

For posterity, the PR fixing it on trunk was #59114.

Agreed, it's probably not work worrying about. On the splitting up front, I have a few other PRs based on this branch so my only question is do you see the different split parts as being pretty stable at this stage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope so, but this PR isn't getting a huge amount of feedback or testing so far, so part of my reason to split it up is hope that smaller PRs might be shippable faster. Stability is hard to predict without feedback! It works fine according to my testing, but I'm likely overlooking something 😅

'heading' => 'h1, h2, h3, h4, h5, h6',
'h1' => 'h1',
'h2' => 'h2',
Expand Down Expand Up @@ -1483,7 +1483,7 @@ protected function get_layout_styles( $block_metadata ) {
$spacing_rule['selector']
);
} else {
$format = static::ROOT_BLOCK_SELECTOR === $selector ? ':where(%s .%s) %s' : '%s-%s%s';
$format = static::ROOT_BLOCK_SELECTOR === $selector ? ':where(%s .%s) %s' : ':where(%s-%s) %s';
$layout_selector = sprintf(
$format,
$selector,
Expand Down Expand Up @@ -2573,7 +2573,7 @@ static function ( $pseudo_selector ) use ( $selector ) {
}

// 2. Generate and append the rules that use the general selector.
$block_rules .= static::to_ruleset( $selector, $declarations );
$block_rules .= static::to_ruleset( ":where($selector)", $declarations );

// 3. Generate and append the rules that use the duotone selector.
if ( isset( $block_metadata['duotone'] ) && ! empty( $declarations_duotone ) ) {
Expand All @@ -2590,7 +2590,7 @@ static function ( $pseudo_selector ) use ( $selector ) {

// 5. Generate and append the feature level rulesets.
foreach ( $feature_declarations as $feature_selector => $individual_feature_declarations ) {
$block_rules .= static::to_ruleset( $feature_selector, $individual_feature_declarations );
$block_rules .= static::to_ruleset( ":where($feature_selector)", $individual_feature_declarations );
}

// 6. Generate and append the style variation rulesets.
Expand Down Expand Up @@ -2666,8 +2666,8 @@ public function get_root_layout_rules( $selector, $block_metadata ) {
if ( $has_block_gap_support ) {
$block_gap_value = static::get_property_value( $this->theme_json, array( 'styles', 'spacing', 'blockGap' ) );
$css .= ":where(.wp-site-blocks) > * { margin-block-start: $block_gap_value; margin-block-end: 0; }";
$css .= ':where(.wp-site-blocks) > :first-child:first-child { margin-block-start: 0; }';
$css .= ':where(.wp-site-blocks) > :last-child:last-child { margin-block-end: 0; }';
$css .= ':where(.wp-site-blocks) > :first-child { margin-block-start: 0; }';
$css .= ':where(.wp-site-blocks) > :last-child { margin-block-end: 0; }';

// For backwards compatibility, ensure the legacy block gap CSS variable is still available.
$css .= "$selector { --wp--style--block-gap: $block_gap_value; }";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
// In the case of this appender, it needs to apply those same rules to avoid layout shifts.
// Such shifts happen when the bottom margin of the Title block has been set to less than the default 1em margin of paragraphs.
:where(body .is-layout-constrained) & {
> :first-child:first-child {
> :first-child {
margin-block-start: 0;
}

Expand Down
Loading
Loading