#60280 closed defect (bug) (fixed)
Ensure base global styles are loaded before block styles.
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.5 | Priority: | normal |
Severity: | normal | Version: | 6.5 |
Component: | Script Loader | Keywords: | has-patch |
Focuses: | css | Cc: |
Description
When should_load_separate_core_block_assets
is true, block-library stylesheets for each block are enqueued separately, so they only get loaded if the block is on the page.
Global styles are enqueued later on, and any block-specific global styles are appended to the above block stylesheets, so they end up sitting above base global styles in the CSS cascade.
Base global styles should instead be loaded before block styles (or at least before global block styles, so they are applied in the correct order.
Having the correct style loading order in place is essential to reduce the specificity of block global styles, in progress in βhttps://github.com/WordPress/gutenberg/pull/57841. This in turn will allow section styles: βhttps://github.com/WordPress/gutenberg/issues/57537 to be generated with sustainable levels of specificity.
Change History (15)
This ticket was mentioned in βPR #5892 on βWordPress/wordpress-develop by β@isabel_brison.
14 months ago
#1
- Keywords has-patch added
#2
@
14 months ago
- Type changed from enhancement to defect (bug)
Base global styles should instead be loaded before block styles
+1. The "global" stylesheets should always go before the "local" stylesheets, no matter what. Makes everything down the line simpler. I'd consider this a bug, not an enhancement.
β@isabel_brison commented on βPR #5892:
14 months ago
#3
OK I think I've addressed the issues mention above:
- Split base and block-specific global styles into separate style tags and ordered them so that block-specific global styles come after block library styles when
should_load_separate_core_block_assets
is false. - Split base and block-specific custom CSS and ordered them so base is attached to base global styles, and block-specific comes after the block global styles. It's still not awesome that the CSS for all blocks will always be enqueued - maybe should look into following the logic for block global styles when
should_load_separate_core_block_assets
is true.
Looking into the test failures now, they look legit so there's a good chance I messed up somewhere π
β@oandregal commented on βPR #5892:
14 months ago
#4
I was able to test this and gave it more thought. I've got two concerns:
- The top-level global styles contain styles coming from WordPress, the theme, and/or the user. These should not come _after_ block styles. Not for 6.5, but I think we should explore absorving block styles as part of the global styles algorithm, instead. See βhttps://github.com/WordPress/gutenberg/issues/45198
- From a practical point of view, the top-level global stylesheet contains rules for elements:
h1, h2, h3, ... .wp-element-button:focus, .wp-block-button__link:focus
These can easily conflict with styles coming in block stylesheets. If top-level global styles only had body and preset classes, I'd feel more confident about this approach (though, presets classes can conflict with some old presets that exist in block-library for backward compatibility).
---
I'd like to provide an alternative. My understanding is that the goal is:
- top-level global styles come _before_ block-level global styles
- all block-level global styles are bundled together (no separation between
styles.[https://github.com/WordPress/wordpress-develop/pull/5892#discussion_r1461723298 block-name].css
andstyles.[block-name].typograpy
) ([see]):
Can we achieve this by updating the code around wp_add_global_styles_for_blocks
, so block-level global styles are always loaded as part of the global stylesheet but we only bundle those in use if should_load_separate_block_assets
is true?
AFAIK, @scruffian @ajlende @aristath are the ones who implemented/worked the most with this (elements, the filter for blocks, etc.), so it'd be great if they provided feedback as well, in case I'm missing something.
β@isabel_brison commented on βPR #5892:
14 months ago
#5
Thanks for the feedback!
- top-level global styles come _before_ block-level global styles
- all block-level global styles are bundled together (no separation between
styles.[https://github.com/WordPress/wordpress-develop/pull/5892#discussion_r1461723298 block-name].css
andstyles.[block-name].typograpy
) ([see]):
Can we achieve this by updating the code around
wp_add_global_styles_for_blocks
, so block-level global styles are always loaded as part of the global stylesheet but we only bundle those in use ifshould_load_separate_block_assets
is true?
What about core block-library styles for each block? I think they should come after top-level global styles, but if we bundle block-level global styles with the top-level global stylesheet, then at best the order will be:
- top-level global styles
- block-level global styles
- block-library block styles
which I don't think is ideal because themes might want to override some of the block-library styles with global styles.
I think ideally the order would be
- top-level global styles
- block-library block styles
- block-level global styles
which is what we have in this PR.
In any case, I tried merging block and top-level global styles into the same stylesheet by changingβthe relevant part of wp_add_global_styles_for_blocks
to something like:
add_filter( 'render_block', static function ( $html, $block ) use ( $metadata, $block_css ) { if ( isset( $metadata['name'] ) ) { wp_add_inline_style( 'global-styles', $block_css ); } // The likes of block element styles from theme.json do not have $metadata['name'] set. if ( ! isset( $metadata['name'] ) && ! empty( $metadata['path'] ) ) { $block_name = wp_get_block_name_from_theme_json_path( $metadata['path'] ); if ( $block_name ) { wp_add_inline_style( 'global-styles', $block_css ); } } return $html; }, 10, 2 );
based on the βblock asset enqueuing logic, but isn't working locally for me, not sure why. I'd appreciate any pointers to what I'm doing wrong here π
Totally agree with merging block custom CSS with the rest of the block global styles though. I'll give that a go next.
β@isabel_brison commented on βPR #5892:
14 months ago
#6
Totally agree with merging block custom CSS with the rest of the block global styles though. I'll give that a go next.
Actually, given that the order of custom CSS isn't really relevant for the main cause of this PR, which is unblocking βa specificity reduction in global styles, it might be best to pull those changes out and tackle them as a separate ticket/PR. I'm happy to look into it myself, but my first priority now is the specificity reduction work.
#7
@
14 months ago
I have tested this to the best of my ability but I am still not sure that I am testing the correct thing.
Twenty Twenty-One: Classic theme without theme.json, that enqueue additional CSS for some blocks: I saw no change in the block style.
Twenty Twenty: same as Twenty Twenty-One.
Astra: Classic theme that uses theme.json. The styles in theme.json are very limited, for example it changes the underline on elements.links, but then still overrides this with some custom CSS.
I saw no change in the block style.
---
Reading the discussion, it sounds like the plan is to change which stylesheets contain what CSS:Further testing needs to be done to ensure that themes and plugins can still enqueu but also deque, remove, the CSS.
β@isabel_brison commented on βPR #5892:
14 months ago
#8
Update: I've worked out why the global styles are interfering with editor chrome styling on this branch: enqueuing base global styles on init
means they get added in the editor as well as the front end, whereas using wp_enqueue_scripts
as they are on trunk they only get added in the front end π€¦
The goal of enqueuing global styles on init
was to get them on the page before the block-specific styles were enqueued, which happens when the blocks render. The block-specific styles have to be attached to block render so that we can load them only when the block is present. I'm not sure that there's any other way of loading styles conditionally - at least I can't think of any, but keen to hear ideas anyone has on this!
I guess we can try to only enqueue global styles on init
in the front end, but I can't see how to achieve @oandregal 's goal of
Can we achieve this by updating the code around wp_add_global_styles_for_blocks, so block-level global styles are always loaded as part of the global stylesheet but we only bundle those in use if should_load_separate_block_assets is true?
Again, ideas welcome! I'm also curious to know if/how the work in βhttps://github.com/WordPress/gutenberg/issues/45198 is taking conditional loading into account.
β@isabel_brison commented on βPR #5892:
14 months ago
#9
Ok I've fixed the tests and I think this is ready for further reviews now. The main point I'm looking for feedback on is whether is is acceptable to have base global styles loaded before core block-library styles.
Given the way that the conditional loading of block styles is coupled with render_block
, I don't see much in the way of alternatives, but ideas are very welcome!
This ticket was mentioned in βPR #6010 on βWordPress/wordpress-develop by β@isabel_brison.
14 months ago
#10
Trac ticket: https://core.trac.wordpress.org/ticket/60280
Alternative to #5892.
Given that it's undesirable to have base global styles load before block-library styles, this PR changes just the block global styles so they load after the base global styles.
The order of loading should now be:
- Block library styles
- Base global styles (element styles, etc)
- Block-specific global styles
This order should be the same regardless of whether should_load_separate_core_block_assets
is true or false.
This addresses part of @oandregal's feedback βhere. It doesn't touch on custom CSS yet. To fix the existing bug and keep this change small, I'll address the custom CSS separately (it's not really the same issue, because the _order_ of custom CSS is fine, the problem is that it's always loading whether the target blocks are on the page or not)
β@oandregal commented on βPR #6010:
14 months ago
#11
OK, I had to jump through some hoops but I finally created a test case for this, was able to reproduce it, and this works nicely. Thanks for keep trying!
This is how I reproduce:
- Using the TwentyTwentyFour theme, with default configuration.
- Created two posts. One with the quote block and one without. Publish them.
- Verify that:
- the post with the quote has the quote styles as part of the
global-styles-inline-css
stylesheet while the other doesn't. - the
wp-block-quote-inline-css
stylesheet doesn't have the block-level styles that are now part ofglobal-styles-inline-css
.
- the post with the quote has the quote styles as part of the
- Go to the
functions.php
file of the theme and add the following:
add_filter( 'should_load_separate_core_block_assets', '__return_false', 11 );
- Reload the two posts and verify that
- both posts have the quote styles as part of the
global-styles-inline-css
stylesheet. - the
wp-block-quote-inline-css
stylesheet doesn't have the block-level styles that are now part ofglobal-styles-inline-css
.
- both posts have the quote styles as part of the
#12
@
14 months ago
- Owner set to isabel_brison
- Resolution set to fixed
- Status changed from new to closed
In 57546:
β@isabel_brison commented on βPR #6010:
14 months ago
#13
Thanks for reviewing @oandregal ! Committed in r57546.
β@isabel_brison commented on βPR #5892:
14 months ago
#14
Thanks for the discussion folks! Closing this one in favour of #6010.
Trac ticket: https://core.trac.wordpress.org/ticket/60280
Not sure if this is the best approach so would appreciate feedback on that! I'm seeing some weirdness in the editor where the theme heading font family is overriding the default editor one, which is a bit unexpected: