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

added 'assets_callback' to 'register_block_type' function #5740

Closed
wants to merge 4 commits into from

Conversation

diegoliv
Copy link

Description

Like suggested in #5705, this PR adds assets_callback to the register_block_type function. With this, developers can access block attributes on enqueue_block_assets and enqueue_block_editor_assets action hooks and use them to do things like conditionally enqueue scripts / styles, add inline styles or scripts, etc.

An example of how to use this callback:

    function my_block_register_block(){
        register_block_type( 'namespace/block-example', array(
            'assets_callback' => 'my_block_do_assets',
        ) );
    }

    add_action( 'init', 'my_block_register_block' );

    function my_block_do_assets( $attributes ){
        // do whatever you need with the block attributes. For example, you can generate <style> tags, 
        // with wp_add_inline_style(), enqueue scripts / styles, or call wp_localize_script().
    }

##Tested with:

  • WordPress 4.9.4
  • Gutenberg 2.4.0
  • PHP 7.0.3 / nginx
  • MySQL 5.5.55

@aduth
Copy link
Member

aduth commented Mar 27, 2018

// do whatever you need with the block attributes. For example, you can generate <style> tags,
// with wp_add_inline_style(), enqueue scripts / styles, or call wp_localize_script().

What of these would not be possible with existing script, editor_script, style, and editor_style properties? (Reference)

@diegoliv
Copy link
Author

Hi @aduth, thanks for reviewing my PR! Currently, script, editor_script, style, and editor_style properties are handlers used only for enqueuing block assets (as described on the documentation you mentioned). That's fine and it is a good architecture. But you can't do much more than just enqueue these assets. You can only assign a style / script handler name to these properties. You can't for example, assign a callback function, to use functions like wp_localize_script or wp_add_inline_style.

Obviously you can just ignore these properties and use the existing hooks, enqueue_block_assets and enqueue_block_editor_assets, but why would I do that? Currently, when you're building a shortcode, for example, you can register a script on an init hook and then, inside the render function for the shortcode, you call wp_enqueue_script, to ensure that your script will only be enqueued if your shortcode is present on the content. But you can also, for example, inside this same render function, and after enqueueing the shortcode's script, call wp_localize_script and generate some variables based on the shortcode's attributes. That's the main keyword: attributes.

On enqueue_block_assets and enqueue_block_assets hooks, you can do whatever you want, but you don't have access to block attributes. So, if for example, I want to generate some custom styles for a block based on its attributes, you can't. The render_callback is similar to a render function for a shortcode, and you do have access to blocks attributes there, but there are some downsides (based on my own research while building some custom blocks) of using it for this purpose:

  1. Some functions may not work properly inside render_callback. On my own tests, using wp_add_inline_style with an existing style handler didn't worked and the <style> tag that should show up was missing.
  2. By using render_callback for the single purpose of manipulate blocks attributes to generate assets or styles / scripts, you are enforced to use this PHP callback to render your block instead of using javascript.

My PR with the assets_callback solves both problems. You have access to attributes inside the proper hooks, methods like wp_add_inline_style works properly, and you have a separation of concerns with a callback for rendering the block html, and another one to "render" / generate blocks styles / scripts. The main goal of this PR is to provide a viable solution if you don't want to inline specific block styles / js variables. For example, by using wp_add_inline_style along with blocks attributes, you can properly generate <style> tags for your block, instead of inlining the css inside html tags.

@danielbachhuber danielbachhuber self-requested a review March 30, 2018 13:07
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

For us to better understand how this is meant to be used, can you provide a real world example and its existing alternative?

@danielbachhuber danielbachhuber added the [Feature] Extensibility The ability to extend blocks or the editing experience label Mar 30, 2018
@aduth
Copy link
Member

aduth commented Mar 30, 2018

wp_localize_script or wp_add_inline_style are sufficient for static additions to script styles and handles.

The compelling bit for me is attribute-dependent additions. I could imagine this being useful, though I'd agree some real world use-cases may be valuable to have.

@diegoliv
Copy link
Author

diegoliv commented Apr 3, 2018

Ok, guys, to better explain how the assets_callback is useful, I'll give a real example. Currently, I'm working on a set of custom blocks. One of them is a slideshow block.

On this block, you select a set of images from your media library, and then you can customize the pieces that compose the slideshow. One of these pieces is the navigational arrows:

gutenberg_slideshow_module

For the navigational arrows, I added options for:

  • Arrow size
  • Arrow color
  • Arrow background color
  • Arrow background opacity
  • Arrow background roundness

Each one of these options is an attribute. All of them are the kind of attributes that you can't set using css classes. You have to either generate the css to apply these attributes, or inline everything. So here is the implementation of these attributes with the current state of Gutenberg, and with the assets_callback:

Current State

The only way to add styles like color, background color and opacity is through inline styles. So you have to generate the html markup with the styles applied. For the arrow background, if you want to add a semi-transparent background, you need to create an html element only for that (if #5835 is accepted and merged, then this extra element would not be necessary anymore).

On your block's JavaScript file, the save callback should return an output similar to this:

<div class="wp-block-slideshow">
    <div class="slideshow-container">
        <div class="slideshow-wrapper">
            <div class="slideshow-slide">
                <img src="(...)" alt="">
            </div>
            <div class="slideshow-slide">
                <img src="(...)" alt="">
            </div>
        </div>
        <div class="slideshow-arrows">
            <div class="slideshow-button-prev">
                <span class="slideshow-arrow" style="width: 95px; height: 95px; color: #ffffff;">
                    <!-- extra element because this is the only way to do a semi-transparent background right now -->
                    <span class="arrow-bg" style="background-color: #340087; opacity: 0.7; border-radius: 47.5px;"></span>
                    <svg width="500" height="500" viewBox="0 0 500 500" class="arrow-icon-prev">(...)</svg>
                </span>
            </div>
            <div class="slideshow-button-next">
                <span class="slideshow-arrow" style="width: 95px; height: 95px; color: #ffffff;">
                    <!-- extra element because this is the only way to do a semi-transparent background right now -->
                    <span class="arrow-bg" style="background-color: #340087; opacity: 0.7; border-radius: 47.5px;"></span>
                    <svg width="500" height="500" viewBox="0 0 500 500" class="arrow-icon-next">(...)</svg>
                </span>
            </div>
        </div>
    </div>
</div>

So all css styles are inline, and the .arrow-bg element is created to generate the background with the opacity properly set.

With assets_callback

By adding the assets_callback to the register_block_type function, we can simplify our html output and remove unnecessary tags. So our output may look like this:

<div class="wp-block-slideshow">
    <div class="slideshow-container">
        <div class="slideshow-wrapper">
            <div class="slideshow-slide">
                <img src="(...)" alt="">
            </div>
            <div class="slideshow-slide">
                <img src="(...)" alt="">
            </div>
        </div>
        <div class="slideshow-arrows">
            <div class="slideshow-button-prev">
                <span class="slideshow-arrow">
                    <svg width="500" height="500" viewBox="0 0 500 500" class="arrow-icon-prev">(...)</svg>
                </span>
            </div>
            <div class="slideshow-button-next">
                <span class="slideshow-arrow">
                    <svg width="500" height="500" viewBox="0 0 500 500" class="arrow-icon-prev">(...)</svg>
                </span>
            </div>
        </div>
    </div>
</div>

And then, with the register_block_type function, we can do this:

<?php

function register_slideshow(){
    register_block_type( 'slideshow', array(
        'editor_script' => 'slideshow-block-js',
        'frontend_script' => 'slideshow-frontend',
        // 'style' => 'slideshow-block',
        'assets_callback' => 'generate_slideshow_styles',
    ) );
}

function generate_slideshow_styles( $attr ){
    wp_enqueue_style( 'slideshow-block' ); //for example purposes let's assume that the block's frontend css file is already registered

    // now we generate the styles for wp_add_inline_style
    $styles = array();
    $id_selector = '[class*="'. $attr['blockID'] .'"]'; // I stored the block ID into an attribute so we can use it to identify the block instance on the frontend

    if( isset( $attr['arrowSize'] ) ){
        $styles[ $id_selector . '.wp-block-slideshow .slideshow-arrow' ]['width'] = $attr['arrowSize'] . 'px';
        $styles[ $id_selector . '.wp-block-slideshow .slideshow-arrow' ]['height'] = $attr['arrowSize'] . 'px';
    }
    if( $attr['arrowColor'] ){
        $styles[ $id_selector . '.wp-block-slideshow .slideshow-arrow' ]['color'] = $attr['arrowColor'];
    }
    // since we can generate our own css rules, we can target pseudo-elements as well
    if( $attr['arrowBgColor'] ){
        $styles[ $id_selector . '.wp-block-slideshow .slideshow-arrow:before' ]['background-color'] = $attr['arrowBgColor'];
    }
    if( $attr['arrowBgOpacity'] ){
        $styles[ $id_selector . '.wp-block-slideshow .slideshow-arrow:before' ]['opacity'] = $attr['arrowBgOpacity'] / 100;
    }

    // map styles for selector
    $mapped = array();
    foreach( $styles as $selector => $props ){
        $css = $selector . '{';
        foreach( $props as $prop => $value ){
            $css .= $prop .': ' . $value . ';';
        }
        $css .= '}';

        $mapped[] = $css;
    }

    if( !empty( $mapped ) ){
        $final_styles = implode( ' ', $mapped );
        wp_add_inline_style( 'slideshow-block', $final_styles );                    
    }
}

add_action( 'init', 'register_slideshow' );

By using the assets_callback, we can do two things:

  1. Keep our markup only for content structure and move all attribute based styles into it's own <style> tag
  2. Since we're free to manipulate the attributes, we can replace unnecessary elements by targeting pseudo elements like :before and :after.

You can also conditionally load other assets based on attributes. Let's suppose that my slideshow module has it's main JavaScript library that adds all the base functionality. An then I have some addons / modules for extra functionality like slideshow thumbnails or zooming into the current slide. You can create an attribute to enable / disable these features, and based on that attribute, you can enqueue the secondary assets.

I hope this example helps to clarify why this PR can be useful. Thoughts?

@mcsf
Copy link
Contributor

mcsf commented Apr 10, 2018

Hi, @diegoliv, thanks for your contribution.

A couple of points:

  • This is my own assessment and not necessarily Daniel's or Andrew's, but I don't think this use case is compelling enough to justify the added interface complexity and render-time burden:
    • For one, this is a very specific kind of optimization; that is, it's not a block-level optimization like async asset loading, but rather an optimization on the amount of styles imported or inlined. Notably, it's not a blocker for the intended plugin, even if it may be less convenient, and the optimization yield isn't expected to be significant (from what I can see, at least).
    • Secondly, nascent Web standards prove quite handy. In this instance, I'm looking at CSS variables and scoping. With them, one can define default values by way of CSS variables in the block type's actual stylesheet, and override them with block-specific values through inline styles during the render_callback call. Here is a concrete jsfiddle proof of concept. Depending on your user base and intended browser support, your could either use this today or very soon. :)
  • More generally, if you're curious:
    • Block-level front-end asset loading is being explored in Bundling Front-end Assets #5445.
    • There are priors for block-level (not attribute-dependent) async asset loading in the Custom HTML block, though there are many kinks to work out there, and thus that block type shouldn't serve as example yet.

