WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.3k stars 4.11k forks source link

Css specificity issues with blockGap from theme.json and from editor (W6.6.1) #64001

Closed atrzyna closed 1 month ago

atrzyna commented 1 month ago

Description

We are trying to apply a different block gap between paragraphs and lists. Our global blockGap is set in theme.json to 2.5rem but for in between paragraphs and lists we want it to be 1rem.

With the introduction of css specificity in wordpress 6.6 this now breaks. This is what we are trying to use:

.is-layout-flow,
.is-layout-constrained {
  :where(p, .wp-block-heading, ul, ol) {
    & + :where(p, ul, ol) {
      margin-block-start: 1rem;
    }
  }
}

When inspecting a paragraph tag we get:

.is-layout-constrained :where(p,.wp-block-heading,ul,ol)+:where(p,ul,ol), .is-layout-flow :where(p,.wp-block-heading,ul,ol)+:where(p,ul,ol) {
    -webkit-margin-before: 1rem;
    margin-block-start: 1rem;
}

.is-layout-flow > * {
    margin-block-start: 2.5rem;
    margin-block-end: 0;
} (index):213

The first rule is applied correctly and both css rules have specificity of (0,1,0). If we make changes to the header, in this case just changing a button from outline to fill, now when inspecting p tag we get:

.is-layout-flow > * {
    margin-block-start: 2.5rem;
    margin-block-end: 0;
} (index):231

.is-layout-constrained :where(p,.wp-block-heading,ul,ol)+:where(p,ul,ol), .is-layout-flow :where(p,.wp-block-heading,ul,ol)+:where(p,ul,ol) {
    -webkit-margin-before: 1rem;
    margin-block-start: 1rem;
}

The first rule is applied incorrectly and notice the index inclusion at line 231 where previously it was 213. I am assuming that since these rules have the same specificity, the inclusion order inline makes a difference here and any changes to the header, where additional css is added inline, affects this inclusion order.

If we make the paragraph css have higher specificity, it will work but then override any block gap we may set in the editor. Here is what happens when we have a group with a bunch of paragraph tags with block gap set on group to 80px:

.wp-block-group.is-layout-constrained :where(p,.wp-block-heading,ul,ol)+:where(p,ul,ol), .wp-block-group.is-layout-flow :where(p,.wp-block-heading,ul,ol)+:where(p,ul,ol) {
    -webkit-margin-before: 1rem;
    margin-block-start: 1rem;
}
.wp-container-core-group-is-layout-6 > * + * {
    margin-block-start: var(--wp--preset--spacing--80);
    margin-block-end: 0;
}
.wp-container-core-group-is-layout-6 > * {
    margin-block-start: 0;
    margin-block-end: 0;
}

Notice we made specificity higher for paragraph rule so its now (0,2,0). It overrides the inline 80px we added in the editor to the group since its specificity is still (0,1,0). (This group has a border in the screenshot attached)

Maybe this last part is the real problem. I am not sure but with 6.6 so much instability has been introduced. There should be a way to opt out of this (css specificity update) since there was no way to test all possible scenarios and it truly breaks sites.

Step-by-step reproduction instructions

  1. Allow blockGap in theme.json
  2. Add paragraph code with higher specificity
    .wp-block-group.is-layout-constrained :where(p,.wp-block-heading,ul,ol)+:where(p,ul,ol), .wp-block-group.is-layout-flow :where(p,.wp-block-heading,ul,ol)+:where(p,ul,ol) {
    -webkit-margin-before: 1rem;
    margin-block-start: 1rem;
    }
  3. In editor add a group with multiple paragraphs and set block gap to something higher than 1rem.
  4. Notice that paragraph code overrides group set block gap. (This group has a border in the screenshot attached)

Screenshots, screen recording, code snippet

Screenshot 2024-07-26 at 10 25 13 AM Screenshot 2024-07-26 at 10 17 42 AM Screenshot 2024-07-26 at 10 19 45 AM

Environment info

Wordpress 6.6.1, custom theme FSE. Mac OS 13.0.1, Chrome Version 126.0.6478.183 (This is not browser specific)

Please confirm that you have searched existing issues in the repo.

Please confirm that you have tested with all plugins deactivated except Gutenberg.

aaronrobertshaw commented 1 month ago

Thanks for writing up such a detailed issue @atrzyna, it's hugely appreciated.

I am assuming that since these rules have the same specificity, the inclusion order inline makes a difference here

💯 It does make a difference.

I have a suspicion this might be a duplicate of https://github.com/WordPress/gutenberg/issues/63912 as you mentioned changing a button's block style from outline to fill.

If we make changes to the header, in this case just changing a button from outline to fill, now when inspecting p tag we get:

The issue with block style variations causing a different dependency order for enqueued stylesheets was resolved in https://github.com/WordPress/gutenberg/pull/63918 and is slated for the release in WordPress 6.6.2 and Gutenberg 19.0.

I'll continue to look into this issue today and will report back with any further findings.

aaronrobertshaw commented 1 month ago

@atrzyna if possible do you think you could expand on the steps to replicate the issue?

After re-reading the description a few times, I was still having to make some assumptions on how and when the CSS was being enqueued etc. A brief clarification or summary of what is expected vs actual behaviour might also make it easier to get some extra eyes on this one.


That said, I've been looking into this a little further and here's what I've found so far:

Style load order

Some of the behaviour does appear related to the bug around style load order when block style variations are in use, which was fixed in #63918

Priority of different style sources

Certain groups of styles are intended to take precedence over others. I believe the order of styles for themes using theme.json and Global Styles, from lowest priority to highest, should be:

  1. Block library styles
  2. Theme styles (before Global Styles as anything managed by Global Styles should be in theme.json?)
  3. Global Styles (including theme.json and layout?)
  4. Block instance element styles (e.g. applying link color to single block)
  5. Block style variation styles applied to individual block instance

@andrewserong is more well-versed with the layout styles, he might be able to correct me on whether layout styles should be a lower priority, or separate, in that list.

Custom layout overrides

Considering the list above, it seems there's a bit of a conflict here. The proposed block gap overrides are intended to override base-level global styles but then have global styles override that if set by the user. I'm not sure that it is possible at the moment or was intended as supported in WP6.5.

Root level layout styles might be able to have their specificity lowered but I'm unclear on the repercussions of that and some time may be needed to work through it.

Summary

My interpretation of the CSS in question is that it is intended to be a "conditional default" across multiple block types which should lie between the default generic layout styles and those for individual block types.

Both the default generic layout styles and individual block type styles are within the Global Styles "layer". This leads to the problems laid out in this issue description:

Workaround

The best option I have found in the short term is to add these styles to the Custom CSS feature of Global Styles.

There are two ways to do this:

  1. Add the styles directly to the theme.json file under styles.css
  2. In the site editor, navigate to Styles > Additional CSS, and add the styles there.

The result is the Custom CSS is added to the Global Styles stylesheet after the base layout styles and before block type specific or block instance styles for layout/elements etc.

This is what I see with the following CSS in the Additional CSS field.

.is-layout-flow :where(p, .wp-block-heading, ul, ol) + :where(p, ul, ol),
.is-layout-constrained :where(p, .wp-block-heading, ul, ol) + :where(p, ul, ol) {
    margin-block-start: 1rem;
}

https://github.com/user-attachments/assets/2ddc96cc-5c3c-4d28-81b9-30880fa76d38

@atrzyna would you mind giving the above workaround a try and seeing if that provides the result you need?

atrzyna commented 1 month ago

@aaronrobertshaw hi. Thank you for looking into this. I have been playing around with fixes for this so much that its getting hard for me to recreate but I found a site that has the problem and then added the suggested css to Additional CSS Field and it worked. After, it consistently applied the correct spacing between paragraphs and also allowed the block gap from editor to override it.

I will setup a simple site and try to replicate it and add additional notes. The custom paragraph styles I'm adding through a plugin, btw. They are not enqueued in the theme. I don't know if this makes a difference.

It very much seems like the issue you mentioned is related but when I used 6.6.2-alpha-58805, it didn't fix the issue. So far only putting the css in Additional CSS Field or theme.json helped.

atrzyna commented 1 month ago

@aaronrobertshaw I made a simple site with a plugin that adds the paragraph code and I can recreate the issue on the homepage. I would prefer not to share the url publicly but let me know how I can get this info to you and I can even provide an admin login if that is helpful.

Screenshot 2024-07-29 at 1 50 40 PM
aaronrobertshaw commented 1 month ago

then added the suggested css to Additional CSS Field and it worked

Sounds like progress!

The custom paragraph styles I'm adding through a plugin, btw

Thanks for the clarification. For the purposes of this issue I don't think the distinction between theme or plugin stylesheets makes much difference. Neither can "insert" styles between two sets of styles within the Global Styles stylesheet.

It very much seems like the issue you mentioned is related but when I used 6.6.2-alpha-58805, it didn't fix the issue

The fix has only landed in Gutenberg with https://github.com/WordPress/gutenberg/pull/63918. The PR bringing those changes to WordPress 6.6.2 is still open but should land soon.

Even with that fix, you'll still need the Custom CSS approach to address your use case.

I made a simple site with a plugin that adds the paragraph code and I can recreate the issue on the homepage

That screenshot is for illustration purposes only, without the suggested Custom CSS added to theme.json, correct?

To add the Custom CSS to a theme's theme.json data you could leverage the theme_json_data filters. If I had to pick one I'd probably recommend using the wp_theme_json_data_theme filter so that your plugin can be sure that a theme adding Custom CSS would remove the style. It would still be customizable by end users within Global Styles > Additional CSS though.

Here's an example that might help:

function my_plugin_add_custom_block_gap_styles( $theme_json ) {
    $blockgap_rule = '.is-layout-flow :where(p, .wp-block-heading, ul, ol) + :where(p, ul, ol), .is-layout-constrained :where(p, .wp-block-heading, ul, ol) + :where(p, ul, ol) { margin-block-start: 1rem; }';
    $existing_css  = $theme_json->get_data()['styles']['css'] ?? '';
    $custom_css    = array(
        'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
        'styles'  => array(
            'css' => $existing_css . $blockgap_rule,
        ),
    );
    return $theme_json->update_with( $custom_css );
}
add_filter( 'wp_theme_json_data_theme', 'my_plugin_add_custom_block_gap_styles', 10, 1 );

Using the above snippet I get consistent application of styles between the frontend and editor:

Frontend Editor
Screenshot 2024-07-30 at 11 20 16 AM Screenshot 2024-07-30 at 11 20 41 AM

Let me know how you go with that. I'm not sure whether we can change specificity on the blockGap styles moving forward so I'm hopeful this workaround will be acceptable for now 🤞

I would prefer not to share the url publicly but let me know how I can get this info to you and I can even provide an admin login if that is helpful.

P.S. WordPress Slack is always a good option if you need to reach out to contributors more directly.

andrewserong commented 1 month ago

Let me know how you go with that. I'm not sure whether we can change specificity on the blockGap styles moving forward so I'm hopeful this workaround will be acceptable for now 🤞

That's my understanding, too. As far as I'm aware, the layout rules were made without factoring in that themes might attempt to inject styles that augment or are related to the layout styles handled by WP, so in this case a workaround for affected themes sounds like the better option to me.

atrzyna commented 1 month ago

@aaronrobertshaw Thanks for your help. This solution works for us.