-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 support styles #42452
Style engine: enqueue block support styles #42452
Conversation
0c1dd59
to
c3bfa2f
Compare
9a07861
to
0f7c753
Compare
0f7c753
to
6c0d5ea
Compare
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.
1b07c83
to
33eb74e
Compare
* 'classnames' => (array) A flat array of classnames. | ||
* ); | ||
*/ | ||
public function parse_block_supports_styles( $block_styles, $options ) { |
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.
Now that the compile and parse functionality is split, it might be a change to move all the parsing-related methods and utilities to another class or file.
2a6a226
to
313019d
Compare
/** | ||
* Fetches, processes and compiles stored styles, then renders them to the page. | ||
*/ | ||
public static function process_and_enqueue_stored_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.
The enqueue and WP hook logic could well live in Gutenberg itself, e.g., script-loader.php
with the style engine only taking care of parse + store + compile
7a58ac9
to
19ab79d
Compare
* abstract stores * "else" not needed * compile_classnames method not needed * we have a method to get the store here * Make the wp_style_engine_add_to_store function always return a store * wp_style_engine_get_stylesheet - always return string * Merged with base branch. Added test for new method. * Whoops! Co-authored-by: ramonjd <ramonjd@gmail.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
} | ||
return null; | ||
return WP_Style_Engine::get_instance()::compile_stylesheet_from_store( $store_key ); |
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.
I believe we don't need to get the instance here, right?
return WP_Style_Engine::get_instance()::compile_stylesheet_from_store( $store_key ); | |
return WP_Style_Engine::compile_stylesheet_from_store( $store_key ); |
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.
I've been looking at this PR and testing it for a couple of days... Overall, it looks good. Should we merge it? If there's anything else, it can be done in follow-up PRs. The code here looks good, and it works fine.
It would be good to get this one in so we can continue implementing the style-engine in other places too 👍
707b8d5
to
1a79985
Compare
* abstract stores * "else" not needed * compile_classnames method not needed * we have a method to get the store here * Make the wp_style_engine_add_to_store function always return a store * wp_style_engine_get_stylesheet - always return string * Merged with base branch. Added test for new method. Co-authored-by: ramonjd <ramonjd@gmail.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
1a79985
to
00d3ae8
Compare
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.
Nice work @ramonjd and @aristath, this looks like it's getting much closer! Nice addition with including a default block-supports context when enqueuing 👍
I ran into a couple of issues while testing the Layout support. The main issue is that it looks like properties with the same selector are being overwritten instead of being concatenated. I've added comments on each of the features I noticed that occurring in, but 🤞 that one should be a fairly straightforward fix.
I wasn't too sure how to go about solving the fallback output for Layout styles, though? I've left a comment on that one.
Happy to do more testing tomorrow!
Update: for ease of testing, I've added some test Layout markup to the PR description. Hopefully it should be pretty clear what the expected result should be when switching between the post editor view and the site front-end.
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.
Thanks for all the updates and tweaks @ramonjd and @aristath! I've re-tested the Elements support and each of the features of the Layout support, and it appears to be all working nicely now 🎉
The tidied up function names look good to me, and let's continue exploring how we register actions, etc, in follow-ups.
I might be overly cautious here, but prior to merging it'd be good to update the PR description with a bit more context, so that if this does introduce any issues, it's a little clearer what's changed to help folks debugging. It could also be worth exploring us switching off the optimisation step of the Processor in this PR, and then switch it on in a separate PR so that we have different commits in trunk
for each step — this way, if there are any issues caused by the changes to Layout output, it'll hopefully be easier to identify exactly the step that caused the issue. Totally not a blocker if you think that's overkill though.
I suspect it'll be perfectly fine, but I recently ran into some weird edge cases with Layout, so just thought I'd mention it in case it helps us with the merge process.
This PR is a great milestone, though, it's a really significant improvement to the Layout style output, and IMO delivers on a lot of the promise of introducing a style engine. Kudos! ✨
Thanks @andrewserong and @aristath for all the help getting this one to a mergeable state! 🙇
Will do, thanks!
I think this is a pretty good step. I'll merge this one with the combination optimizations turned on, but follow it up quickly with an |
Follow ups here: #42878 |
Follow up for enqueuing over here: |
What?
This PR extends the backend style engine, and:
Why?
This is part of the effort to reduce the number of style tags in the rendered HTML.
How?
Using the classes laid down by @aristath in #42222, it's now possible to build stylesheets and enqueue CSS for rendering on the frontend.
There are three global functions that help to achieve this:
wp_style_engine_get_styles()
Use this function for style objects that require parsing, for instance, the value of a block's attributes.style object or the top level styles in theme.json.
To enqueue a style for rendering in the frontend, the
$options
array requires the following:'block-supports'
.wp_style_engine_get_styles
will return the compiled CSS and CSS declarations array.wp_style_engine_add_to_store()
Use this function to enqueue any CSS rules. It will automatically merge declarations and combine selectors.
The resulting stylesheet will be:
wp_style_engine_get_stylesheet_from_css_rules()
Use this function to compile and return a stylesheet for any CSS rules. This function does not enqueue styles, but rather acts as a CSS compiler.
It has a similar function signature to
wp_style_engine_add_to_store
,Future iterations
Testing Instructions
This PR enqueues both link elements styles and Layout block support styles.
In a new post add some text links and assign them a color.
Insert a few group blocks and adjust the layout settings (alignment, widths, justifications). Ideally, manually test each of the features of both default/flow and flex-based Layouts using blocks such as Group, Columns, Buttons.
Ensure that the colors and layout appear on the frontend as expected.
Here's some test block HTML. I'm testing using emptytheme, but we should also test in other themes including classic themes.
Example markup for custom link colors
Example markup for Layout support
Enable
"useRootPaddingAwareAlignments": true
in your theme.json and give a group block full alignment and some left and right padding.The layout styles should be printed as expected, e.g.,
.wp-block-group.wp-container-6 > .alignfull {margin-right: calc(26px * -1); margin-left: calc(69px * -1);}
Also make sure the CSS rules are being batched into a single, inline style sheet. For example, this:
Instead of this: