Automattic / default-small-business-theme

GNU General Public License v2.0
10 stars 7 forks source link

Editor styles: Redundant enqueuing of editor styles #16

Open laurelfulford opened 5 years ago

laurelfulford commented 5 years ago

I mentioned this during Business Elegant's review, but right now the editor styles are enqueued two different ways into the new block editor:

This method is standard, and I think a bit more predictable: https://github.com/Automattic/default-small-business-theme/blob/master/functions.php#L192-L199 It links the stylesheet, as is, to the page.

This method is actually intended for existing TinyMCE stylesheets in older themes (like Twenty Seventeen): https://github.com/Automattic/default-small-business-theme/blob/master/functions.php#L75-L76 So it's basically expecting stylesheets that only reference HTML elements (h1, table), and maybe some classes like .gallery.

The second method embeds the styles in the head of the page, prefixed with the class .editor-styles-wrapper. This makes these styles a lot more "powerful" and overridy than they would be enqueued in the first method. Because of that, when this is updated some careful testing will be needed, to make sure those styles still work as expected.

laurelfulford commented 5 years ago

Adding a follow up here, since after doing some digging, I don't think I have this 100% right :)

This method can still be used for block styles in the editor:

// Add support for core editor styles
add_theme_support( 'editor-styles' );

// Add support for custom editor styles
add_editor_style( 'editor.css' );

As I mentioned above, it will prefix each style with .editor-styles-wrapper, and embed the styles in the page, so they're pretty "strong". It might be possible to audit and simplify selectors because of this, compared to what we've been doing.

Using this method isn't necessarily the best option for all styles though. Something like .wp-block, for example, gets really over-powery if you add it there, if you're trying to set a different max-width for your blocks. So something like that may be better suited to enqueue in a separate stylesheet using:

/**
 * Enqueue Gutenberg editor styles
 */
function business_theme_editor_styles() {
    wp_enqueue_style( 'business_theme-editor-style', get_template_directory_uri() . '/additional-editor-styles.css' );
}
add_action( 'enqueue_block_editor_assets', 'business_theme_editor_styles' );

As Business does it now, the same stylesheet is enqueued using both methods, which isn't needed. But there's probably a better balance between these methods than what I've been doing (using the second option and a lot of classes, since that's how I did the default themes as they had existing editor styles already). Some experimentation can probably get us to a better middle ground :)