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

Add: Automatic editor styles generation #18028

Closed
wants to merge 1 commit into from

Conversation

@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented Oct 18, 2019

Description

Fixes: #17860

This PR implements a mechanism to auto-generate colors and font sizes styles (and gradients will also support this when merged).

The API is the following:

  • add_theme_support( '__experimental-editor-automatic-styles' ); - Enables automatic styles for everything.
  • add_theme_support( '__experimental-editor-automatic-styles', array( 'editor-font-sizes') ); - Enables editor styles for the theme support elements passed in the array e.g: editor-font-sizes, editor-color-palette
  • By default the support is not enable and automatic styles are not generated.

The block editor adds the styles in an inline style element. This behavior is far from perfect because the browser does not cache the styles. WordPress passes these styles to the browser in every page request. I checked how the customizer loads the custom user style sheet -- It also uses inline styles. Themes that allow the user to dynamically change colors (e.g: 2019) also load the styles in inline styles. More recently, the block styles server API relied on the same system.
I think it would be good if the core could offer an alternative used in these and other similar cases.

I thought about some possibilities:

Create an endpoint that outputs CSS with a never expiring cache header. We would generate a hash of the styles and append it to the URL. When the styles change (e.g: the theme has set dynamic colors depending on the post type ) even though the caching was never expiring, the browser will make a new request because the URL is different.
The problem with this approach is that during the first visit, we load WordPress 2 times one for generating the HTML page and another one to generate the styles. If the styles are very dynamic, the cache would also not be useful, and WordPress may end up loading many times during the visit of a user.

Generate a hash of the styles and verify if a CSS file exists on the file system for that hash; if not, WordPress generates the styles and writes them to the file-system.
Then we enqueue a style sheet that references the file in the file system.
In this approach, we load WordPress one time, but we add an overhead on the loading: hashing things, check the file system, maybe write to the filesystem.
The styles may be very dynamic (e.g., a theme may change colors depending on the day of the month). We would need logic to clean old style files and to decide what is considered old. Another problem is that WordPress may not have permission to write files in some cases, and there are also certainly security concerns when writing files, so this option does not seem very viable.

The possibilities I thought are not perfect either, so this PR follows the typical approach used until now.

How has this been tested?

I added a color palette:

		add_theme_support(
			'editor-color-palette',
			array(
				array(
					'name'  => __( 'Dark Gray', 'twentynineteen' ),
					'slug'  => 'dark-gray',
					'color' => '#111',
				),
				array(
					'name'  => __( 'Light Gray', 'twentynineteen' ),
					'slug'  => 'light-gray',
					'color' => '#767676',
				),
				array(
					'name'  => __( 'White', 'twentynineteen' ),
					'slug'  => 'white',
					'color' => '#FFF',
				),
			)
		);

I added editor font sizes:

		add_theme_support(
			'editor-font-sizes',
			array(
				array(
					'name'      => __( 'Small', 'twentynineteen' ),
					'shortName' => __( 'S', 'twentynineteen' ),
					'size'      => 19.5,
					'slug'      => 'small',
				),
				array(
					'name'      => __( 'Normal', 'twentynineteen' ),
					'shortName' => __( 'M', 'twentynineteen' ),
					'size'      => 22,
					'slug'      => 'normal',
				),
				array(
					'name'      => __( 'Large', 'twentynineteen' ),
					'shortName' => __( 'L', 'twentynineteen' ),
					'size'      => 36.5,
					'slug'      => 'large',
				),
				array(
					'name'      => __( 'Huge', 'twentynineteen' ),
					'shortName' => __( 'XL', 'twentynineteen' ),
					'size'      => 49.5,
					'slug'      => 'huge',
				),
			)
		);

I enabled __experimental-editor-automatic-styles theme support:

add_theme_support( '__experimental-editor-automatic-styles' )

I verified that the block editor added the styles for the color pallet and font sizes on the editor and the frontend. We can find the styles by searching on the pages source code for wp-editor-automatic-styles-inline-css.

I enabled __experimental-editor-automatic-styles theme support just for font sizes.

add_theme_support( '__experimental-editor-automatic-styles', array( 'editor-font-sizes' ) );

I verified that the block editor added the styles for the font sizes on the editor and the frontend. We can find the styles by searching on the pages source code for wp-editor-automatic-styles-inline-css.

Copy link
Contributor

youknowriad left a comment

Nice one @jorgefilipecosta

Can we have more php and design 👀 on this.

Do you think we enable this by default and do a dev note about it instead?

@youknowriad youknowriad requested a review from WordPress/gutenberg-core Oct 21, 2019
@karmatosed karmatosed added this to In Progress in Tightening Up Oct 22, 2019
@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented Nov 13, 2019

Do you think we enable this by default and do a dev note about it instead?

Hi @youknowriad, this solution uses inline styles that need to be sent on every page request so from the performance point of view is inferior to what would happen if a theme included these styles on the theme stylesheet.
We may offer this mechanism for simplicity but I think it should not be the default so at least developers make a conscious choice when using this mechanism.

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Nov 18, 2019

Closed the issue, I'm closing the PR as well. Let's reconsider this if we come up with a better approach. Thanks for taking a look @jorgefilipecosta

@youknowriad youknowriad deleted the add/automatic-style-genetion branch Nov 18, 2019
@karmatosed karmatosed removed this from In Progress in Tightening Up Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.