@diegoliv
Copy link
Author

@mcsf, thanks for your comment! Here are some considerations:

For one, this is a very specific kind of optimization; that is, it's not a block-level optimization like async asset loading, but rather an optimization on the amount of styles imported or inlined. Notably, it's not a blocker for the intended plugin, even if it may be less convenient, and the optimization yield isn't expected to be significant (from what I can see, at least).

The example provided is just one example of what you can do when you have access to blocks attributes inside the right action hook. My point of not using inline styles is not about being convenient. It's more about best practices and flexibility. The decrease of the amount of styles inlined is just a plus. My point is that:

  1. Inline styles are not a best practice and should be used wisely. I think that this article on Codecademy is a good one to clarify when inline styles are considered good or bad practice.
  2. It is ok, and probably the only way to go, to have inline styles inside Gutenberg. But by saying Gutenberg I mean the editor interface. Since we're live previewing our changes, it is ok and natural to use inline styles inside a React component. But on the frontend, there's no real compelling reason to favor inline styles over an external stylesheet (or even a <style> tag with CSS rules).
  3. There are some things that you can't do using inline styles. For example, how can you target a pseudo element with inline styles? With CSS classes you can easily use :before or :after.
  4. CSS is only one side of the coin. You can also use these attributes with JavaScript.
  5. Even if my personal opinion is that inline styles are not a good practice, I'm not against using them on the frontend. What I'm against is to not provide a more flexible approach to attribute based styles / scripts if we can.

