WordPress / block-theme-examples

A repository of feature examples for block themes.
50 stars 7 forks source link

example-block-style-js doesn't display proper style in the block editor #9

Open juanmaguitar opened 10 months ago

juanmaguitar commented 10 months ago

There's an issue with the playground created for example-block-style-js that is a bit strange:

In the block editor, the image component with the "HaIn Drawn" block style look like this

Screenshot 2023-12-07 at 17 01 14

And it should have the same look in the editor than the example-block-style-php

Screenshot 2023-12-07 at 17 20 50

The issue with example-block-style-js is that is not loading style.css and it should according the the code at functions.php of that example

justintadlock commented 10 months ago

The change from wp_enqueue_scripts to enqueue_block_assets in #10 is not the correct fix. It may fix things in Playground, but it makes the example code incorrect.

In a normal WordPress install, we're now getting the same CSS (style.css) loading twice in the editor.

The problem from Playground stems from this code in functions.php:

add_action( 'after_setup_theme', 'themeslug_editor_styles' );

/**
 * Adds the `style.css` file as an editor stylesheet.
 *
 * @since 1.0.0
 */
function themeslug_editor_styles() {
    add_editor_style( get_stylesheet_uri() );
}

Why is Playground not loading the CSS for add_editor_style()? That's what needs to be addressed.

justintadlock commented 10 months ago

You can see the block style being loaded twice with the change in #10 (once in <head> and once via style.css):

block-style-twice

juanmaguitar commented 10 months ago

The change from wp_enqueue_scripts to enqueue_block_assets in https://github.com/wptrainingteam/block-theme-examples/pull/10 is not the correct fix. It may fix things in Playground, but it makes the example code incorrect. In a normal WordPress install, we're now getting the same CSS (style.css) loading twice in the editor.

Isn't enqueue_block_assets the recommended method to load assets in both the Editor (iframed) and the frontend from WP 6.3 as per Enqueueing assets in the Editor > Editor content scripts and styles

As of WordPress 6.3, all assets added through the enqueue_block_assets PHP action will also be enqueued in the iframed Editor. See https://github.com/WordPress/gutenberg/pull/48286 for more details (source: Enqueueing assets in the Editor)

justintadlock commented 10 months ago

add_editor_style() is the recommended method for themes to add editor stylesheets, and wp_enqueue_scripts + wp_enqueue_style() for the front end. This is the method outlined in the Including Assets Theme Handbook doc and is generally recommended by the WP Themes Team.

To be clear, we can use enqueue_block_assets (with some other changes in functions.php) to make this work with Playground. But that's only going to fix it for this specific example.

I'd rather make sure this is corrected in Playground if there's a bug. If Playground is not loading styles via add_editor_style(), that's going to be problematic for 100s or 1,000s of themes which use this method to load any editor styles. It's not just specific to this example. We definitely need some more testing of this

ndiego commented 10 months ago

add_editor_style() is the recommended method for themes to add editor stylesheets, and wp_enqueue_scripts + wp_enqueue_style() for the front end. This is the method outlined in the Including Assets Theme Handbook doc and is generally recommended by the WP Themes Team.

Yup, this is the recommended approach for themes. enqueue_block_assets is generally for plugins but does of course work in themes.

I wonder if playground is having trouble with get_stylesheet_uri()? 🤔

justintadlock commented 10 months ago

As far as I can tell, add_editor_style() is not loading stylesheets in Playground at all.

I've tried the standard get_stylesheet_uri():

add_action( 'after_setup_theme', 'themeslug_editor_styles' );

function themeslug_editor_styles() {
    add_editor_style( get_stylesheet_uri() );
}

I've tried get_theme_file_uri():

add_action( 'after_setup_theme', 'themeslug_editor_styles' );

function themeslug_editor_styles() {
    add_editor_style( get_theme_file_uri( 'style.css' ) );
}

And a relative URL (usually not recommended unless specifically overwriting a parent theme doing this):

add_action( 'after_setup_theme', 'themeslug_editor_styles' );

function themeslug_editor_styles() {
    add_editor_style( 'style.css' );
}

All three of these methods are acceptable in standard WP themes, but none of them seem to work with Playground.

juanmaguitar commented 10 months ago

Pinging @adamziel for visibility of this conversation I have created the issue in the Playground repo to keep track of this issue https://github.com/WordPress/wordpress-playground/issues/869

adamziel commented 9 months ago

I can't reproduce the issue, unfortunately.

This URL loads an editor with the "handdrawn" style applied for me:

https://playground.wordpress.net/?blueprint-url=https://raw.githubusercontent.com/wptrainingteam/block-theme-examples/master/example-block-style-js/_playground/blueprint.json

In a wp-env WordPress installation, the following plugin does not load mystyle.css for me in the post editor:

add_action( 'after_setup_theme', 'themeslug_editor_styles' );
function themeslug_editor_styles() {
    file_put_contents('/var/www/html/wp-content/themes/twentytwentythree/mystyle.css', 'body { background: red; }');
    add_editor_style(get_theme_file_uri('mystyle.css'));
}
juanmaguitar commented 9 months ago

@adamziel sorry about the confusion, let me clarify it. This issue was indeed fixed with this PR but the fix wasn't correct as explained by @justintadlock in this comment. The fix was causing the CSS to be loaded twice (see this comment)

I have updated the code to use wp_enqueue_scripts which is the recommended method as per this comment so right now the issue should be reproduced with the link https://playground.wordpress.net/?blueprint-url=https://raw.githubusercontent.com/wptrainingteam/block-theme-examples/master/example-block-style-js/_playground/blueprint.json

The issue in Playground seems to be with add_editor_style function as per this comment

adamziel commented 9 months ago

