Consolidate overall Global Styles mechanics in the server by oandregal · Pull Request #20047 · WordPress/gutenberg

oandregal · 2020-02-05T11:01:38Z

This PR seeks to consolidate the overall server-side mechanism for Global Styles. Continues work from #19883

It adds the user's Global Styles to the resolver, so it now works using core, theme, and user data. It stores the user's data using a Custom Post Type, so we can leverage existing editor mechanisms (undo/redo, draft/publish status for Global Styles, etc). This also presumes we want Global Styles to be tied to the theme so when theme changes we want to start with a new blank slate for Global Styles, which is implemented by using the theme's textdomain as part of the CPT's post name.
It exposes to the editor a couple of variables necessary for live-editing the styles: the ID of the entity that holds the user's styles + the base styles (core+theme). How this works will be demonstrated in subsequent PR. Note this is made to work only with the Beta Site Editor.
As per past conversations, it flattens the Global Styles tree stripping out the block variables. It looks like this:

{
	'color': {
		'primary': '<primary color>',
		'background': '<background color>',
		'text': '<text color>',
	},
        'line-height': '<line height>'
}

which will be converted to this CSS rule (see discussion about syntax):

:root {
	--wp--color--primary: '<primary color>';
	--wp--color--background: '<background color>';
	--wp--color--text: '<text color>';
        --wp--line-height: '<line height>';
}

Note that the names and variables we expose are still in flux, so take the current ones as only valid for demo purposes.

How to test

Install and enable one of the block-based themes (find one at theme-experiments). Instructions here assume Twenty Twenty Blocks.
Check out this PR and npm install && npm run build.
Create a void file named experimental-theme.json in your current theme's folder. For example, wp-content/themes/twentytwenty-blocks/experimental-theme.json.
Got to Gutenberg > Experiments and enable "Full Site Editing".
Go to Gutenberg > Site Editor.

1) Check that core's Global Styles work:

Add a button and verify that its background color is #52accc.

2) Check that theme's Global Styles override core's if present:

Add the following into the experimental-theme.json file created before.

{
	"color": {
		"primary": "hotpink"
	}
}

Reload the Site Editor.
Add a button and verify that the button's color is now hotpink.

3) Check that user's Global Styles override theme's if present:

Insert in the database a new post with the following data [1]:
- post_type='wp_global_styles'
- post_name='wp-global-styles-THEME-TEXTDOMAIN' (ex: 'wp-global-styles-twentytwenty-blocks')
- post_content='{"color":{"primary":"blue"}}'
- post_status='publish'
Reload the Site Editor.
Add a button and verify that the button's color is now blue.

[1] Make sure there isn't another post with the same post_name; if there is one, delete it first. Find below some ways you can add the required data for testing purposes. This is for the experimental Twenty Twenty Blocks theme, change to use your active theme's text-domain as required.

wp-cli -- you may need to provide other connection parameters depending on your setup:

wp post delete $(wp post list --post_type='wp_global_styles' --format=ids) --force
wp post create
    --post_type='wp_global_styles'
    --post_name='wp-global-styles-twentytwenty-blocks'
    --post_content='{"color":{"primary":"blue"}}'
    --post_status='publish'

PHP -- paste the following code within the gutenberg_experimental_global_styles_enqueue_assets when it comes to test user's Global Styles:

	// Delete existing CPT, if any.
	$gs_cpt_list = wp_get_recent_posts( [
		'post_type' => 'wp_global_styles',
	] );
	foreach ( $gs_cpt_list as $gs_cpt ) {
		wp_delete_post( $gs_cpt['ID'] );
	}

	// Insert new one with required data.
	wp_insert_post( [
	    'ID'           => 0,
	    'post_content' => json_encode( [
	        'color'=> [
	            'primary' => 'blue',
	        ],
	    ] ),
	    'post_type'   => 'wp_global_styles',
	    'post_name'   => 'wp-global-styles-twentytwenty-blocks',
	    'post_status' => 'publish',
	] );

oandregal · 2020-02-05T14:18:31Z

I just realized we didn't hide the GlobalStyles code behind a experimental flag. I'll add that in this PR.

Edit: added in 76e744e and e33a7a5.

ItsJonQ · 2020-02-05T22:35:45Z

Heya @nosolosw !! Thank you for working on this 😍

I walked through the steps you outlined:

Check out this PR and npm install && npm run build.
Create a void file named experimental-theme.json in your current theme's folder.
Enable the "Global Styles" in the experiments page.
Create a new post with a block button, publish, and go to the front end.

However, I wasn't able to see the Global styles stuff render. No wp-gs className being added, no hotpink button.

Is there something I need to do in addition to this?

Note: Testing locally on fresh Docker install and TwentyTwenty theme.

Thanks!

Update, I also tried running the wp command, but I see this:

❯ wp post create --post_type='wp_global_styles' --post_name='wp-global-styles-twenty-twenty' --post_content='{"color":{"primary":"blue"}}' --post_status='publish'
Warning: mysqli_real_connect(): php_network_getaddresses: getaddrinfo failed: nodename nor servname provided, or not known in /Users/Q/Sites/@wordpress/gutenberg/wordpress/src/wp-includes/wp-db.php on line 1638
Warning: mysqli_real_connect(): (HY000/2002): php_network_getaddresses: getaddrinfo failed: nodename nor servname provided, or not known in /Users/Q/Sites/@wordpress/gutenberg/wordpress/src/wp-includes/wp-db.php on line 1638
Error: `php_network_getaddresses: getaddrinfo failed: nodename nor servname provided, or not known`
Error establishing a database connection
This either means that the username and password information in your `wp-config.php` file is incorrect or we can’t contact the database server at `mysql`. This could mean your host’s database server is down.

Are you sure you have the correct username and password?
Are you sure you have typed the correct hostname?
Are you sure the database server is running?

If you’re unsure what these terms mean you should probably contact your host. If you still need help you can always visit the WordPress Support Forums. `php_network_getaddresses: getaddrinfo failed: nodename nor servname provided, or not known`

oandregal · 2020-02-06T10:08:18Z

Nope, that's all you need! Just tested with a brand new site, fresh clone PR, and the steps above: it worked fine for me.

If there is no wp-gs or #global-styles-inline-css stylesheet it means GlobalStyles isn't active: either the experimental-theme.json couldn't be found in the top-level dir of the theme or the experiment flag isn't enabled.

As per the CLI error. It looks like you need to provide some extra info (user/pass)? If you use the env bundled with gutenberg it may be more convenient to do npm run env cli instead. An alternative to CLI if that gets cumbersome would be pasting the following code within the gutenberg_experimental_global_styles_enqueue_assets function for testing user's Global Styles:

	// Delete existing CPT, if any.
	$gs_cpt_list = wp_get_recent_posts( [
		'post_type' => 'wp_global_styles',
	] );
	foreach ( $gs_cpt_list as $gs_cpt ) {
		wp_delete_post( $gs_cpt['ID'] );
	}

	// Insert new one with required data.
	wp_insert_post( [
	    'ID'           => 0,
	    'post_content' => json_encode( [
	        'color'=> [
	            'primary' => 'blue',
	        ],
	    ] ),
	    'post_type'   => 'wp_global_styles',
	    'post_name'   => 'wp-global-styles-twenty-twenty', // Note this is for TwentyTwenty!
	    'post_status' => 'publish',
	] );

oandregal · 2020-02-06T11:00:51Z

I saw that we were using the Full Site Editing flag in #20061 so I thought it'd be best to use it here as well. Changed in f9d90e0 and updated the instructions accordingly.

ItsJonQ · 2020-02-06T13:59:58Z

@nosolosw Hallooo!! Thank you for your support 🙏
Unfortunately, I'm still not able to get it working on my end

I've recorded a video here walking through the instructions:

https://www.loom.com/share/0635e0c69b1042129e09a12f555920e3

Hopefully this helps with some debugging?

ItsJonQ · 2020-02-07T15:22:09Z

@nosolosw Hallo!!!

Just tested this again, and it's working as expected! I have no idea why it wasn't working for me previously. Thank you so much for your efforts in helping me debug! 🤗

It was most likely my local Docker environment. My machine has been having issues lately.
I reinstalled all Docker things 2 days ago, and it's been pretty stable since.

I left a comment for something to fix. After that, I would say this is 👍 from me!

oandregal · 2020-02-07T17:46:44Z

I left a comment for something to fix.

@ItsJonQ I don't seem to find any comment in the PR?

ItsJonQ

@nosolosw Oops! I forgot to hit Request changes! 🙇

ItsJonQ · 2020-02-07T15:20:45Z

lib/global-styles.php

 			$result = array_merge(
 				$result,
-				gutenberg_get_css_vars( $value, $new_key, false )
+				gutenberg_experimental_global_styles_get_css_vars( $value, $new_key . '-' )


@nosolosw Haii!!!

Would we be able to change this from - to -- (double hyphen)?

@jorgefilipecosta made the suggestion of using double hyphen to better indicate depth levels from the global styles object within the flattened CSS var string.

https://github.com/WordPress/gutenberg/pull/19883/files#r373700289

Ah! But this was when we had different levels (globals vs blocks) so it was useful to differentiate the element part (global/block name) vs the CSS custom property name. Now that we don't have the element part (everything is prefixed by --wp-), is the -- still useful?

I mean, now we have --wp-color-primary, would we want to have --wp--color-primary instead?

Updated at 7efa53e and 798a0d3

@jorgefilipecosta and I talked this over and I want to write down the trade-offs of the current approach so we have them documented somewhere until we find a better place.

Jorge's rationale for suggesting the double dash is this: in scoped styles (we store the CSS custom properties as block attributes) we need to deserialize the string into an object for editor things. In that case, we won't be able to distinguish between a valid CSS property name (ex: line-height) and a name that results of flattening the object keys (background-color from {background: { color: 'red' }}). Let's work through an example. We've got this string we want to deserialize back into an object:

--wp-background-color: 'red'; --wp-line-height: 1.2;

How do we know which object to deserialize into without information about the object keys/CSS property names?

This is what we want:

{ background: { color: 'red', }, line-height: 1.2 }

A simple parser that creates one level per dash would return (which is not what we want, so we have to make the parser a bit more complex):

{ background: { color: 'red', }, line { height: 1.2, } }

By using double dashes the deserialization parser is simpler (each object level is separated by --):

--wp--background--color: 'red'; --wp--line-height: 1.2;

Personally, I'm not a fan of this implementation detail leaking into the API design/naming schema. However, I also think this is a fair approach to simplify the parser and get us going. The underlying assumption is that we store this information only once in the block attribute as a string, not as an object: when we cross that river, we'd have the opportunity to reevaluate this and change if necessary.

oandregal · 2020-02-11T10:54:55Z

This PR is ready to land, although it only works in the block-editor. We're going to integrate Global Styles in the edit-site editor, for which we need to add support for some things in edit-site:

hook admin_body_class: we use this to add the wp-gs class. Although the hook is fired, we need a way to identify the edit-site context server-side (something akin to $current_screen->is_block_editor).
hook block settings: doesn't fire for edit-site context.
hook enqueue_block_assets: doesn't fire for edit-site context.

I'm going to work on the edit-site things separately, so we can land this PR without being blocked.

oandregal · 2020-02-11T12:44:15Z

I was able to make this work for edit-site in a decoupled and simple way, so I went ahead and added the code in this branch. This works now only with edit-site and frontend.

ItsJonQ

🚀 @nosolosw Awesome work!!! Thank you for creating and iterating on this! Not only that, but you've also added it to FSE "Edit Site".

Testing locally, and it's working as outlined in your instructions.

From a code perspective, the mechanics of the PHP updates make sense of me.

ItsJonQ · 2020-02-11T18:42:54Z

@jorgefilipecosta If you're around, it would be great to get your thoughts on the PHP implementation 🙏 . Thank you!!

this gives a well-known string and avoids the need to parse the theme name for spaces, etc.

This has been probably inadvertenly added in a rebase.

lib/experiments-page.php

jorgefilipecosta

Hi @nosolosw nice work here 👍 generally the changes seem good and I think they are going in the right direction.

jorgefilipecosta · 2020-02-14T18:20:12Z

lib/global-styles.php

+function gutenberg_experimental_global_styles_get_user_cpt( $post_status_filter = array( 'publish' ), $should_create_draft = false ) {
+	$user_cpt         = array();
+	$post_type_filter = 'wp_global_styles';
+	$post_name_filter = 'wp-global-styles-' . strtolower( wp_get_theme()->get( 'TextDomain' ) );


I think as an id of the theme we should use get_stylesheet() and not the 'TextDomain'. I think the database setting the WordPress core stores in the database to identify the current theme is the stylesheet.

The template parts use the TextDomain for this, so I thought we may want to use the same mechanism for API consistency. I'm happy to change it to something else, but I guess it makes sense to update template parts as well?

jorgefilipecosta · 2020-02-14T18:25:36Z

lib/global-styles.php

+		$cpt_post_id = wp_insert_post(
+			array(
+				'post_content' => '{}',
+				'post_status'  => 'draft',


I think we may have a bug, in gutenberg_experimental_global_styles_get_user we are querying published posts, but we insert a draft post. On the next execution, a published post will still not exist and we will probably create another one.
I think we may be able to create the post with the published status ( the empty object will not do anything);

Note the optional parameter $should_create_draft, which is false by default. We only insert a draft if there is no CPT during the editor call gutenberg_experimental_global_styles_get_user_cpt_id. That's the only place we need a pre-existing CPT: the entities API only works with pre-existing objects, can't create new ones from the client-side.

jorgefilipecosta · 2020-02-14T18:34:13Z

lib/global-styles.php

-add_action( 'enqueue_block_assets', 'gutenberg_enqueue_global_styles_assets' );

 /**
- * Adds class wp-gs to the frontend body class if the theme defines a experimental-theme.json.


If we unconditionally add the wp-gs class we will break the styles of some blocks.

.wp-gs .wp-block-button__link:not(.has-background) { background-color: var(--wp--color--primary); }

As the class is added even if the theme does not support global styles a theme that was changing the button color using the following code will stop working:

.wp-block-button__link { background-color: red; }

I think the whole idea of adding wp-gs was to mark themes that support global styles if we unconditionally add the class we would be able to not add it at all as well.
I know that for now we only add if the experiment is enabled but one day we remove the experiment and in that case, we would unconditionally add the class.
I think we should add the class when the experiment is enabled and when the theme supports global styles.

We already do that! The class is only added when the experiment is enabled, the theme has support, and the editor is site editor (for back-end, doesn't apply to front-end).

jorgefilipecosta · 2020-02-14T18:37:16Z

packages/block-library/src/button/style.scss


 .wp-gs .wp-block-button__link:not(.has-background) {
-	background-color: var(--wp-block-core-button--color--background, var(--wp-color--primary, $dark-gray-700));
+	background-color: var(--wp--color--primary);


Should we pass $dark-gray-700 as a fallback?

Note that --wp--color--primary is going to have always a value (user-defined, the theme default, or core default), so the fallback won't be necessary.

oandregal requested review from Soean, TimothyBJacobs, ajitbohra, jorgefilipecosta and talldan as code owners February 5, 2020 11:01

oandregal self-assigned this Feb 5, 2020

oandregal added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Feb 5, 2020

oandregal requested a review from ItsJonQ February 5, 2020 11:02

oandregal changed the title ~~Consolidate overrall Global Styles mechanics in the server~~ Consolidate overall Global Styles mechanics in the server Feb 5, 2020

ItsJonQ requested changes Feb 7, 2020

View reviewed changes

oandregal force-pushed the try/global-styles-server branch from f9d90e0 to d950b5f Compare February 11, 2020 10:00

oandregal requested a review from ItsJonQ February 11, 2020 10:30

oandregal force-pushed the try/global-styles-server branch from 0933ad7 to a218643 Compare February 11, 2020 16:13

oandregal mentioned this pull request Feb 11, 2020

Try/global styles support WordPress/theme-experiments#17

Closed

ItsJonQ approved these changes Feb 11, 2020

View reviewed changes

oandregal added 5 commits February 11, 2020 20:21

Warm up changes

f52ca32

Use same pattern name

846e8e6

Extract get_from_file function

c41405b

Add placeholder to retrieve user global styles

f49f113

Simplify css vars retrieval

f598d4a

oandregal added 19 commits February 11, 2020 20:21

Centralize theme support check

455cc17

Better docs

c89779c

Consolidate block settings names

2d0fa0e

We should check theme support at later stages

ac12333

Fix typo

d4fed71

Follow post name convention: use dash instead of underscore

3e40c29

Make linter happy

caf9773

Only load Global Styles if experiment is enabled

946c398

Add Global Styles setting to experiments page

1a6db21

Use FSE experiment flag instead of dedicated Global Styles one

3aaccc1

Update token

527c263

Adapt CSS custom properties to new convention

ef09131

Use theme textdomain

1588f8c

this gives a well-known string and avoids the need to parse the theme name for spaces, etc.

Check for array key presence

1d9169e

Detect whether we are in the site editor context.

aeddf60

Fire block settings hook in edit-site

1e45e2d

Enqueue assets in a way that works for edit-site

9fe7800

Add some editor checks: only load if edit-site

ee550ad

Remove unrelated changes

6f1e87b

This has been probably inadvertenly added in a rebase.

oandregal commented Feb 11, 2020

View reviewed changes

lib/experiments-page.php Outdated Show resolved Hide resolved

oandregal force-pushed the try/global-styles-server branch from a218643 to 6f1e87b Compare February 11, 2020 19:25

oandregal merged commit d870797 into master Feb 14, 2020

oandregal deleted the try/global-styles-server branch February 14, 2020 09:30

github-actions bot added this to the Gutenberg 7.6 milestone Feb 14, 2020

jorgefilipecosta reviewed Feb 14, 2020

View reviewed changes

oandregal mentioned this pull request Feb 18, 2020

Manage CSS for Global Styles #20290

Merged

mtias mentioned this pull request Feb 20, 2020

Block Styles Breakdown #20331

Closed

82 tasks

This was referenced May 12, 2020

Global Styles: recap & next steps #22296

Closed

Global Styles: use CSS variables to mark block style hooks #22317

Closed

ellatrix mentioned this pull request Jun 16, 2020

WP 5.5: merge all PHP changes #23210

Closed

12 tasks

Conversation

oandregal commented Feb 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to test

Uh oh!

oandregal commented Feb 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ItsJonQ commented Feb 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oandregal commented Feb 6, 2020

Uh oh!

oandregal commented Feb 6, 2020

Uh oh!

ItsJonQ commented Feb 6, 2020

Uh oh!

ItsJonQ commented Feb 7, 2020

Uh oh!

oandregal commented Feb 7, 2020

Uh oh!

ItsJonQ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oandregal Feb 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oandregal Feb 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oandregal commented Feb 11, 2020

Uh oh!

oandregal commented Feb 11, 2020

Uh oh!

ItsJonQ left a comment

Choose a reason for hiding this comment

Uh oh!

ItsJonQ commented Feb 11, 2020

Uh oh!

Uh oh!

jorgefilipecosta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oandregal Feb 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oandregal Feb 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

oandregal commented Feb 5, 2020 •

edited

Loading

oandregal commented Feb 5, 2020 •

edited

Loading

ItsJonQ commented Feb 5, 2020 •

edited

Loading

oandregal Feb 10, 2020 •

edited

Loading

oandregal Feb 11, 2020 •

edited

Loading

oandregal Feb 17, 2020 •

edited

Loading

oandregal Feb 17, 2020 •

edited

Loading