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.

Sign up for GitHub

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

Load block support styles in the head for block themes #2323

Closed
wants to merge 1 commit into from
Closed

Load block support styles in the head for block themes #2323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions src/wp-includes/block-supports/elements.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function wp_render_elements_support( $block_content, $block ) {
}
$link_color_declaration = esc_html( safecss_filter_attr( "color: $link_color" ) );

$style = "<style>.$class_name a{" . $link_color_declaration . ";}</style>\n";
$style = ".$class_name a{" . $link_color_declaration . ';}';

// Like the layout hook this assumes the hook only applies to blocks with a single wrapper.
// Retrieve the opening tag of the first HTML element.
Expand All @@ -68,17 +68,7 @@ function wp_render_elements_support( $block_content, $block ) {
$content = substr_replace( $block_content, ' class="' . $class_name . '"', $first_element_offset + strlen( $first_element ) - 1, 0 );
}

/*
* Ideally styles should be loaded in the head, but blocks may be parsed
* after that, so loading in the footer for now.
* See https://core.trac.wordpress.org/ticket/53494.
*/
add_action(
'wp_footer',
static function () use ( $style ) {
echo $style;
}
);
wp_enqueue_block_support( $style );

return $content;
}
Expand Down
12 changes: 1 addition & 11 deletions src/wp-includes/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,17 +175,7 @@ function wp_render_layout_support_flag( $block_content, $block ) {
1
);

/*
* Ideally styles should be loaded in the head, but blocks may be parsed
* after that, so loading in the footer for now.
* See https://core.trac.wordpress.org/ticket/53494.
*/
add_action(
'wp_footer',
static function () use ( $style ) {
echo '<style>' . $style . '</style>';
}
);
wp_enqueue_block_support( $style );

return $content;
}
Expand Down
26 changes: 26 additions & 0 deletions src/wp-includes/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1332,3 +1332,29 @@ function _wp_multiple_block_styles( $metadata ) {
return $metadata;
}
add_filter( 'block_type_metadata', '_wp_multiple_block_styles' );

/**
* This function takes care of adding inline styles
* in the proper place, depending on the theme in use.
*
* For block themes, it's loaded in the head.
* For classic ones, it's loaded in the body
* because the wp_head action (and wp_enqueue_scripts)
* happens before the render_block.
*
* See https://core.trac.wordpress.org/ticket/53494.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs @since annotation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* See https://core.trac.wordpress.org/ticket/53494.
* @link https://core.trac.wordpress.org/ticket/53494.

*
* @param string $style String containing the CSS styles to be added.
*/
function wp_enqueue_block_support( $style ) {
Copy link
Contributor

@hellofromtonya hellofromtonya Feb 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The function name does not fully identify what happens when this function is invoked. Its purpose is to enqueue styles rather than "support". How about wp_enqueue_inline_block_styles ? @oandregal what do you think?

  2. Let's move this function to wp-includes/script-loader.php where like functions exist.

  3. Nitpick: @since annonation is needed (though can be added by core committer in prepping the commit.

Copy link
Member Author

@oandregal oandregal Feb 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Naming. I'd like to retain the block_support if possible, perhaps wp_enqueue_block_support_styles?
  2. Moving: I'm fine with that place if folks are opinionated about that. I also wanted to share more of the rationale as to why blocks.php sounds like a good place as well: this is where other functions related to blocks live, such as wp_enqueue_block_style or wp_migrate_old_typography_shape that deal with block things (styles, block supports, etc).
  3. Sorry I've missed the @since tag.

$action_hook_name = 'wp_footer';
if ( wp_is_block_theme() ) {
$action_hook_name = 'wp_enqueue_scripts';
}
add_action(
$action_hook_name,
function () use ( $style ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function () use ( $style ) {
static function () use ( $style ) {

Tiny performance boost where PHP creates the embedded function once as a static instead of it being created each time the parent function is invoked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can someone unhook this function if desired?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This same problem exists elsewhere too. Here's the ticket for that work https://core.trac.wordpress.org/ticket/54028.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also preexisting to this PR, note that this focused on fixing a bug with block themes (and moved this code around for convenience). I didn't have the context on why an anonymous function was used, so I didn't want to mix things. That can be updated separately.

echo "<style>$style</style>\n";
}
);
}