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

Fix duotone filters in classic themes #4839

Closed
wants to merge 2 commits into from
Closed

Fix duotone filters in classic themes #4839

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06ab3dc
Fix duotone filters in classic themes
ajlende Jul 12, 2023
75e3bc0
Register separate stylesheet for duotone
ajlende Jul 12, 2023
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
11 changes: 8 additions & 3 deletions src/wp-includes/class-wp-duotone.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1180,9 +1180,14 @@ public static function output_footer_assets() {
echo self::get_svg_definitions( self::$used_svg_filter_data );
}

// This is for classic themes - in block themes, the CSS is added in the head via wp_add_inline_style in the wp_enqueue_scripts action.
if ( ! wp_is_block_theme() && ! empty( self::$used_global_styles_presets ) ) {
wp_add_inline_style( 'core-block-supports', self::get_global_styles_presets( self::$used_global_styles_presets ) );
// In block themes, the CSS is added in the head via wp_add_inline_style in the wp_enqueue_scripts action.
if ( ! wp_is_block_theme() ) {
if ( ! empty( self::$used_global_styles_presets ) ) {
wp_add_inline_style( 'core-block-supports', self::get_global_styles_presets( self::$used_global_styles_presets ) );
}
if ( ! empty( self::$block_css_declarations ) ) {
wp_add_inline_style( 'core-block-supports', wp_style_engine_get_stylesheet_from_css_rules( self::$block_css_declarations ) );
}
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
if ( ! empty( self::$used_global_styles_presets ) ) {
wp_add_inline_style( 'core-block-supports', self::get_global_styles_presets( self::$used_global_styles_presets ) );
}
if ( ! empty( self::$block_css_declarations ) ) {
wp_add_inline_style( 'core-block-supports', wp_style_engine_get_stylesheet_from_css_rules( self::$block_css_declarations ) );
}
$handle = 'core-block-supports';
wp_register_style( $handle, false );
if ( ! empty( self::$used_global_styles_presets ) ) {
wp_add_inline_style( $handle, self::get_global_styles_presets( self::$used_global_styles_presets ) );
}
if ( ! empty( self::$block_css_declarations ) ) {
wp_add_inline_style( $handle, wp_style_engine_get_stylesheet_from_css_rules( self::$block_css_declarations ) );
}
wp_enqueue_style( $handle );

You can't add inline styles without registering and enqueuing style.

This fixes the issue in my testing.

Copy link
Author

@ajlende ajlende Jul 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I thought it was being registered already in script-loader, but it only looks like that happens when wp_style_engine_get_stylesheet_from_context returns something when SCRIPT_DEBUG is false.

wordpress-develop/src/wp-includes/script-loader.php

Lines 3049 to 3068 in cdf3ac0

$core_styles_keys = array( 'block-supports' );
$compiled_core_stylesheet = '';
$style_tag_id = 'core';
// Adds comment if code is prettified to identify core styles sections in debugging.
$should_prettify = isset( $options['prettify'] ) ? true === $options['prettify'] : defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG;
foreach ( $core_styles_keys as $style_key ) {
if ( $should_prettify ) {
$compiled_core_stylesheet .= "/**\n * Core styles: $style_key\n */\n";
}
// Chains core store ids to signify what the styles contain.
$style_tag_id .= '-' . $style_key;
$compiled_core_stylesheet .= wp_style_engine_get_stylesheet_from_context( $style_key, $options );
}
// Combines Core styles.
if ( ! empty( $compiled_core_stylesheet ) ) {
wp_register_style( $style_tag_id, false );
wp_add_inline_style( $style_tag_id, $compiled_core_stylesheet );
wp_enqueue_style( $style_tag_id );
}

Would it be okay to always register core-block-supports?

Copy link
Member

Choose a reason for hiding this comment

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

@ajlende Didn't know about that. You will need to enqueue it as well. Like this

         wp_register_style( $style_tag_id, false );
	// Combines Core styles.
	if ( ! empty( $compiled_core_stylesheet ) ) {
		wp_add_inline_style( $style_tag_id, $compiled_core_stylesheet );
	}
	wp_enqueue_style( $style_tag_id );

Copy link
Author

@ajlende ajlende Jul 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I just registered and enqueued a new stylesheet. The 'core-block-supports' name is semi-dynamic, so we'll run into issues in the future if someone tries to add to $core_styles_keys and doesn't update duotone.

It's unfortunate that the duotone block supports styles won't be grouped with the rest of block supports, but it's probably not that big of a deal.

}
}

Expand Down