Secondly, nascent Web standards prove quite handy. In this instance, I'm looking at CSS variables and scoping. With them, one can define default values by way of CSS variables in the block type's actual stylesheet, and override them with block-specific values through inline styles during the render_callback call. Here is a concrete jsfiddle proof of concept. Depending on your user base and intended browser support, your could either use this today or very soon. :)

The scoped attribute is currently obsolete and seems like will eventually be deprecated. But dude, I really like CSS variables! They're really cool, and your jsfiddle is a good example on how they can be handy. But one of the downsides is that they're not supported on IE. Of course, the current support is pretty good. But still, like you said, depending on your user base, it might be a good fit or not. If I have a the option to style my block and at the same time support all browsers, even some old versions, I think that this can be a better fit.

and override them with block-specific values through inline styles during the render_callback call.

Yes, you can use the render_callback call to do some stuff, but like I said on one of my comments, if you use it, then you're forced to use this callback to render your whole block. This means that the output will be always dynamically generated. If I just use the save callback on the registerBlockType function, The html output of my block will be stored on the post content and will be just static HTML, which is faster than a generated output done with PHP. With assets_callback you have a clear separation of concerns. You can use whatever you like to generate the output of your block. The block styles (or scripts) can be independent of how you render your block.

With all of that said, I took a look on #5445 and I really like the idea of dynamic bundles. One way of doing it is not only bundling CSS assets for all blocks on a page, but we could also add to the bundle some attribute based styles generated for a block. This is a approach similar to what Beaver Builder does. On BB, for a custom module, there's a php file where you can use a module's attributes to generate CSS. This CSS is then bundled with all of the generated CSS styles from the modules used on the page, and then this bundle is served on a single CSS file enqueued for that specific page. If we could do something like that, it would be awesome.

@danielbachhuber
Copy link
Member

Hi @diegoliv,

Thanks for all of your work on this pull request. For now, I'd suggest we close it, but keep the issue open, and revisit the conversation in a couple of months once we've seen Gutenberg in the wild a little bit longer. I think time will help guide a better decision on this.

@gziolo
Copy link
Member

gziolo commented Apr 20, 2018

Closing this in favor of #5445 and related PRs to make sure we don' duplicate the same discussions. @diegoliv thanks for opening this PR, it was very helpful in the context of the work in other PRs 🙇

@gziolo gziolo closed this Apr 20, 2018
@prajaktagadhave
Copy link

any reason why 'assets_callback' not working in 'register_block_type' ?

@mcsf
Copy link
Contributor

mcsf commented Sep 17, 2018

@prajaktagadhave: This pull request was closed without merging, meaning its changes were not effected. See #5445 for a broader issue on the topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants