-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Deprecate wp_enqueue_block_support_styles #4015
base: trunk
Are you sure you want to change the base?
Deprecate wp_enqueue_block_support_styles #4015
Conversation
Note: the failing PHP tests are expected here! Once the JS packages update has been completed, this PR should be rebased against
|
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 @andrewserong, PR looks good to me and ready for merge when the package updated. Great work!
Thank you, @andrewserong! The PR will need to be rebased after #3914 is merged to fix PHPUnit tests. |
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.
Thank you @andrewserong ! I tested here and verified that the deprecation message is shown. Also tested against current trunk with the packages update and there is no warning.
851c78c
to
a7aa10f
Compare
Update: even thought the tests are now passing, it looks like this PR is likely not going to be viable in time as a recently backported feature (#4013) appears to also use |
Thank you, @andrewserong! |
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.
There is an instance of this function being used in Core in _wp_add_block_level_preset_styles()
function https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-supports/settings.php#L139 (from changeset 55255).
This should also be changed if this function is to be deprecated.
@@ -2998,6 +2999,7 @@ function wp_enqueue_global_styles_css_custom_properties() { | |||
* @param int $priority To set the priority for the add_action. | |||
*/ | |||
function wp_enqueue_block_support_styles( $style, $priority = 10 ) { |
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 function should be moved to wp-includes/deprecated.php
.
@@ -2998,6 +2999,7 @@ function wp_enqueue_global_styles_css_custom_properties() { | |||
* @param int $priority To set the priority for the add_action. | |||
*/ | |||
function wp_enqueue_block_support_styles( $style, $priority = 10 ) { | |||
_deprecated_function( __FUNCTION__, '6.2.0', 'wp_style_engine_get_stylesheet_from_css_rules()' ); | |||
$action_hook_name = 'wp_footer'; |
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.
$action_hook_name = 'wp_footer'; | |
$action_hook_name = 'wp_footer'; |
Empty new line to separate the deprecation from the original function's code.
Thanks for reviewing @hellofromtonya — I think this PR is stalled right now due to #4013 landing after I opened this PR. I added a comment to that PR with some feedback about possible next steps: #4013 (comment). Given the timing, and that I don't think refactoring that usage would be very straightforward, I think we should probably park this deprecation proposal for the time being. I can close out this PR if it keeps things neater 🙂 |
@@ -2998,6 +2999,7 @@ function wp_enqueue_global_styles_css_custom_properties() { | |||
* @param int $priority To set the priority for the add_action. | |||
*/ | |||
function wp_enqueue_block_support_styles( $style, $priority = 10 ) { | |||
_deprecated_function( __FUNCTION__, '6.2.0', 'wp_style_engine_get_stylesheet_from_css_rules()' ); |
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.
_deprecated_function( __FUNCTION__, '6.2.0', 'wp_style_engine_get_stylesheet_from_css_rules()' ); | |
_deprecated_function( __FUNCTION__, '6.3.0', 'wp_style_engine_get_stylesheet_from_css_rules()' ); |
@@ -2988,6 +2988,7 @@ function wp_enqueue_global_styles_css_custom_properties() { | |||
* | |||
* @since 5.9.1 | |||
* @since 6.1.0 Added the `$priority` parameter. | |||
* @deprecated 6.2.0 Use wp_style_engine_get_stylesheet_from_css_rules() instead. |
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.
* @deprecated 6.2.0 Use wp_style_engine_get_stylesheet_from_css_rules() instead. | |
* @deprecated 6.3.0 Use wp_style_engine_get_stylesheet_from_css_rules() instead. |
In 6.2, once the JS packages have been updated, which includes changes to core blocks'
index.php
files, all usage in core ofwp_enqueue_block_support_styles
will have been removed. It should then be possible to deprecatewp_enqueue_block_support_styles
as a function.Since the style engine classes were added in 6.1, the style engine is now a better way to enqueue block support styles, as styles can be registered in multiple places, with those styles grouped together to be output only a single time, whereas
wp_enqueue_block_support_styles
resulted in multiple<style>
tags being output on the site frontend (one for each instance of a block support being rendered), along with redundant style output.Note: this PR should only land after the JS packages update that includes the changes to the gallery block, which roll in this PR: WordPress/gutenberg#43070. Without that, then you if a request a post or page containing a Gallery block, the following will be logged, as the gallery block in
trunk
currently still contains a call towp_enqueue_block_support_styles
:Trac ticket: https://core.trac.wordpress.org/ticket/57647
CC: @Mamaduka
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.