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

New inline CSS rules for nested blocks difficult to override #36135

Closed
markhowellsmead opened this issue Nov 2, 2021 · 44 comments
Closed

New inline CSS rules for nested blocks difficult to override #36135

markhowellsmead opened this issue Nov 2, 2021 · 44 comments
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Type] Bug An existing feature does not function as intended

Comments

@markhowellsmead
Copy link

markhowellsmead commented Nov 2, 2021

Description

Using the option to “inherit default layout” adds a custom inline style block to the markup. Thanks to the unique ID-based class name applied per block, the highly-specific descendant selector rules, and the use of !important, it's difficult to override the rules with theme or plugin CSS.

e.g.

.wp-container-61810012475e7 > * {max-width: 660px;margin-left: auto !important;margin-right: auto !important;}.wp-container-61810012475e7 > .alignwide { max-width: 1140px;}.wp-container-61810012475e7 .alignfull { max-width: none; }.wp-container-61810012475e7 .alignleft { float: left; margin-right: 2em; }.wp-container-61810012475e7 .alignright { float: right; margin-left: 2em; }

Step-by-step reproduction instructions

  1. Use a nested block with the option “inherit default layout” activated.
  2. Try to set a custom max-width on a child element with alignwide applied using theme CSS.

Screenshots, screen recording, code snippet

No response

Environment info

No response

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

Yes

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

Yes

@markhowellsmead
Copy link
Author

markhowellsmead commented Nov 2, 2021

It's also worth noting that the float rules are unnecessary in this context, because they are already defined in theme CSS for all elements with the classes alignleft, alignright and aligncenter.

@keithdevon
Copy link

Not only is this incredibly difficult to override in theme CSS, it also introduces loads of bloat.

@markhowellsmead markhowellsmead changed the title New inline CSS rules for nested blocks almost impossible to override New inline CSS rules for nested blocks difficult to override Nov 2, 2021
@carolinan
Copy link
Contributor

they are already defined in theme CSS for all elements with the classes alignleft, alignright and aligncenter.

That is not always accurate it depends on the theme. One of the purposes with the block alignments is that these styles no longer has to be in the theme.

@markhowellsmead
Copy link
Author

markhowellsmead commented Nov 2, 2021

@carolinan Is that documented anywhere? I've not come across any information about leaving certain rules to be defined by core. Defining them on a per-use case like this isn't efficient; global rules like this which apply to many different elements shouldn't be re-defined over and over again. (But that's a different topic.)

@keithdevon
Copy link

Would it be possible to add a class to the block instead, e.g. layout-inherit, and then CSS be added to the head like so?

.layout-inherit > * {
  max-width: 750px;
  margin-left: auto !important;
  margin-right: auto !important;
}
.layout-inherit > .alignwide { 
  max-width: 1250px;
}
.layout-inherite .alignfull { 
  max-width: none; 
}
.layout-inherit .alignleft { 
  float: left; 
  margin-right: 2em; 
}
.layout-inherit .alignright { 
  float: right; 
  margin-left: 2em; 
}

The block specific class could then be used for custom alignment widths only.

Also, why aren't the max-width values custom properties generated from the settings.layout options? E.g.

.layout-inherit > .alignwide { 
  max-width: var(--wp--preset--layout--wide-size);
}

@overclokk
Copy link

This is a very annoying thing that I was thinking opening an issue about it.
I don't know why should be generated a new class for every container that inherit layout instead of using the same declaration for avoiding duplication and bloating.

@markhowellsmead
Copy link
Author

markhowellsmead commented Nov 2, 2021

Like @keithdevon's suggestion. Perhaps the class should be e.g. .wp-block-group--inherit-layout or .wp-block--inherit-layout though, to better conform to the BEM principles in use elsewhere.

@overclokk
Copy link

You have also to consider other blocks that inherit layout like the template-part block.

@markhowellsmead
Copy link
Author

In that case, using a generic, independent class name may be better.

@colorful-tones
Copy link
Member

I wonder if this is solved by adding in add_filter( 'styles_inline_size_limit', '__return_zero' );? See: Block-styles loading enhancements in WordPress 5.8 > Inlining small assets 🤔

@overclokk
Copy link

No, because that css is auto-generated, it is not included by styles files.

@markhowellsmead
Copy link
Author

The size of the CSS isn't relevant in this case: the issue is its specificity and the bloat caused by adding (essentially) the same rules to a page over and over again.

@stevenlinx stevenlinx added [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Type] Bug An existing feature does not function as intended CSS Styling Related to editor and front end styles, CSS-specific issues. labels Nov 3, 2021
@markhowellsmead
Copy link
Author

See also #17069.

@cbirdsong
Copy link

Is there a filter to disable these? They're breaking layouts on existing non-FSE/theme.json themes.

@mrwweb
Copy link

mrwweb commented Jan 26, 2022

Also related: #36053

@langhenet
Copy link

God, I opted to use gutenberg and develop custom blocks to avoid this pagebuilder kind of stuff... and I am also interested in knowing if there is any way to opt out of it.

@Azragh
Copy link

Azragh commented Mar 9, 2022

I noticed this just broke all my columns on sites with the gutenberg plugin activated.

.wp-block-columns {
	margin-bottom: 0;
	column-gap: $p_margin;
	row-gap: 0;
}

Doesn't work anymore, all columns now have .5em gaps.. Why such highly specific inline styles for default stuff!? I didn't customize anything, my theme.json is still on version 1, I didnt set any variable, didn't customize the blocks at all and I didn't find any information about this except the discussions here, which are damn hard to follow if you're just a frontend dev.

At least give us a proper documentation for such "features" and a way to opt out or better opt in via theme.json.

I'm writing CSS for over 15 years now, there's almost nothing I can't achieve (in a clean, DRY way!), but Gutenberg drives me insane. When will you start to accept that not only experienced JS developers create themes? Im honestly thinking about quitting my job.

@landwire
Copy link

I just stumbled across this, as I wanted to create a ticket after seeing all that markup printed on the page. I definitely think the bloat and specificity need to go and the layout/alignment/width behaviour needs to be defined in one central place that can easily be overridden by a theme if it chooses to do so.

It might also be worth thinking about separating widths and alignments settings. At the moment it's all muddled up in alignment. A separation of width and alignment would give us more control over the layout for blocks.

@frg-tech
Copy link

I am not a skilled dev, but have some practical experience with css styling. I just spent a day and a half trying to track down what's going on with this and still not sure what I can do for now.

From what I'm reading, is the only way to override this is to disable inherit? Could someone point me in the right direction?

@Azragh
Copy link

Azragh commented Mar 16, 2022

I just ended up spamming !important all across in my _blocks.scss which is damn annoying. I can live with such overrides in backend, but on frontend it just feels bad.

@markhowellsmead
Copy link
Author

It's been several months now, but I still think that the suggestion by @keithdevon (#36135 (comment)) is the best solution which could be implemented by core. For now, I, too, am having to live with using !important in many more places than should be necessary.

@frg-tech
Copy link

Thanks for the replies. I'm using the twenty twenty-two theme and don't seem to have a _blocks.scss.

I'm trying to get rid of a bottom margin on an img block. I can override it easily in dev tools, but using the same rules in style.css has no effect whatsoever.

.wp-container-6231ecd6d3fb6 > .alignwide  {
    margin-bottom: 0px !important;
}

and many variations that work fine in dev tools but not in style.css

.wp-block-image {
    margin-bottom: 0px !important;
}

I've also tried defining it in theme.json, but I don't think you can add !important to those statements.

"styles": {
    "blocks": {
        "core/image": {
            "margin-bottom": "0px",
            "block-end": "0px"
         }

I know this isn't a support forum, so I'll leave it at that. I just thought I was overlooking an easier fix. I'll just keep poking it with a stick for now.

What an incredibly frustrating exercise. I wish they would take this more seriously. Not all of us are web devs, but we still need the ability to style our sites.

@frg-tech
Copy link

@Azragh
Thanks for the reply. Turns out it I had the wrong function to load my child theme style.css. I had just copied over my old functions.php and once I figure that out I was able to override with the !important tag.

Appreciate the follow-up!

@CyberDomovoy
Copy link

I just stumbled on that one too.

First let me say this:
I was excited to start working with blocks and FSE, imho it is a wonderful idea. BUT the more i use it, the more i feel the time spent to understand it and work around its quirks is not worth the added value.

  • Documentation is sparse, and inconsistent (different official sources will sometime give contradictory information)
  • A lot of redundancy, adding as much possibilities of user error (i.e. when writing templates, most of the blocks need to have settings set in the comment for the block and repeated in an html tag right after???)
  • Too many opinionated decisions, with no way for the user to make a choice by themself

Now, concerning this issue, i first thought of getting around it by setting the default layout to inherit (to leverage this test, and because it makes sense to me), so i wrote this little thing:

function rlk_theme_filter_blocktype_args($args, $blocktype) {
    // Only mess with core blocks
    if( ! str_starts_with($blocktype, 'core/') ) {
        return $args;
    }

    // We're only interested in supports
    if(empty($args['supports'])) {
        return $args;
    }
    $supports = $args['supports'];

    // We're only interested in layout
    if(empty($supports['__experimentalLayout'])) {
        return $args;
    }
    $layout = $supports['__experimentalLayout'];

    // Array or bool or... what?
    // Type inconsistencies are just wrong
    if( ! is_array($layout) ) {
        // Guess we can't help... can we?
        if($layout !== true) {
            return $args;
        }

        $layout = array(
            "default" => array(
                "inherit" => true,
            )
        );
    }
    else
    {
        // Not allowed
        if( isset($layout['allowInheriting']) && $layout['allowInheriting'] === false ) {
            return $args;
        }

        // Already set
        if( isset($layout['default']) ) {
            return $args;
        }

        $layout['default'] = array(
            "inherit" => true,
        );
    }

    $args['supports']['__experimentalLayout'] = $layout;
    return $args;
}
add_filter('register_block_type_args', 'rlk_theme_filter_blocktype_args', 10, 2);

It works, any block allowing to set the layout to "inherit" gets it by default.
Unfortunately, it is not enough, since the following test decides that those modifications should always be applied, unless the theme doesn't set a default layout:

                if ( ! $default_layout ) {
			return $block_content;
		}
		$used_layout = $default_layout;

Which actually doesn't make any sense to me, since that, in effect, makes "inherit" actually mean "use global layout", when it should mean "do not do anything special, let the parent set the layout".

So i'm still going to try and find a way around this, since it only makes things difficult when writing a theme. If i do find a solution, i'll post it here.

I just wish gutenberg would stop being so opinionated and circumvoluted, and start giving the user more control, as it is supposed to.

@andronocean
Copy link

andronocean commented Mar 20, 2022

I'm also developing a new FSE theme, and I can sympathize with a lot of the frustrations expressed here.

First off, to everyone struggling with this — you can remove most of these styles if you need to. @carolinan has a lifesaver guide that goes through how to remove all the different styles Gutenberg creates here: https://fullsiteediting.com/lessons/how-to-remove-default-block-styles/ (thank you so much for everything you've done on that site!). There are quite a few to go through.

What I don't like is that this is the nuclear option. Removing or dequeuing one of these stylesheets to get rid of a particular problem style also removes a lot of other styles that may be fine and useful.

As a little case study, I'm currently trying to adjust how blocks with alignleft and alignright classes are positioned. The design is very close to what Gutenberg does by default: blocks in a single column centered with auto margins. I want left/right aligned content to line up with the text margin on the aligned side. Simple, right?

Today, however, the Gutenberg plugin shoves left-right aligned blocks all the way to the edge of the screen. It writes the left-right align classes like this:

.wp-container-16 .alignleft {
  float: left;
  margin-right: 2em;
  margin-left: 0;
}

That margin-left kills me. First, this has a specificity 0-2-0, so it's immediately impossible for me to simply declare an .alignleft style globally with my own custom margin-left. Secondly, the <style> tag gets printed at the very end of the <head>, and it doesn't seem possible to enqueue an override stylesheet after it. So in order to change this, I need to write a style with specificity at least 0-3-0 — meaning I either target a probably-brittle container tree of selectors or do something silly like .alignleft.alignleft.alignleft { margin-left: var(--inset); }. (Yes, that is a CSS hack you can use if you're frustrated!)

The other option would be to disable these styles entirely. They're created by the gutenberg_render_layout_support_flag function. So what happens if I do this?

remove_filter( 'render_block', 'gutenberg_render_layout_support_flag', 10, 2 );

What happens is that I lose all layout styles generated by Gutenberg, as well as all the wp-container-x classes. It makes the layout settings in theme.json completely useless.

So the developer/designer has to either replicate all the functionality of those styles, with the added loss that it's now decoupled from the editor UI, OR write override styles with higher specificity. I can tell you that I am thoroughly sick of specificity wars in 2022, and yet that seems like the saner option.


From other issues I've read, one of Gutenberg's main rules is that user-applied styles set in the UI should always take precedence. I entirely agree with that.

In practice, however, that is getting muddled with the need for Gutenberg to generate default, global styles based on theme.json. These styles and Gutenberg's own built-in helper styles (like the .alignleft ones) are treated the same as user styles. They're merged together and dumped out in inline stylesheets.

I feel like Gutenberg's style system would do well to separate these things into multiple "layers":

  1. Built-in helpers
  2. Styles from theme.json, color schemes, etc.
  3. Theme-enqueued stylesheets — Note, Gutenberg has NO knowledge what these may contain, so it cannot make assumptions._
  4. User styles set in the UI.

Happily CSS already gives us the tools for this: specificity and the cascade order. The theme styling process would be made far easier and more reliable if Gutenberg kept to a "contract" of sorts for its styles:

  • CSS rules for layers 1 and 2 will never exceed a particular specificity threshold
  • themes can enqueue stylesheets in the <head> after layers 1 and 2 (they can enqueue earlier too, if needed). User styles are printed after all theme/plugin styles.

For that specificity threshold, I'd like 0-1-0, maybe 0-1-X. I could live with 0-2-0 for limited, documented situations where nothing else will work. Though now that browser support for :where() is getting good those are probably slim indeed.

Has anything like this been explored previously?


P.S. Yes, I could write my layout CSS a little differently to address the alignleft issue — put padding around all post content, then use negative margins to stretch full-width blocks, etc. But I shouldn't have to do that.

P.P.S. If anyone's not familiar with specificity shorthand like 0-2-0, the helpful Specifishity diagram explains it. A selector example in the grid beats the selectors to its left, and every row of selectors above it.

@CyberDomovoy
Copy link

CyberDomovoy commented Mar 20, 2022

Great post @andronocean , thanks for all this.

@CyberDomovoy
Copy link

CyberDomovoy commented Mar 21, 2022

Ok, this is what i came up with:

 function rlk_theme_render_layout_support_flag($block_content, $block)
 {
    $block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );
    if(function_exists('gutenberg_block_has_support')) {
        $support_layout = gutenberg_block_has_support( $block_type, array( '__experimentalLayout' ), false );
    }
    else
    {
        $support_layout = block_has_support( $block_type, array( '__experimentalLayout' ), false );
    }

    // Block doesn't support layout
    if( ! $support_layout ) {
        return $block_content;
    }

    // Help us easily manipulate the block
    // Wordpress actually kind of recreates a DOM with its blocks
    // They could have done it with a XML Doctype/schema
    // and let us use all those marvelous functions that manipulates XML/DOM
    // The issue has already been raised, they don't want to
    $dom = new DOMDocument('1.0', 'UTF-8');

    // HTML5 is not well accepted by loadHTML: use LIBXML_NOERROR
    // Make sure UTF-8 is correctly handled: use meta Content-Type
    $dom->loadHTML('<html>
    <head>
        <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
    </head>
    <body>'. $block_content .'</body>
</html>', LIBXML_NOERROR);

    // Get direct child _elements_ of body
    // Oddly enough,
    // wordpress adds garbage around the root node of the block
    // (i.e. white spaces)
    $body = $dom->getElementsByTagName('body')[0];
    $elems = array();
    foreach($body->childNodes as $node) {
        if($node->nodeType === XML_ELEMENT_NODE) {
            $elems[] = $node;
        }
    }

    // No root element, or more than 1?
    // Should not happen, abort
    if(count($elems) != 1) {
        return $block_content;
    }
    $block_dom = $elems[0];
    $block_classes = $block_dom->getAttribute("class");

    //~ $frag = $dom->createDocumentFragment();
    //~ $frag->appendXML('<pre><code>'. esc_html(print_r($block, true)) .'</code></pre>');
    //~ $block_dom->insertBefore($frag, $block_dom->firstChild);

    // At this point, we already know the block uses layout
    // Set a class
    $block_classes .= ' has-layout';

    $default_block_layout = _wp_array_get( $block_type->supports, array( '__experimentalLayout', 'default' ), array() );
    $layout = isset( $block['attrs']['layout'] ) ? $block['attrs']['layout'] : $default_block_layout;

    //~ $frag = $dom->createDocumentFragment();
    //~ $frag->appendXML('<pre><code>'. print_r($layout, true) .'</code></pre>');
    //~ $block_dom->insertBefore($frag, $block_dom->firstChild);

    $layout_type = 'inherit';
    if( ! isset( $layout['inherit'] ) || $layout['inherit'] !== true ) {
        // Not inherit, get type
        $layout_type = isset( $layout['type'] ) ? $layout['type'] : 'default';
    }
    // Add a class for the type
    $block_classes .= ' layout-type-'. $layout_type;

    // Build the style
    $style = '';
    $selector = wp_unique_id( 'wp-container-' );
    $block_classes .= ' '. $selector;
    $selector = '.'. $selector;
    switch($layout_type) {
        case "default":
                $content_size = isset( $layout['contentSize'] ) ? $layout['contentSize'] : '';
                $wide_size    = isset( $layout['wideSize'] ) ? $layout['wideSize'] : '';

                // Make sure there is a single CSS rule, and all tags are stripped for security.
                // TODO: Use `safecss_filter_attr` instead - once https://core.trac.wordpress.org/ticket/46197 is patched.
                // This should be done way before now,
                // WP spends a lot of time parsing values when it gets them
                // It should be done once and for all when they are _set_
                $content_size  = esc_html(wp_strip_all_tags( explode( ';', $content_size )[0] ));
                $wide_size = esc_html(wp_strip_all_tags( explode( ';', $wide_size )[0] ));

                if ( ! empty($content_size) || ! empty($wide_size) ) {
                    // If one is missing, use the other
                    $content_max_width_value  = empty($content_size) ? $wide_size : $content_size;
                    $wide_max_width_value = empty($wide_size) ? $content_size : $wide_size;

                    $style .= '
'. $selector .' > *:not(.alignleft):not(.alignright),
'. $selector .' .layout-type-inherit > *:not(.alignleft):not(.alignright) {
    max-width: '. $content_max_width_value .';
}
'. $selector .' > .alignwide,
'. $selector .' .layout-type-inherit > .alignwide {
    max-width: '. $wide_max_width_value .';
}
'. $selector .' > .alignfull,
'. $selector .' .layout-type-inherit > .alignfull {
    max-width: 100vw;
}
';
                }

            break;
        case "flex":
                $layout_orientation = empty( $layout['orientation'] ) ? 'horizontal' : $layout['orientation'];
                $block_classes .= ' orientation-'. $layout_orientation;

                $justify = ($layout_orientation == 'horizontal') ? 'center' : 'left';
                $layout_justify = empty( $layout['justifyContent'] ) ? $justify : $layout['justifyContent'];
                $block_classes .= ' justify-'. $layout_justify;

                if(!empty( $layout['flexWrap'] ) && $layout['flexWrap'] == "wrap") {
                    $block_classes .= ' is-wrap';
                }
            break;
    }


    //~ $frag = $dom->createDocumentFragment();
    //~ $frag->appendXML('<pre><code>'. $style .'</code></pre>');
    //~ $block_dom->insertBefore($frag, $block_dom->firstChild);
    if(!empty($style)) {
        if(function_exists('gutenberg_enqueue_block_support_styles')) {
            gutenberg_enqueue_block_support_styles( $style );
        }
        else
        {
            wp_enqueue_block_support_styles( $style );
        }
    }
    $block_dom->setAttribute("class", $block_classes);

    return $dom->saveHTML($block_dom);
 }

/**
 * Fix bloated CSS due to the use of layout options
 */
if( function_exists( 'wp_render_layout_support_flag' ) ) {
    remove_filter( 'render_block', 'wp_render_layout_support_flag' );
}
if( function_exists( 'gutenberg_render_layout_support_flag' ) ) {
    remove_filter( 'render_block', 'gutenberg_render_layout_support_flag' );
}
add_filter( 'render_block', 'rlk_theme_render_layout_support_flag', 10, 2 );

And my CSS:

/****************/
/*    Layout    */
/****************/

.layout-type-default > *:not(.alignleft):not(.alignright),
.layout-type-default .layout-type-inherit > *:not(.alignleft):not(.alignright) {
    margin-left: auto;
    margin-right: auto;
}

.layout-type-default > .alignleft,
.layout-type-default .layout-type-inherit > .alignleft {
    float: left;
    margin-left: 0px;
    margin-right: var(--wp--custom--gap--large);
}

.layout-type-default > .alignright,
.layout-type-default .layout-type-inherit > .alignright {
    float: right;
    margin-left: var(--wp--custom--gap--large);
    margin-right: 0px;
}

.layout-type-flex {
    display: flex;
}

.layout-type-flex.is-wrap {
    flex-wrap: wrap;
}

.layout-type-flex.justify-left {
    justify-content: flex-start;
}

.layout-type-flex.justify-right {
    justify-content: flex-end;
}

.layout-type-flex.justify-center {
    justify-content: center;
}

.layout-type-flex.orientation-horizontal {
    align-items: center;
}

.layout-type-flex.orientation-horizontal.justify-space-between {
    justify-content: space-between;
}

.layout-type-flex.orientation-vertical {
    flex-direction: column;
}

.layout-type-flex > * {
    margin: 0px;
}

A few things to notice:

  • The global layout is not applied, this should be done maybe in a filter that sets it on <body>
  • Most of it is done with classes: we only really need to set user values as specific element styles (max-width), everything else can be generalized with classes
  • As a consequence of the above, wordpress could add a general CSS similar to the one i posted, it doesn't have to be specific to each block with a layout
  • blockGap is gone, easy enough to add, but imho, unless the user can set the gap from the UI, the theme developer knows if he uses it, it could also be easily added as a general style, doesn't need to be set on every specific element
  • There are some comments containing $dom->createDocumentFragment(), i was using those for debugging, left them there as an example for anybody who wants to play with this code

I have not extensively tested this, but AFAICT, it does the job (much less bloated CSS, much easier to override).

I wonder about my rules about layout-type-inherit, those selecting like:

.layout-type-default .layout-type-inherit > *:not(.alignleft):not(.alignright)

(i now just see i didn't write those for layout-type-flex)

What happens when there is a block with a layout that is not inherit between a .layout-type-default and the targeted inherit? we have multiple rules competing, which layout wins?

I'm not an expert with CSS, maybe someone can correct my rules.

Hoping this will help some of you

@markhowellsmead
Copy link
Author

Thank you @andronocean for such a good explanation, and for referring to the specificity calculator, which should help to explain why such precise style rules are so problematic for theme and plugin developers trying to work around core rules.

@CyberDomovoy
Copy link

Some simple thing just crossed my mind...

Shouldn't setting a new layout be the purpose of a block?
I mean, it is something quite specific, you're not going to change the maximum space allowed to blocks that often, are you?

@CyberDomovoy
Copy link

In an effort to keep my theme clean, i made a plugin to fix this issue.
It also makes it simple to disable other styles.
You can find it here.

The thing will probably become more in the future (add inline styles to handles, filters, replace styles, admin UI...), but for now i have to resume work on my theme.
If any of you want to work on it, you're welcome to send pull requests.

Here is an example of how to use it:

/**
 * Blocks with layout
 */
function rlk_theme_add_block_layout($style, $selector, $layout) {
    switch($layout['type']) {
        case "default":
            $content_size = isset( $layout['contentSize'] ) ? $layout['contentSize'] : '';
            $wide_size    = isset( $layout['wideSize'] ) ? $layout['wideSize'] : '';

            // Make sure there is a single CSS rule, and all tags are stripped for security.
            // TODO: Use `safecss_filter_attr` instead - once https://core.trac.wordpress.org/ticket/46197 is patched.
            // This should be done way before now,
            // WP spends a lot of time parsing values when it gets them
            // It should be done once and for all when they are _set_
            $content_size  = esc_html(wp_strip_all_tags( explode( ';', $content_size )[0] ));
            $wide_size = esc_html(wp_strip_all_tags( explode( ';', $wide_size )[0] ));

            if ( ! empty($content_size) || ! empty($wide_size) ) {
                // If one is missing, use the other
                $content_max_width_value  = empty($content_size) ? $wide_size : $content_size;
                $wide_max_width_value = empty($wide_size) ? $content_size : $wide_size;

                $style .= '
'. $selector .' > *:not(.alignleft):not(.alignright),
'. $selector .' .layout-type-inherit > *:not(.alignleft):not(.alignright) {
max-width: '. $content_max_width_value .';
}
'. $selector .' > .alignwide,
'. $selector .' .layout-type-inherit > .alignwide {
max-width: '. $wide_max_width_value .';
}
'. $selector .' > .alignfull,
'. $selector .' .layout-type-inherit > .alignfull {
max-width: 100vw;
}
';
            }
            break;
    }

    return $style;
}

// Disable unwanted styles
RLK_Plugin_StyleMan::disable(array(
    // Core blocks with layout
    'wp-block-layout'
));

// Apply our own style to blocks with layout
add_filter('rlk_styleman_add_block_layout', 'rlk_theme_add_block_layout', 1, 3);

IMHO, this is a better way of dealing with that layout attribute, the CSS rules are not hard coded, but come from a filter, the user is in control.
As stated previously, the use of classes also makes it so that the most "general" rules (margin: auto and such) are not applied to every block specifically, should make those way easier to override.

@anthonyeden
Copy link

I have read through this thread, and think it is semi-related to an issue I am having (but not quite).

While I am happy for the Block Editor to generate inline CSS, it outputs the HTML & CSS it in a way which is impossible to filter. The function wp_enqueue_block_support_styles echos the <style> tags and associated CSS, with no filter.

In my case, I have a quasi-SPA run by WordPress - and I need to add unique IDs to all Style tags in order to load the correct ones into page loads.

The best I can currently come up with as a proof of concept is this:

add_action('wp_footer', function() {
    ob_start();
}, 1, 0);

add_action('wp_footer', function() {
    $html = ob_get_contents();
    ob_end_clean();

    // Add unique IDs to all <style> tags
    $html = preg_replace_callback("#<style>(.*?)</style>#is", function($matches) {
        $id = 'style-' . md5($matches[1]) . '_inline';
        return "<style id='".$id."' class='style-inline'>".$matches[1]."</style>";
    }, $html);

    echo $html;

}, 9998, 0);

But let's be honest, this use of output buffering and regex is hacky. This approach will likely break at some point in the future.

I wonder if the Block Editor should be leveraging wp_add_inline_style, or at least adding a filter to the output of wp_enqueue_block_support_styles.

@markhowellsmead
Copy link
Author

See also #41230.

@markhowellsmead
Copy link
Author

This is being discussed and solved through PR #38974.

@markhowellsmead
Copy link
Author

See also #40159 (comment).

@drdogbot7
Copy link

they are already defined in theme CSS for all elements with the classes alignleft, alignright and aligncenter.

That is not always accurate it depends on the theme. One of the purposes with the block alignments is that these styles no longer has to be in the theme.

I don't agree that this is necessary, but if that's the intent, why not just add a few basic alignment classes to the block library CSS that's already being loaded?

What is the possible utility of adding a new set of redundant, overly specific styles for every single container? I've being following several threads on this issue and I still don't even understand why this was done in the first place.

I've spent years learning how to write CSS that's efficient, comprehensible and maintainable. I see this, and I literally want to cry:

image

@markhowellsmead
Copy link
Author

100x this, @drdogbot7.

@davidwebca
Copy link

I just encountered this issue by enabling the align feature for the first time since 6.0. I'm having even more specificity than that here with a CSS selector of 050.

I'm genuinely curious as to how this ends up passing review. Is there no one in core developing themes with actual code anymore? Who wrote this horrendous function that generates inline CSS for every block? Why is this even a thing?

@Azragh
Copy link

Azragh commented Jul 14, 2022

I could understand the idea behind generating inline-styles for blocks with different settings than default, but I don't get this:

image

Isn't it possible to disable this, but still being able to use group blocks, margin and padding? I mean alignment classes exist in my themes (and in any other I assume) since ever, no need to define them again. The owl selector for inner blocks eg. the default could be done in one declaration with a global .wp-container class which only gets a number when changes are made so yeah, I just don't get it.

The only solution I found so far is to disable the new layout stuff alltogether which is sad actually..

@markhowellsmead
Copy link
Author

@Azragh @davidwebca See #40875.

@drdogbot7
Copy link

I'm genuinely curious as to how this ends up passing review.

This is speculation, but I think there's 2 reasons.

(1) The core team are VERY focused on Full Site Editing.

They're looking at these containers in that context and thinking that this is a unique, one-off layout element (e.g. a site header or footer), therefore these janky, oddly-specific internal stylesheets aren't a big problem… there's not gonna be like 15 of them on a page right? Right??

The problem, of course, is that in practice a lot of us aren't building FSE themes, but we would really like to use the group block, columns block, gallery block, etc, without having it generate this all this jank.

(2) Too many core developers don't take CSS seriously

In theory there is a pretty sensible BEM-ish standard for block css, but too often it's simply not enforced.

This container block is a great example. There should be a generic class named .wp-block-container, and then if individual containers need their own ID's, they should have an additional class like .wp-block-container--id-7, or in a more verbose, gutenberg-y style .has-container-id, .is-id-7… or something like that.

Instead, every single container is treated like a unique element with a set of !important styles, because [see (1)]

How do we fix this?

#40875 does seem like a good attempt to rein some of this in. This is what we need more of.

It is exhausting, but we need more people who care about good CSS to get involved and start pushing back on this kind of feature before it actually gets accepted. If we just wait until something gets released and then complain about it (and I fully admit I am guilty of this), we're never going to get what we want.

@Rostyk27
Copy link

For now, I've managed just to remove annoying inline styles with JS since there's currently no way to disable it with PHP:

$('style').each(function () {
    let _t = $(this),
        _h = _t[0].innerHTML;

    if (_h.includes('.wp-container-')) {
        _t.remove();
    }
});

@mrfoxtalbot
Copy link

mrfoxtalbot commented Sep 29, 2022

I believe the most prominent issue regarding IDs has been solved here:
#41434 (comment)

The two related issues, #17069, and #36053 have been closed and the PR that was mentioned has been merged too #40875

I am going to close this issue but if is there something else we should still address, please feel free to reopen it, @markhowellsmead. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests