Take block data for Global Styles from block.json #22698

oandregal · 2020-05-28T10:12:19Z

Follow-up to #20290 and #22491

This PR takes the block data necessary for Global Styles (block selector + the supported properties of a block) from the block registry.

How to test

Make sure that it works as before:

Take this PR.
Download and activate this demo theme.
Go to the front end and check that the embedded stylesheet with id #global-styles-inline-css has -among other things- these selectors:

h1 {
  font-size: calc(1px * var(--wp--preset--font-size--huge));
  color: var(--wp--preset--color--primary);
}
h4 {
  font-size: var(--wp--preset--font-size--normal);
  color: var(--wp--preset--color--secondary);
}
p {
  line-height: 1.4;
  font-size: calc(1px * var(--wp--preset--font-size--normal));
  color: var(--wp--preset--color--quaternary);
}

Make sure changes in block.json affect the global styles

Modify the block.json of the paragraph block. For example, set __experimentalColor to false.
Compile, so block.json is moved to the proper place: npm run build.
Reload the front-end and verify that the paragraph style rule doesn't contain the color declaration.

Make sure that blocks can only output the properties they support at the block level

Some examples to try:

Add the text color within the global context in the config for the theme. Verify that the generated CSS does not contain a declaration for the color.
Add a gradient config for the paragraph context in the config for the theme. Verify that the generated CSS does not contain a declaration for the background.

github-actions · 2020-05-28T10:18:46Z

Size Change: +46 B (0%)

Total Size: 1.12 MB

Filename	Size	Change
`build/block-library/index.js`	126 kB	+46 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/annotations/index.js`	3.62 kB	0 B
`build/api-fetch/index.js`	3.4 kB	0 B
`build/autop/index.js`	2.83 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	6.48 kB	0 B
`build/block-directory/style-rtl.css`	787 B	0 B
`build/block-directory/style.css`	787 B	0 B
`build/block-editor/index.js`	106 kB	0 B
`build/block-editor/style-rtl.css`	11.3 kB	0 B
`build/block-editor/style.css`	11.3 kB	0 B
`build/block-library/editor-rtl.css`	7.87 kB	0 B
`build/block-library/editor.css`	7.88 kB	0 B
`build/block-library/style-rtl.css`	7.69 kB	0 B
`build/block-library/style.css`	7.68 kB	0 B
`build/block-library/theme-rtl.css`	684 B	0 B
`build/block-library/theme.css`	686 B	0 B
`build/block-serialization-default-parser/index.js`	1.88 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/blocks/index.js`	48.2 kB	0 B
`build/components/index.js`	190 kB	0 B
`build/components/style-rtl.css`	19.5 kB	0 B
`build/components/style.css`	19.5 kB	0 B
`build/compose/index.js`	9.32 kB	0 B
`build/core-data/index.js`	11.4 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/data/index.js`	8.43 kB	0 B
`build/date/index.js`	5.47 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.11 kB	0 B
`build/edit-navigation/index.js`	8.05 kB	0 B
`build/edit-navigation/style-rtl.css`	857 B	0 B
`build/edit-navigation/style.css`	856 B	0 B
`build/edit-post/index.js`	302 kB	0 B
`build/edit-post/style-rtl.css`	5.43 kB	0 B
`build/edit-post/style.css`	5.43 kB	0 B
`build/edit-site/index.js`	14.1 kB	0 B
`build/edit-site/style-rtl.css`	2.96 kB	0 B
`build/edit-site/style.css`	2.96 kB	0 B
`build/edit-widgets/index.js`	8.88 kB	0 B
`build/edit-widgets/style-rtl.css`	2.4 kB	0 B
`build/edit-widgets/style.css`	2.4 kB	0 B
`build/editor/editor-styles-rtl.css`	425 B	0 B
`build/editor/editor-styles.css`	428 B	0 B
`build/editor/index.js`	44.6 kB	0 B
`build/editor/style-rtl.css`	4.26 kB	0 B
`build/editor/style.css`	4.27 kB	0 B
`build/element/index.js`	4.64 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.71 kB	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/hooks/index.js`	2.13 kB	0 B
`build/html-entities/index.js`	622 B	0 B
`build/i18n/index.js`	3.56 kB	0 B
`build/is-shallow-equal/index.js`	711 B	0 B
`build/keyboard-shortcuts/index.js`	2.51 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/index.js`	3.12 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/media-utils/index.js`	5.29 kB	0 B
`build/notices/index.js`	1.79 kB	0 B
`build/nux/index.js`	3.4 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/plugins/index.js`	2.56 kB	0 B
`build/primitives/index.js`	1.5 kB	0 B
`build/priority-queue/index.js`	789 B	0 B
`build/redux-routine/index.js`	2.85 kB	0 B
`build/rich-text/index.js`	14.8 kB	0 B
`build/server-side-render/index.js`	2.68 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.28 kB	0 B
`build/url/index.js`	4.02 kB	0 B
`build/viewport/index.js`	1.84 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

youknowriad · 2020-05-28T10:34:02Z

lib/global-styles.php

+		'color'       => '__experimentalColor',
+		'line-height' => '__experimentalLineHeight',
+		'font-size'   => '__experimentalFontSize',
+	);


not sure how important is it for this PR but noting that some of these features are not booleans.

For example for the "color" one, you can enable "colors" and "gradients" if the support for these is enabled, theme.json should be able to define three things:

background: gradient; backgroundColor: red; color: green;

Yeah, I've been reviewing the existing style properties and also noticed that background (solid or gradients) was missing from the original hard-coded list in global styles. I'll fix it in this PR.

Noting also that priority is important here (we can't apply the backgroundColor before the gradient otherwise it's overridden)

I'm thinking that priority can work like it works today (and in CSS, so people's mental models are aligned): all declarations are transformed to CSS in the order they appear in the theme.json. You can't have both a solid & a gradient background, so I think this assumption should be fine.

gziolo · 2020-05-28T10:54:11Z

I just wanted to raise that by using a top-level field in the block.json brings a few follow-up tasks:

it has to be included in WP_Block_Type object on the server
it needs to be whitelisted as a value that is exposed from the server
it needs to be added to the schema in the newly created REST API endpoint for block types
it deserves its own section in the RFC for block registration API
it deserves its own section in the documentation for the block type shape

I probably missed something :)

youknowriad · 2020-05-28T11:11:53Z

packages/block-library/src/columns/block.json

@@ -1,6 +1,7 @@
 {
 	"name": "core/columns",
 	"category": "layout",
+	"selector": ".wp-block-columns",


This should be the default and only needed for blocks that don't have the generated class.

ah, so would you prefer doing:

selector: true, for blocks that use the generated class

selector: <some-other-selector> for blocks that don't

After Greg's comment I'm thinking of moving this under the supports key and do the same as other experimental features, so we'd have:

supports: { __experimentalSelector: {boolean|string|object} }

selector: true, for blocks that use the generated class

Why do we need the selector: true. Can't we just consider it as the default if the "selector" is not specified?

This is not part of the block type RCF is it? @gziolo ?

As automatic selectors we should also use the block styles:
e.g: 'core/quote/large' automatically mapped to .wp-block-quote.is-style-large.

Yes I agree if no selector is specified at all we should target the block by the default class.

Can't we just consider it as the default if the "selector" is not specified?

Yeah, I think we can.

My mind-frame in coming to this was that it'd be an opt-in mechanism, but it can perhaps work fine by generating the class if no selector is provided. Thinking out loud: what would be the worst that can happen if this isn't opt-in? This is what I can think of:

We're duplicating the class generation logic client & server-side. I think this should be fine because that's an area that I don't expect to change.

Class conflicts due to namespacing not matching the class name. Ex:

core/media-text block name => .wp-block-media-text class name.

custom-library/media-text block name => .wp-block-media-text class name.

I don't think third-party blocks are overriding core's class names, otherwise, this is problematic on its own. So, perhaps what we should do is to generate this class by default: .wp-{namespace}-{block-name} to prevent conflicts. With the exception that we don't add the namespace for core blocks, so they'll be .wp-{block-name} instead.

As automatic selectors we should also use the block styles:
e.g: 'core/quote/large' automatically mapped to .wp-block-quote.is-style-large.

@jorgefilipecosta I think adding support for style variations merits its own PR / discussion. The first step would be to look at them as "sets of style attributes" as suggested at #20331 Perhaps after that experiment we end up doing some things (like inlining those values) that remove the need of targeting the existing classes.

spacedmonkey · 2020-05-28T12:10:01Z

Should this new field be added to the block type api?

lib/global-styles.php

jorgefilipecosta · 2020-05-28T12:13:51Z

Currently, we are using selectors like “core/heading/h1” for global styles. Which means that blocks will need away in the client to know if they are currently affected by a given selector (so their UI can show default values set in theme.json e.g.: font line, height, for a heading 1 etc):
One way to match a block instance to a selector would be that blocks provide matchers something like:

{ ‘core/heading/h1’: function( attributes ) => attributes.level === 1, ‘core/heading/h2’: function( attributes ) => attributes.level ===  2 }

But in the future, we may also want matchers on the server, e.g: to output CSS only for selectors currently used by some block. And that would require a matcher on the server as well.
What when specifying selectors like core/heading/h1 blocks allow pass the matcher with a syntax like [level= “1"] (similar to the native CSS syntax) to target a block only in specific cases?

Regarding the sector specification:
- Blocks may not set any sector. In that case, it is assumed a block will be targeted by their class.
- Blocks may set a normal block selector with a custom dom selector 'core/paragraph': 'p'.
- Blocks may set multiple selectors and their matches: 'core/heading/h1': [ 'h1', '[level= “1"]'.

jorgefilipecosta · 2020-05-28T12:25:50Z

packages/block-library/src/heading/block.json

+		"h1": "h1",
+		"h2": "h2",
+		"h3": "h3",
+		"h4": "h4",


As referred in #22698 (comment), we will need some way to know if a given block instance is h4 or not.

jorgefilipecosta · 2020-05-28T12:27:35Z

packages/block-library/src/columns/block.json

@@ -1,6 +1,7 @@
 {
 	"name": "core/columns",
 	"category": "layout",
+	"selector": ".wp-block-columns",


As automatic selectors we should also use the block styles:
e.g: 'core/quote/large' automatically mapped to .wp-block-quote.is-style-large.

oandregal · 2020-05-28T12:28:21Z

@jorgefilipecosta that's a neat idea!

To make sure I fully follow, there are few things to unpack.

How we target those selectors in theme.json. Currently, we use core/heading/h1, do you propose we should do core/heading[level=1] instead?
How do we declare the mappings in block.json. Would you mind expanding a bit on how would you do this, perhaps with examples for the paragraph and heading blocks?

oandregal · 2020-05-28T12:33:15Z

Should this new field be added to the block type api?

@spacedmonkey I think we want to move this under the supports key and use the __experimental prefix, like the rest of the experimental features. See thread.

I saw that there's a task to add the missing fields to the block type API, so I guess whatever we do for the supports subkeys that are experimental, we should do for this new field as well.

jorgefilipecosta · 2020-05-28T13:32:25Z

How we target those selectors in theme.json. Currently, we use core/heading/h1, do you propose we should do core/heading[level=1] instead?

I guess blocks could specify something like 'core/heading/h1': [ 'h1', '[level= “1"]'.
But thinking more I think a syntax like [level= “1"] odes does not make much sense we can use JSON directly {level: 1}. And than it seems block variations are already that a set of attributes for the block.
And I guess we can identify if a given block instance matches a specific variation. In that case, maybe we should simply include the ability to specify dom selectors for each block variation?

lib/utils.php

oandregal · 2020-06-01T10:41:08Z

lib/global-styles.php

@@ -80,6 +80,13 @@ function gutenberg_experimental_global_styles_get_from_file( $file_path ) {
 			file_get_contents( $file_path ),
 			true
 		);
+
+		$json_decoding_error = json_last_error_msg();


If theme.json contains a single comma in the wrong place makes json_decode to output a void array. As the file grows, this is more likely to happen and more difficult to spot. Adding this code at least helps in quickly determining what's happening by inspecting the error log.

oandregal · 2020-06-01T10:48:12Z

lib/global-styles.php

-		'core/media-text' => array(
-			'selector' => '.wp-block-media-text',
-			'supports' => array( 'color' ),
+			'supports' => array( 'background-color' ),


This one-liner allows the global context to generate CSS for background-color. Adding it in this PR (which is a refactor to take data from the block.json) because it's so small a change.

oandregal · 2020-06-02T12:08:26Z

The last bit for this PR is figuring out how the block can determine which selector it uses, which was brought up by @jorgefilipecosta I've done some unsuccessful explorations with block variations and now I'm looking at attribute sources.

However, I'd think this is PR is ready to be merged and we can iterate on that part in a follow-up PR, in the context in which that part is needed. By merging this I'll be able to unblock some work I plan to do this week, which is to revive the PR to enable the global styles sidebar in edit-site.

So, if @jorgefilipecosta or anyone wants to give this a try and perhaps ✔️ if it's ready, I'd happy to merge this as soon as possible.

oandregal · 2020-06-02T12:31:49Z

This is what I'm thinking for "dynamic selectors" (selectors for blocks that represent many HTML elements, such as core/heading -h1 to h6- and core/list -ol, ul-).

They could be captured like this:

block.json for core/heading:

{
    "name": "core/heading",
    "supports": {
        "__experimentalSelector": {
            "core/heading/h1": {
                "selector": "h1",
                "attributes": {
                    "level": 1
                },
            },
            "core/heading/h2": {
                "selector": "h2",
                "attributes": {
                    "level": 2
                },
            },
        },
    },
}

block.json for core/list

{
    "name": "core/list",
    "supports": {
        "__experimentalSelector": {
            "core/list/ol": {
                "selector": "ol",
                "attributes": {
                    "ordered": true
                },
            },
            "core/heading/ul": {
                "selector": "ul",
                "attributes": {
                    "ordered": false
                },
            },
        },
    },
}

Essentially: attributes declares how a specific selector matches the block attributes. It could be called match or something like that, I'm not attached to the name.

I'm thinking this could work and avoids having a JavaScript callable/function within block.json, so it's parseable by any means we want (JavaScript, PHP, mobile).

If folks are convinced about this approach, I can implement it. However, given that this PR doesn't have a use case for this yet (it's a refactor from hardcoding data to take them from block.json) I think I'll still want to merge as it is and explore this approach where it makes sense (for example, in the edit-site/global styles PR I plan to work on next).

jorgefilipecosta · 2020-06-02T13:31:59Z

Hi @nosolosw,

              "core/heading/h1": {
                "selector": "h1",
                "attributes": {
                    "level": 1
                },
            },

This approach makes sense to me. My only worry is that we are just specifying a set of attribute values. What is the difference between block variations? Conceptually block variations are just a set of attribute values. To see if the block matches core/heading/h1 we will need to see if the attributes of the block are equal to attributes. Could we just do the same with variations and verify if the attributes of a block match a variation?

jorgefilipecosta · 2020-06-02T13:39:47Z

There is a also a possible solution that would not involve specifying the set of attributes. It is using dom functions and verifying if the block dom container matches the selector using the matches function https://developer.mozilla.org/en-US/docs/Web/API/Element/matches.
This solution involves accessing dom then using effects to apply the styles or not to the context, it has some performance issues so I'm not sure it is something we should try.

jorgefilipecosta

I think we can merge this PR so we can continue the iterations.
There are some followups that should be proposed after this PR is merged:

Have a way to match a selector to a given block.
Automatically generate selectors based on block styles. So block styles can be defined with theme.json.

lib/global-styles.php

If a block doesn't provide a selector, we'll generate the class name: .wp-{namespace}-{block-name}

oandregal · 2020-06-02T15:56:58Z

Rebased to fix a legit CI error that was presumably fixed by this.

oandregal requested review from ajitbohra, jorgefilipecosta, Soean, spacedmonkey, talldan and TimothyBJacobs as code owners May 28, 2020 10:12

oandregal requested review from gziolo, ItsJonQ and youknowriad May 28, 2020 10:12

oandregal self-assigned this May 28, 2020

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

oandregal mentioned this pull request May 28, 2020

Document experimental theme.json #22518

Merged

youknowriad reviewed May 28, 2020

View reviewed changes

spacedmonkey reviewed May 28, 2020

View reviewed changes

lib/global-styles.php Show resolved Hide resolved

jorgefilipecosta reviewed May 28, 2020

View reviewed changes

oandregal mentioned this pull request May 28, 2020

Consolidate the style attributes of blocks #22700

Closed

oandregal requested review from chrisvanpatten and mkaz as code owners May 29, 2020 15:07

oandregal commented May 29, 2020

View reviewed changes

lib/utils.php Outdated Show resolved Hide resolved

oandregal mentioned this pull request May 29, 2020

Make global styles shape consistent with local styles shape. #22744

Merged

oandregal force-pushed the try/read-block-json-global-styles branch from 2f90eb7 to 61afe88 Compare May 29, 2020 17:41

oandregal commented Jun 1, 2020

View reviewed changes

oandregal mentioned this pull request Jun 1, 2020

Encode the heading block as block variations of h1-h6 elements #22805

Closed

jorgefilipecosta approved these changes Jun 2, 2020

View reviewed changes

lib/global-styles.php Outdated Show resolved Hide resolved

lib/global-styles.php Outdated Show resolved Hide resolved

oandregal added 15 commits June 2, 2020 17:54

Take block data from block.json

41480b1

Enable background

1312f9f

Make selector optional and move it under support as experimental

5fb5a35

If a block doesn't provide a selector, we'll generate the class name: .wp-{namespace}-{block-name}

Enable background-color

40a45e9

Update how object selectors are processed

c9db087

Add background to supported features

6582c3e

Add docs

96f71dd

Document core/heading/*

12f2de8

Improve docs

9de210b

Inform about JSON errors

7457a1b

Update docs

04f410b

Remove general selector

df3bac3

Update comments

b923780

Use str_replace instead of preg_replace

7f94cf9

Check error code instead of string

41d378b

oandregal force-pushed the try/read-block-json-global-styles branch from 3346f24 to 41d378b Compare June 2, 2020 15:55

oandregal merged commit df4bf4a into master Jun 3, 2020

oandregal deleted the try/read-block-json-global-styles branch June 3, 2020 08:05

github-actions bot added this to the Gutenberg 8.3 milestone Jun 3, 2020

ellatrix mentioned this pull request Jun 16, 2020

WP 5.5: merge all PHP changes #23210

Closed

12 tasks

oandregal mentioned this pull request Aug 6, 2020

Try new syntax for dynamic selectors #24423

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Take block data for Global Styles from block.json #22698

Take block data for Global Styles from block.json #22698

oandregal commented May 28, 2020 •

edited

Loading

github-actions bot commented May 28, 2020 •

edited

Loading

youknowriad May 28, 2020

oandregal May 28, 2020

youknowriad May 28, 2020

oandregal May 28, 2020

gziolo commented May 28, 2020 •

edited

Loading

youknowriad May 28, 2020

oandregal May 28, 2020

oandregal May 28, 2020

youknowriad May 28, 2020

spacedmonkey May 28, 2020

jorgefilipecosta May 28, 2020

jorgefilipecosta May 28, 2020

oandregal May 28, 2020

oandregal Jun 1, 2020 •

edited

Loading

spacedmonkey commented May 28, 2020

jorgefilipecosta commented May 28, 2020

jorgefilipecosta May 28, 2020

jorgefilipecosta May 28, 2020

oandregal commented May 28, 2020 •

edited

Loading

oandregal commented May 28, 2020

jorgefilipecosta commented May 28, 2020

oandregal Jun 1, 2020

oandregal Jun 1, 2020

oandregal commented Jun 2, 2020 •

edited

Loading

oandregal commented Jun 2, 2020 •

edited

Loading

jorgefilipecosta commented Jun 2, 2020

jorgefilipecosta commented Jun 2, 2020

jorgefilipecosta left a comment

oandregal commented Jun 2, 2020

Take block data for Global Styles from block.json #22698

Take block data for Global Styles from block.json #22698

Conversation

oandregal commented May 28, 2020 • edited Loading

How to test

github-actions bot commented May 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented May 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal Jun 1, 2020 • edited Loading

Choose a reason for hiding this comment

spacedmonkey commented May 28, 2020

jorgefilipecosta commented May 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented May 28, 2020 • edited Loading

oandregal commented May 28, 2020

jorgefilipecosta commented May 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal commented Jun 2, 2020 • edited Loading

oandregal commented Jun 2, 2020 • edited Loading

jorgefilipecosta commented Jun 2, 2020

jorgefilipecosta commented Jun 2, 2020

jorgefilipecosta left a comment

Choose a reason for hiding this comment

oandregal commented Jun 2, 2020

oandregal commented May 28, 2020 •

edited

Loading

github-actions bot commented May 28, 2020 •

edited

Loading

gziolo commented May 28, 2020 •

edited

Loading

oandregal Jun 1, 2020 •

edited

Loading

oandregal commented May 28, 2020 •

edited

Loading

oandregal commented Jun 2, 2020 •

edited

Loading

oandregal commented Jun 2, 2020 •

edited

Loading