@juanmaguitar is that specific to Playground, though? I just installed that theme on my local WordPress installation and ran into the exact same problem there:

CleanShot 2024-01-11 at 12 29 06@2x
juanmaguitar commented 9 months ago

@adamziel you're right, I've tested this locally with wp-env

⬢  block-theme-examples  master ⦾ cd example-block-style-js
⬢  example-block-style-js  master ⦾ wp-env start
WordPress development site started at http://localhost:8888
WordPress test site started at http://localhost:8889
MySQL is listening on port 50938
MySQL for automated testing is listening on port 50937

 ✔ Done! (in 47s 618ms)

and it also doesn't work...

Screenshot 2024-01-11 at 12 34 41

@justintadlock can you provide more context about this issue?

justintadlock commented 9 months ago

The issue is that when using this code to load an editor stylesheet (as the theme is doing now):

add_action( 'after_setup_theme', 'themeslug_editor_styles' );

function themeslug_editor_styles() {
    add_editor_style( get_stylesheet_uri() );
}

It looks like this in Playground (because the stylesheet is not being loaded and inlined in the iframe <head>):

image

It looks like this on a normal WordPress site (the stylesheet is being correctly loaded and inlined in <head>):

image

That last one is from my local install, but this is also the method for loading editor styles for thousands of WordPress themes.

justintadlock commented 9 months ago

Or, if you want to test another theme, try Powder: https://playground.wordpress.net/?theme=powder

It uses add_editor_style() to load its editor stylesheet, which includes CSS for several block style variations.

Create a new page and insert a Group block. Add the "Shadow Solid" style to it. You should see that the box-shadow is missing:

image

The block should look like this (but the stylesheet is not loaded and inlined in the <head> on the editor side):

image

This only happens in Playground as far as I know. It works fine for me on a local test install.

adamziel commented 9 months ago

This is super weird @justintadlock! I just installed the powder theme in wp-env and I'm not getting that CSS loaded in my editor:

CleanShot 2024-01-11 at 16 10 03@2x
adamziel commented 9 months ago

It does seem to work on my WordPress.com site, though. How weird.

adamziel commented 9 months ago

add_editor_style() updates the global $editor_styles variable which is used in get_editor_stylesheets() which is only used in _WP_Editors::editor_settings() which is called in wp_tiny_mce() and in wp_editor(). All of this seems to be related to TinyMCE, not Gutenberg. I'm not sure what would render these styles in <head> 🤔

adamziel commented 9 months ago

On wp.com these styles are added via...

wp.domReady( function() {
    resolve(wp.editPost.initializeEditor('editor', "post", 1394, {
        "alignWide": false,
        "allowedBlockTypes": true,
        "styles": [
            { "css": "<!-- HERE -->" }
justintadlock commented 9 months ago

I don't use wp-env, so I hadn't tested with it.

Maybe this is a Gutenberg ticket since it seems to not be working in at least two environments? I'm not sure. At this point, it's a bit beyond me.

adamziel commented 9 months ago

wp-admin/edit-form-blocks.php:

$editor_settings = get_block_editor_settings($editor_settings, $block_editor_context);
$init_script = <<<JS
( function() {
    window._wpLoadBlockEditor = new Promise( function( resolve ) {
        wp.domReady( function() {
            resolve( wp.editPost.initializeEditor( 'editor', "%s", %d, %s, %s ) );
        } );
    } );
} )();
JS;
$script = sprintf($init_script, $post->post_type, $post->ID, wp_json_encode($editor_settings), 

get_block_editor_settings($editor_settings, $block_editor_context);:

$editor_settings['styles'] = array_merge($global_styles, get_block_editor_theme_styles());

Then get_block_editor_theme_styles():

function get_block_editor_theme_styles()
{
    global $editor_styles;
    $styles = array();
    if ($editor_styles && current_theme_supports('editor-styles')) {
        foreach ($editor_styles as $style) {
            if (preg_match('~^(https?:)?//~', $style)) {
                $response = wp_remote_get($style);
                if (!is_wp_error($response)) {
                    $styles[] = array('css' => wp_remote_retrieve_body($response), '__unstableType' => 'theme', 'isGlobalStyles' => false, );
                }
            } else {
                $file = get_theme_file_path($style);
                if (is_file($file)) {
                    $styles[] = array('css' => file_get_contents($file), 'baseURL' => get_theme_file_uri($style), '__unstableType' => 'theme', 'isGlobalStyles' => false, );
                }
            }
        }
    }
    return $styles;
}

So it uses the global $editor_styles; and does this:

And then it returns the retrieved CSS. How interesting!

I just tried enabling networking in Playground just to see if it would download that stylesheed and indeed it did:

CleanShot 2024-01-11 at 16 35 22@2x
justintadlock commented 9 months ago

I just tried enabling networking in Playground just to see if it would download that stylesheet and indeed it did:

Should we consider this fixed for Playground now? Will that setting continue to be enabled?


@juanmaguitar We might still need to address this with wp-env too since we're planning to ship .wp-env.json files with these. Is there a similar setting?

juanmaguitar commented 9 months ago

Pinging @mikachan for visibility on this issue as it seems to be beyond Playground. Any thoughts on why add_editor_style doesn't work on wp-env (see this)?

adamziel commented 9 months ago

@justintadlock oh I just checked the checkbox in my browser tab. To enable it in a Blueprint you'll need to add the following entry:

{
    "features": {
        "networking": true
    }
}

See also https://wordpress.github.io/wordpress-playground/blueprints-api/data-format#features

juanmaguitar commented 9 months ago

@adamziel @justintadlock I have updated the blueprint of example-block-style-js, adding

{
    "features": {
        "networking": true
    }
}

and regenerated the zip, and it works fine now ✅