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.

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

Global Styles: Fix for third-party blocks #3529

Closed
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
1 change: 1 addition & 0 deletions phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@
<element value="WP_Import_UnitTestCase"/>
<element value="WP_Test_Adjacent_Image_Link_TestCase"/>
<element value="WP_Tests_Image_Resize_UnitTestCase"/>
<element value="WP_Theme_UnitTestCase"/>

<!-- Mock classes. -->
<element value="Spy_REST_Server"/>
Expand Down
15 changes: 11 additions & 4 deletions src/wp-includes/global-styles-and-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,18 @@ function wp_add_global_styles_for_blocks() {
continue;
}

$stylesheet_handle = 'global-styles';
if ( isset( $metadata['name'] ) ) {
$block_name = str_replace( 'core/', '', $metadata['name'] );
/*
* These block styles are added on block_render.
* This hooks inline CSS to them so that they are loaded conditionally
* based on whether or not the block is used on the page.
*/
wp_add_inline_style( 'wp-block-' . $block_name, $block_css );
if ( str_starts_with( $metadata['name'], 'core/' ) ) {
$block_name = str_replace( 'core/', '', $metadata['name'] );
$stylesheet_handle = 'wp-block-' . $block_name;
}
wp_add_inline_style( $stylesheet_handle, $block_css );
}

// The likes of block element styles from theme.json do not have $metadata['name'] set.
Expand All @@ -242,8 +246,11 @@ function ( $item ) {
)
);
if ( isset( $result[0] ) ) {
$block_name = str_replace( 'core/', '', $result[0] );
wp_add_inline_style( 'wp-block-' . $block_name, $block_css );
if ( str_starts_with( $result[0], 'core/' ) ) {
$block_name = str_replace( 'core/', '', $result[0] );
$stylesheet_handle = 'wp-block-' . $block_name;
}
wp_add_inline_style( $stylesheet_handle, $block_css );
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions tests/phpunit/data/themedir1/block-theme/theme.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@
"filter": {
"duotone": "var(--wp--preset--duotone--custom-duotone)"
}
},
"my/third-party-block": {
Copy link
Member

@oandregal oandregal Oct 27, 2022

Choose a reason for hiding this comment

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

These styles will be ignored by any other tests because the block my/third-party-block won't be registered.

"color": {
"background": "hotpink"
}
}
},
"elements": {
Expand Down
66 changes: 66 additions & 0 deletions tests/phpunit/tests/theme/base.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

abstract class WP_Theme_UnitTestCase extends WP_UnitTestCase {
Copy link
Contributor

@hellofromtonya hellofromtonya Oct 27, 2022

Choose a reason for hiding this comment

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

Why add this base test case? Test classes in this group are repeating the same code. This base abstract test case encapsulates the repeating code, removing it from the test class itself. It's the starting point for improving the other tests in this same group.

Why in this PR? This "repeating set up and tear down code" is copied from the other tests in the same group. This means it's not part of the code review for the scope of this PR. By moving this copied logic to a base test case, it's signaling to focus the review on the new test class, not this copied setup/teardown code.

Note: A follow-up PR will address the other tests in this group.


/**
* Theme root directory.
*
* @var string
*/
private $theme_root;

/**
* Original theme directory.
*
* @var string
*/
private $orig_theme_dir;

public function set_up() {
parent::set_up();

$this->orig_theme_dir = $GLOBALS['wp_theme_directories'];
$this->theme_root = realpath( DIR_TESTDATA . '/themedir1' );

// /themes is necessary as theme.php functions assume /themes is the root if there is only one root.
$GLOBALS['wp_theme_directories'] = array( WP_CONTENT_DIR . '/themes', $this->theme_root );

// Set up the new root.
add_filter( 'theme_root', array( $this, 'filter_set_theme_root' ) );
add_filter( 'stylesheet_root', array( $this, 'filter_set_theme_root' ) );
add_filter( 'template_root', array( $this, 'filter_set_theme_root' ) );

// Clear caches.
wp_clean_themes_cache();
unset( $GLOBALS['wp_themes'] );
}

public function tear_down() {
$GLOBALS['wp_theme_directories'] = $this->orig_theme_dir;

// Clear up the filters to modify the theme root.
remove_filter( 'theme_root', array( $this, 'filter_set_theme_root' ) );
remove_filter( 'stylesheet_root', array( $this, 'filter_set_theme_root' ) );
remove_filter( 'template_root', array( $this, 'filter_set_theme_root' ) );

wp_clean_themes_cache();
unset( $GLOBALS['wp_themes'] );

parent::tear_down();
}

/**
* Cleans up global scope.
*
* @global WP_Styles $wp_styles
*/
public function clean_up_global_scope() {
global $wp_styles;
parent::clean_up_global_scope();
$wp_styles = null;
}

public function filter_set_theme_root() {
return $this->theme_root;
}
}
130 changes: 130 additions & 0 deletions tests/phpunit/tests/theme/wpAddGlobalStylesForBlocks.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
<?php

require_once __DIR__ . '/base.php';

/**
* Tests wp_add_global_styles_for_blocks().
*
* @group themes
*
* @covers ::wp_add_global_styles_for_blocks
*/
class Tests_Theme_WpAddGlobalStylesForBlocks extends WP_Theme_UnitTestCase {

/**
* Test blocks to unregister at cleanup.
*
* @var array
*/
private $test_blocks = array();

public function tear_down() {
// Unregister test blocks.
if ( ! empty( $this->test_blocks ) ) {
foreach ( $this->test_blocks as $test_block ) {
unregister_block_type( $test_block );
}
$this->test_blocks = array();
}

parent::tear_down();
}

/**
* @ticket 56915
*/
public function test_third_party_blocks_inline_styles_not_register_to_global_styles() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this be

Suggested change
public function test_third_party_blocks_inline_styles_not_register_to_global_styles() {
public function test_third_party_blocks_inline_styles_not_registered_to_global_styles() {

?

switch_theme( 'block-theme' );

wp_register_style( 'global-styles', false, array(), true, true );
wp_add_global_styles_for_blocks();

$this->assertNotContains(
'.wp-block-my-third-party-block{background-color: hotpink;}',
$this->get_global_styles()
);
}

/**
* @ticket 56915
*/
public function test_third_party_blocks_inline_styles_get_registered_to_global_styles() {
$this->set_up_third_party_block();

wp_register_style( 'global-styles', false, array(), true, true );

$this->assertNotContains(
'.wp-block-my-third-party-block{background-color: hotpink;}',
$this->get_global_styles(),
'Third party block inline style should not be registered before running wp_add_global_styles_for_blocks()'
);

wp_add_global_styles_for_blocks();

$this->assertContains(
'.wp-block-my-third-party-block{background-color: hotpink;}',
$this->get_global_styles(),
'Third party block inline style should be registered after running wp_add_global_styles_for_blocks()'
);
}

/**
* @ticket 56915
*/
public function test_third_party_blocks_inline_styles_get_registered_to_global_styles_when_per_block() {
$this->set_up_third_party_block();
add_filter( 'should_load_separate_core_block_assets', '__return_true' );

wp_register_style( 'global-styles', false, array(), true, true );

$this->assertNotContains(
'.wp-block-my-third-party-block{background-color: hotpink;}',
$this->get_global_styles(),
'Third party block inline style should not be registered before running wp_add_global_styles_for_blocks()'
);

wp_add_global_styles_for_blocks();

$this->assertContains(
'.wp-block-my-third-party-block{background-color: hotpink;}',
$this->get_global_styles(),
'Third party block inline style should be registered after running wp_add_global_styles_for_blocks()'
);
}

/**
* @ticket 56915
*/
public function test_third_party_blocks_inline_styles_get_rendered() {
$this->set_up_third_party_block();
add_filter( 'should_load_separate_core_block_assets', '__return_true' );

wp_register_style( 'global-styles', false, array(), true, true );
wp_enqueue_style( 'global-styles' );
wp_add_global_styles_for_blocks();

$this->assertStringContainsString(
'.wp-block-my-third-party-block{background-color: hotpink;}',
get_echo( 'wp_print_styles' )
);
}

private function set_up_third_party_block() {
switch_theme( 'block-theme' );

$name = 'my/third-party-block';
$settings = array(
'icon' => 'text',
'category' => 'common',
'render_callback' => 'foo',
);
register_block_type( $name, $settings );

$this->test_blocks[] = $name;
}

private function get_global_styles() {
$actual = wp_styles()->get_data( 'global-styles', 'after' );
return is_array( $actual ) ? $actual : array();
}
}