search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.55k stars 4.22k forks source link

Move away from inline styles #23893

Open afercia opened 4 years ago

afercia commented 4 years ago

Describe the bug Splitting this out from https://github.com/WordPress/gutenberg/pull/20775#issuecomment-657231556

Some of the blocks custom values are saved as inline CSS styles. For example:

Inline styles are permanently stored in the HTML blob in the database. This breaks the principle of separation of content from presentation.

It also breaks interoperability of the data: exporting the content for reuse in other applications will bring all these presentational values in the content, where they would likely break the presentation. This brings us back to 15 years ago when web content was often an inextricable "HTML soup" of real content and presentation.

Admittedly, this is not new to the block editor. But, instead of moving away from inline styles, I see the block editor keeps using inline styles like in the case of the new "Line height" control. I think the block editor is doing this maybe a bit too lightly, without a in depth consideration of the final result for the web as a whole.

Instead of more inline styles, I'd expect Gutenberg to explore new solutions to make presentational values be stored separately from content.

Worth also considering that, as of July 2020, WordPress is powering 37.8% of the top 10 million websites in the world. This means the block editor is actually polluting the content of 37.8% of the websites with presentational data. Not a great progress for the Web overall. I'd like the block editor team to be more aware of this problem and feel a stronger responsibility to make the Web better.

WordPress should lead by example and fully embrace Web Standards and best practices.

I don't have a solution, but i strongly feel this should be one of the top priorities for the next releases. Hopefully, this issue can help to start a broader conversation and explore solutions.

afercia commented 4 years ago

Example of inline style rendered in the front end content:

<p style="line-height:1.5;font-size:20px"> ... </p>

Example of HTML blob saved in the database:

<!-- wp:paragraph {"style":{"typography":{"fontSize":20,"lineHeight":"1.5"}}} -->
<p style="line-height:1.5;font-size:20px"> ... </p>
<!-- /wp:paragraph -->
ellenbauer commented 4 years ago

I experience the same issues with inline styles added by the blocks, which make it difficult to add my own CSS styles to e.g. a theme.

For instance as mentioned above, the font-size should not be included as an inline style in the editor, so theme authors are able to provide their own CSS styling for the custom font sizes. At the moment the inline style overrides my own CSS styles in the editor.

If inline style is added to the frontend this makes it even more difficult and often overwrites custom responsive CSS styles I want to provided via the theme.

mtias commented 4 years ago

This is the tip of a very large problem space that continues to be discussed at length and is definitely not taken lightly! To put it short, style attributes need to be defined in a way that:

Please, read #20331 and #22296 for more context on the full style project if necessary. Particularly the item about "Look at style variations as packages of style attributes" aims at turning what is currently expressed through inline attributes into an encapsulated class affecting those attributes and hooked to that specific block. This requires several layers to be implemented and defined before it can start working in unison, though. In the mean time, some block tools fallback to inline styles when the values are custom.

In terms of the vertical scope, these attributes need to be able to affect the global space (theme defaults), the block area space (a sidebar might need to be able to redefine the default attributes), and the block space (a block needs to be able to depart). The aim is for this to work with native web tools as much as possible — at least on the output — for these style contexts, likely through pairing of classes and CSS variables to achieve all the different requirements at render time and keep concerns as separated as possible. However, this all requires a comprehensive system to work effectively, one that themes can opt into and integrate properly, otherwise specificity battles are going to be extremely fragile and seemingly broken in terms of user experience.

In your example above, the "style" object specified in the comment is the seed of that encapsulation, as we'll be able to store it separately and only reference it through a class or style id, which needs to be printed at time of render in a style block or separate stylesheet. This could also help cut down on amount of CSS delivered to the user as it can be determined ahead of time what needs to be present on the page load.

afercia commented 4 years ago

aims at turning what is currently expressed through inline attributes into an encapsulated class affecting those attributes and hooked to that specific block.

Glad to hear there are plans to not use inline styles in the long term.

I created this issue to mainly focus on the architectural side. While inline styles certainly affect themes etc. and I do understand all the above considerations, I do believe the fact the block editor is polluting the content of 37.8% of the web (okay, "the 37.8% of the top 10 million websites", to be more exact) is way more important.

In my opinion, this issue should be one of the top priorities of the next release. It's already more than one year that the block editor is spreading inline styles all around the web.

ZebulanStanphill commented 4 years ago

I think the way that a lot of stuff should be handled is by allowing themes (and/or users) to create presets at a global level.

Here's one specific example I have in mind:

Users/themes would be able to create a font size + line height preset.

This user/theme-defined font size preset would then appear in the font size select dropdowns in the inspector for various blocks like the Paragraph block. (By default, the Custom option would not be available.)

I think line height should be directly tied to font size, so when creating a font size preset, you would also set the associated line height. There would not be separate line height presets or controls. Both the font size and line height would be applied by the same CSS class assigned to the preset. (I guess something like is-font-size-PRESET-NAME.)

This would help ensure consistent theming across the entire website. People would be encouraged to create reusable solutions, rather than one-off band-aid fixes that would result in tons of inline styles.

aristath commented 4 years ago

Just thinking out loud here, but stripping inline styles, putting them all together and then printiing them in the footer wouldn't be that hard to do...

As a proof of concept here's a simple class that will do just that. (If you remove all the inline comments it's less than 30 lines and pretty simple):

class GutenStylesConcat {

    /**
     * An array of styles.
     * 
     * @access protected
     * @var array
     */
    protected $styles = [];

    /**
     * Run our filters & actions.
     * 
     * @access public
     * @return void
     */
    public function init() {
        // Filter block content.
        add_filter( 'render_block', [ $this, 'render_block' ], 10, 2 );

        // Print all block styles in the footer.
        add_action( 'wp_footer', [ $this, 'print_styles' ] );
    }

    /**
     * Filter the block content.
     * 
     * @access public
     * @param string $block_content The block content.
     * @param array  $block         The block definition.
     * @return string               Returns the $block_content with nline styles stripped.
     */
    public function render_block( $block_content, $block ) {

        // Find instances of inline styles.
        preg_match( '/style=\"[^\"]*\"/', $block_content, $matches );

        // Loop matches.
        foreach ( $matches as $match ) {
            $styles = rtrim( ltrim( $match, 'style="' ), '"' );
            $key    = md5( $styles );

            // Replace the "style" attribute with a custom "block-style".
            $block_content = str_replace( $match, 'block-style="' . $key . '"', $block_content );

            // Set the style in the object's array so we can later print all styles in the footer.
            $this->styles[ $key ] = $styles;
        }

        // Return the content.
        return $block_content;
    }

    /**
     * Print block styles.
     * 
     * @access public
     * @return void
     */
    public function print_styles() {
        // Early exit if there are no inline styles.
        if ( empty( $this->styles ) ) {
            return;
        }

        echo '<style id="block-inline-styles">';
        foreach ( $this->styles as $key => $style ) {
            echo '[block-style="' . $key . '"]{' . $style . '}';
        }
        echo '</style>';
    }
}

$styles_concat = new GutenStylesConcat();
$styles_concat->init();

The above code does the following:

It's a simple approach and in principle it works perfectly fine. One extra benefit is that if there are multiple blocks with the same styles, these styles will only be printed once since their md5 key will be the same.

The md5 thing is ugly and will generate something like <p block-style="3dd9b4c541ac60568b77fd9c54f995b3">, but for the purposes of a proof-of-concept it's good enough.

EDIT: We could probably use hash( 'crc32', $styles ) instead of md5( $styles )... The result would be significantly shorter keys and I believe they'd still be unique

kanlukasz commented 4 years ago

I have a similar case where I have my own custom blocks. There is a lot of inline styles (because they come from block components like RangeControl etc.)

Right now I am curious which method will be the fastest:

  1. Using render_block filter as shown by @aristath

or

  1. Using inside render_callback something like:

    `wp_register_style('dummy-handle-md5', false);`
`wp_enqueue_style('dummy-handle-md5');`
`wp_add_inline_style('dummy-handle-md5', $build_class);`

Another thing is that both methods generate classes in the footer. It would be much better if it happened in the head

Tropicalista commented 3 years ago

@aristath This code does not solve the problem of inline hover or pseudoclass. A more generic interface must be provided..

aristath commented 3 years ago

@aristath This code does not solve the problem of inline hover or pseudoclass. A more generic interface must be provided..

Yes, that code was a simple proof of concept, something to show that it is doable. If it is decided that we should move away from inline styles, then a more robust and abstract solution will need to be implemented.

caseymilne commented 2 years ago

Is there a filter we can use in a custom theme to stop the inline styles from being rendered?

I'm building a custom theme with Tailwinds, and for some blocks we can add Tailwinds CSS classes under the block advanced and this works. However the aggressive inline styling of columns for instance specifies .column { margin: 0; } which makes using .mx-auto (left/right margin auto) impossible.

huubl commented 1 year ago

Are there any decisions/updates about removing inline styles on attributes?

Besides the arguments already mentioned. Right now it's very hard, nearly impossible, to add a strict Content Security Policy (CSP) on inline styles on attributes (https://content-security-policy.com/examples/allow-inline-style/).

cbirdsong commented 1 year ago

Short version - no. In one of the Slack chat meetings a few weeks ago, I brought up how the editor has made Wordpress essentially incompatible with strict CSPs and I was encouraged to make a new issue explicitly about that, but I hadn't gotten around to it.