-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Style engine: enqueue block supports styles in Gutenberg #42880
Merged
ramonjd
merged 13 commits into
trunk
from
update/style-engine-enqueue-block-supports-scripts-in-gutenberg
Aug 9, 2022
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
5e782cf
Enqueuing block supports styles in gutenberg
ramonjd 6196b91
get_instance() and constructor no longer needed
aristath d6e3552
remove $instance declaration
ramonjd 255c66d
Rename hook callback to `gutenberg_enqueue_stored_styles`
ramonjd 34ac0e2
Removing layout-block-supports store
ramonjd f8aadae
Removing layout-block-supports store
ramonjd 3b833a0
Assign $duplicate_selectors var to avoid duplicate implodes
ramonjd 15300bf
Update packages/style-engine/README.md
ramonjd 1227f28
Apply suggestions from code review
ramonjd 48fb1fd
Remove unnecessary conditional
ramonjd e9bf336
Remove deprecation notice for now until we can deal with other usages…
ramonjd 13785f1
For block themes, print stored styles in the header.
ramonjd 93b49bf
Update packages/style-engine/README.md
aristath File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,61 @@ static function () use ( $style ) { | |
); | ||
} | ||
|
||
/** | ||
* Fetches, processes and compiles stored core styles, then combines and renders them to the page. | ||
* Styles are stored via the style engine API. See: packages/style-engine/README.md | ||
*/ | ||
function gutenberg_enqueue_stored_styles() { | ||
$is_block_theme = wp_is_block_theme(); | ||
$is_classic_theme = ! $is_block_theme; | ||
|
||
/* | ||
* For block themes, print stored styles in the header. | ||
* For classic themes, in the footer. | ||
*/ | ||
if ( | ||
( $is_block_theme && doing_action( 'wp_footer' ) ) || | ||
( $is_classic_theme && doing_action( 'wp_enqueue_scripts' ) ) | ||
) { | ||
return; | ||
} | ||
|
||
$core_styles_keys = array( 'block-supports' ); | ||
$compiled_core_stylesheet = ''; | ||
$style_tag_id = 'core'; | ||
foreach ( $core_styles_keys as $style_key ) { | ||
// Add comment to identify core styles sections in debugging. | ||
if ( defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ) { | ||
$compiled_core_stylesheet .= "/**\n * Core styles: $style_key\n */\n"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea to aid debugging! |
||
} | ||
// Chain core store ids to signify what the styles contain. | ||
$style_tag_id .= '-' . $style_key; | ||
$compiled_core_stylesheet .= gutenberg_style_engine_get_stylesheet_from_store( $style_key ); | ||
} | ||
|
||
// Combine Core styles. | ||
if ( ! empty( $compiled_core_stylesheet ) ) { | ||
wp_register_style( $style_tag_id, false, array(), true, true ); | ||
wp_add_inline_style( $style_tag_id, $compiled_core_stylesheet ); | ||
wp_enqueue_style( $style_tag_id ); | ||
} | ||
|
||
// If there are any other stores registered by themes etc, print them out. | ||
$additional_stores = WP_Style_Engine_CSS_Rules_Store_Gutenberg::get_stores(); | ||
foreach ( array_keys( $additional_stores ) as $store_name ) { | ||
if ( in_array( $store_name, $core_styles_keys, true ) ) { | ||
continue; | ||
} | ||
$styles = gutenberg_style_engine_get_stylesheet_from_store( $store_name ); | ||
if ( ! empty( $styles ) ) { | ||
$key = "wp-style-engine-$store_name"; | ||
wp_register_style( $key, false, array(), true, true ); | ||
wp_add_inline_style( $key, $styles ); | ||
wp_enqueue_style( $key ); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* This applies a filter to the list of style nodes that comes from `get_style_nodes` in WP_Theme_JSON. | ||
* This particular filter removes all of the blocks from the array. | ||
|
@@ -109,5 +164,8 @@ function gutenberg_enqueue_global_styles() { | |
remove_action( 'wp_enqueue_scripts', 'gutenberg_enqueue_global_styles_assets' ); | ||
remove_action( 'wp_footer', 'gutenberg_enqueue_global_styles_assets' ); | ||
|
||
// Enqueue global styles, and then block supports styles. | ||
add_action( 'wp_enqueue_scripts', 'gutenberg_enqueue_global_styles' ); | ||
add_action( 'wp_footer', 'gutenberg_enqueue_global_styles', 1 ); | ||
add_action( 'wp_enqueue_scripts', 'gutenberg_enqueue_stored_styles' ); | ||
add_action( 'wp_footer', 'gutenberg_enqueue_stored_styles', 1 ); | ||
andrewserong marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Just checking, the idea with using a generic name like
wp_enqueue_stored_styles
rather than being tied to the style engine, is that we're not exposing any style engine behaviour here? I think that sounds good to me, in that this is about enqueuing stored styles in general, (and deals with essential styles for WordPress) rather than being specific to the style engine.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.
Another tricky naming issue 😄
"Stored" indirectly implies the style engine, but yeah the motivation was to expand this function later to apply to all stores. See comment above.
If that's true, it will probably require a strategy to subsume/integrate
gutenberg_enqueue_global_styles()
when we get to global 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.
Only pulling from style engine stores. Do you think we should be more specific in the naming? 🤔
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.
Naming's hard! For anything that's specifically style engine related (rather than the style engine being an implementation detail), I like the idea of including
style_engine
in the public function name so that it's all grouped together. So I'd probably lean towardwp_style_engine_enqueue_stored_styles
as the function name, but I don't feel at all strongly about it if you prefer the shorter name 🙂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 sounds infinitely reasonable.
I don't really have a preference either 🙉
I suppose a weak argument to keep it generic is that
wp_style_engine_enqueue_stored_styles
might communicate to some that it's a public function of the style engine package.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.
Good point! Since neither of us feels strongly about it, I think that nudges us more toward keeping the shorter title — after all, it's a function defined outside of the style engine package.