Server side block styles - add default value support #24300

Addison-Stavlo · 2020-07-30T19:43:39Z

Description

Following up on the style support added for server side blocks in #23007 - This PR adds support for default values to be applied. These default values are already picked up by the corresponding hooks in the editor, but in the case of server side blocks are never applied on the front end.

How has this been tested?

There are not currently any server side blocks in Gutenberg that define defaults for these values. In my testing I have added default values (and their corresponding support flags if necessary) to current blocks (such as Post Author). After verifying all default value styles are applied as expected to the front-end, I added tests in class-block-supported-styles-test.php to make this easier to see. Each tests verifies the same output as its saved value counterpart, but is set up to have no attributes saved and defaults defined instead.

To test this manually for a server side block, you can add default values (and their corresponding support flags) to its block.json file under attributes. (see below)

For named default values you can add the following attributes (note - backgroundColor and gradient to be added and tested separately since they are both for the background):

"textColor": {
	"type": "string",
	"default": "secondary"
},
"backgroundColor": {
	"type": "string",
	"default": "foreground"
},
"gradient": {
	"type": "string",
	"default": "diagonal"
},
"fontSize": {
	"type": "string",
	"default": "large"
},
"align": {
	"type": "string",
	"default": "wide"
}

For custom default values you can add the following to attributes (same note with background and gradient to be tested separately):

"style": {
	"type": "object",
	"default": {
		"color": {
			"background": "#00F",
			"gradient": "linear-gradient(90deg, rgba(2,0,36,1) 0%, rgba(0,212,255,1) 100%)",
			"text": "#0F0",
			"link": "#F00"
		},
		"typography": {
			"fontSize": "33",
			"lineHeight": "2.0"
		}
	}
}

also, be sure the required flags are added under supports in block.json:

"__experimentalFontSize": true,
"__experimentalColor": {
	"gradients": true,
	"linkColor": true
},
"__experimentalLineHeight": true,
"align": true

Add the block to a post in the editor, save, and view it on the front-end. You should see the styles visually applied, and their corresponding classes and/or inline styles present on the wrapping element.

Screenshots

Before - A server side block with hook style support with default values defined but not applied:

After - Default values applied as expected:

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

github-actions · 2020-07-30T19:50:06Z

Size Change: +15 B (0%)

Total Size: 1.16 MB

Filename	Size	Change
`build/block-editor/index.js`	125 kB	+13 B (0%)
`build/block-library/index.js`	132 kB	+2 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	0 B
`build/annotations/index.js`	3.67 kB	0 B
`build/api-fetch/index.js`	3.44 kB	0 B
`build/autop/index.js`	2.82 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	7.93 kB	0 B
`build/block-directory/style-rtl.css`	953 B	0 B
`build/block-directory/style.css`	952 B	0 B
`build/block-editor/style-rtl.css`	10.8 kB	0 B
`build/block-editor/style.css`	10.8 kB	0 B
`build/block-library/editor-rtl.css`	7.59 kB	0 B
`build/block-library/editor.css`	7.59 kB	0 B
`build/block-library/style-rtl.css`	7.76 kB	0 B
`build/block-library/style.css`	7.77 kB	0 B
`build/block-library/theme-rtl.css`	728 B	0 B
`build/block-library/theme.css`	729 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.3 kB	0 B
`build/components/index.js`	200 kB	0 B
`build/components/style-rtl.css`	15.7 kB	0 B
`build/components/style.css`	15.7 kB	0 B
`build/compose/index.js`	9.68 kB	0 B
`build/core-data/index.js`	11.8 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/data/index.js`	8.45 kB	0 B
`build/date/index.js`	5.38 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.23 kB	0 B
`build/edit-navigation/index.js`	10.9 kB	0 B
`build/edit-navigation/style-rtl.css`	1.08 kB	0 B
`build/edit-navigation/style.css`	1.08 kB	0 B
`build/edit-post/index.js`	304 kB	0 B
`build/edit-post/style-rtl.css`	5.61 kB	0 B
`build/edit-post/style.css`	5.61 kB	0 B
`build/edit-site/index.js`	17 kB	0 B
`build/edit-site/style-rtl.css`	3.06 kB	0 B
`build/edit-site/style.css`	3.06 kB	0 B
`build/edit-widgets/index.js`	9.38 kB	0 B
`build/edit-widgets/style-rtl.css`	2.45 kB	0 B
`build/edit-widgets/style.css`	2.45 kB	0 B
`build/editor/editor-styles-rtl.css`	537 B	0 B
`build/editor/editor-styles.css`	539 B	0 B
`build/editor/index.js`	45.3 kB	0 B
`build/editor/style-rtl.css`	3.8 kB	0 B
`build/editor/style.css`	3.79 kB	0 B
`build/element/index.js`	4.65 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.72 kB	0 B
`build/format-library/style-rtl.css`	547 B	0 B
`build/format-library/style.css`	548 B	0 B
`build/hooks/index.js`	2.13 kB	0 B
`build/html-entities/index.js`	621 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.52 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/index.js`	3.11 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	476 B	0 B
`build/list-reusable-blocks/style.css`	476 B	0 B
`build/media-utils/index.js`	5.33 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`	671 B	0 B
`build/nux/style.css`	668 B	0 B
`build/plugins/index.js`	2.56 kB	0 B
`build/primitives/index.js`	1.41 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`	13.9 kB	0 B
`build/server-side-render/index.js`	2.71 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.27 kB	0 B
`build/url/index.js`	4.06 kB	0 B
`build/viewport/index.js`	1.85 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

apeatling

I tested this with the Site Title, and Site Tagline blocks. Added the style attribute and confirmed on refresh that the styles were applied. 👍

One concern -- these default styles applied even though I had manual style changes already to the blocks. I think this was because the blocks were already present and modified before applying these code changes, I could not reproduce when adding new blocks to the site editor.

apeatling · 2020-08-04T18:19:54Z

lib/blocks.php

 * @return array Colors CSS classes and inline styles.
 */
-function gutenberg_experimental_build_css_colors( $attributes, $block_attributes, $supports ) {
+function gutenberg_experimental_build_css_colors( $attributes, $block_attributes, $supports, $block_type ) {


This function is getting really long, and has a lot of conditionals. Not in this PR, but I think it would be good to consider ways to simplify and break out different parts into separate functions. With the number of conditionals it's becoming harder to follow.

Addison-Stavlo · 2020-08-04T19:49:00Z

One concern -- these default styles applied even though I had manual style changes already to the blocks.

Interesting! I am finding that if we have named default values they can overwrite the saved custom values in the editor but not the front-end. Is this what you are experiencing @apeatling ? Also which theme are you testing on? (Im noticing it is visible on some but not others, but with the problematic class names being added in either case).

I am finding this happening on master as well, and it seems specific to the server side blocks. Although, it seems standard blocks have some separate issues in the same circumstance. I will investigate more and write up some issues with more details.

Edit - From what I am seeing the default styles applied by the changes in this PR are working as expected, but there are some other issues with them being added in the editor that already exist if you test similarly on master #24363. (and also stumbled upon another issue with simple blocks like paragraph/heading - #24387)

Addison-Stavlo · 2020-08-06T23:37:13Z

@youknowriad - do you think this approach makes sense for how we should apply default defined values for these hooks? It seems to work well (not counting the already existing in-editor weirdness noted in #24363). If so, tomorrow I can rebase/reorg these based on your recent refactoring of these functions. 😁

youknowriad · 2020-08-10T08:34:01Z

Thanks for the work here, though, I'm hesitant personally because of a few considerations:

In the frontend, the default attributes handling is not done by the "support flags" themselves but it's applied at parsing time on the "attributes parsing stage" (second stage or parsing), this second stage is not done on the backend (and I don't think it can be done for several reasons: performance and DOM)
This makes the default values work for the attributes added by the built-in support flags but it doesn't make it work for all the other attributes.

That said, I don't have an alternative solution for that. So whether we accept that default values don't work on the server (like attributes parsing), or we move forward with this 50%solution for support flags.

I'd like more opinions on this from other contributors @mcsf @mtias

lib/blocks.php

Addison-Stavlo · 2020-08-10T15:13:09Z

This makes the default values work for the attributes added by the built-in support flags but it doesn't make it work for all the other attributes.
That said, I don't have an alternative solution for that. So whether we accept that default values don't work on the server (like attributes parsing), or we move forward with this 50%solution for support flags.

Interesting! For non-supported attributes maybe the solution is block developers handling this case in the blocks specific index.php file? For attributes related to our support flags however, it would be really nice if default values were supported and handling them as fallbacks where saved values are applied seems to make sense.

Addison-Stavlo · 2020-08-12T21:02:53Z

@mcsf - was that 👍 for:

we move forward with this 50%solution for support flags.

or did you have other thoughts?

I feel it would be good to support default values for the supported styles at the least.

My main motivation for this (besides completeness of these style applications) was to see about allowing default block alignment of full for the Post Content block. Being listed in the FSE block parity issue to have parity with 'group' block feature wise implies it should have wide/full alignment support.

But if we support this and do not set the default to full that means every time the Post Content block is inserted (possibly by other mechanisms, query/loops etc.) its default state of no alignment would mean that any blocks inside that post who have 'wide'/'full' alignment settings saved would be restricted in width by default instead of showing wide/full as expected.

Hence starting with full as the default option would seem much more friendly and make much more sense. Inner blocks that have wide/full support settings would show their intended width by default and if a user wanted to restrict the width of the post content container from there they would have the option.

🤔 Maybe there are other ways to get around this problem, but applying the full default seems like a sensible solution if we can support it.

mcsf · 2020-08-13T10:42:07Z

@mcsf - was that 👍 for:

we move forward with this 50%solution for support flags.

or did you have other thoughts?

Indeed, that was a bit opaque of me. :) I was trying to come to a position on this before I could explain my thoughts, but essentially my 👍 was an acknowledgement that this is a thorny issue.

I fully understand the end-user benefits of letting Post Content default to align: full. If this were a third-party block, I wouldn't object to ad hoc attribute handling on the back end, but with fundamental core blocks I worry about the precedents we set.

Some questions on my mind:

Yes, we don't have the full parsing abilities that we have on the client, meaning that we can't resolve sourced attributes (e.g. values sourced from the block markup, or from sources such as post meta). However, in practice, this server-side limitation has been around for a couple of years now, and any blocks that need to do work on the server have had to work around this already by using explicit (non-sourced) attributes. So, since this isn't a new problem, can't we ignore the matter of "stage 2 parsing" for the purposes of FSE / Global Styles?
In that case, can we generalise the solution so that, for any block with a render_callback being processed for rendering on the back end, for any of its attributes that a) is not sourced and b) has a default value, then we equate ! isset( $block_attributes[$attribute_name] ) with $block_attributes[$attribute_name] = $block_type->attributes[$attribute_name]['default']? This process would be applied during render_block before gutenberg_experimental_apply_classnames_and_styles.
Right now I can't think of any, but could there be issues where this hypothetical solution clashes with server-side block context?

Addison-Stavlo · 2020-08-13T17:44:05Z

@mcsf thank you for the insights! A couple questions:

meaning that we can't resolve sourced attributes (e.g. values sourced from the block markup

Does this mean that if a themes markup file for a template includes a template part with attribute align: full in the markup, that we would not be able to resolve that setting? Or am I misunderstanding 'sourced' and this mostly pertains to attributes that are not defined in the block.json or by the supported style hooks?

This process would be applied during render_block before gutenberg_experimental_apply_classnames_and_styles.

I wondered about this direction as well, it may make more sense! iirc Global Styles would do something similar to apply globally saved values before gutenberg_experimental_apply_classnames_and_styles if there haven't been any specific ones saved for the block? I assume default values should have lower priority than global styles (?), so we would run this after the global styles version and before gutenberg_experimental_apply_classnames_and_styles?

Moving forward from here

So if I am understanding correctly, instead of applying the default values 'ad hoc' as fallbacks as we are in this PR we should create a function to run before gutenberg_experimental_apply_classnames_and_styles that essentially does the following?:

If - $block-type uses render_callback:

Loop - through the $attribute_names as defined in $block_type->attributes
- If - the attribute is not saved for the block && a default value is defined
  - apply - the default value to $block_attributes

I think this does make more sense than the current approach in this PR.

Addison-Stavlo · 2020-08-17T18:24:08Z

@mcsf - expanding more on my questions above:

This process would be applied during render_block before gutenberg_experimental_apply_classnames_and_styles.

Doesn't the render_block filter only filter the actual content string? Or is there a way to use this to filter and return the actual $block array as well?

Addison-Stavlo · 2020-09-01T00:49:19Z

Closing this as it is the wrong approach. Opened #24909 - we can continue discussion over there.

mcsf · 2020-09-08T16:22:47Z

@Addison-Stavlo: I got back from holidays and am still catching up on things. 😓

meaning that we can't resolve sourced attributes (e.g. values sourced from the block markup

Does this mean that if a themes markup file for a template includes a template part with attribute align: full in the markup, that we would not be able to resolve that setting?

Correct.

I wondered about this direction as well, it may make more sense! iirc Global Styles would do something similar to apply globally saved values before gutenberg_experimental_apply_classnames_and_styles if there haven't been any specific ones saved for the block? […]

So if I am understanding correctly, instead of applying the default values 'ad hoc' as fallbacks as we are in this PR we should create a function to run before gutenberg_experimental_apply_classnames_and_styles that essentially does the following?:

To your question of whether you're understanding correctly: yes. :) That said, I don't like what past-me suggested, so... mostly disregard it.

Overall, there are a bunch of fragilities in how we're implementing Global Styles as far as the back end is concerned. Adding more logic like automatically resolving default attributes on the server (knowing that we can only support explicit (non-sourced) attributes and that we can't perform proper block validation), especially when it's done magically without the block author's intervention or realisation, gets us deeper into this problem.

We need to rethink a lot of this, and I don't yet have good ideas to propose.

Doesn't the render_block filter only filter the actual content string? Or is there a way to use this to filter and return the actual $block array as well?

Yeah, it needs to return a string. Strictly speaking, I suppose that block authors could get clever with reference-passed arguments (Gutenberg source, PHP references), but I hope I never see that. 😅

Addison-Stavlo · 2020-09-09T02:57:53Z

Interesting, I may have some more thoughts later but for now im curious about:

Does this mean that if a themes markup file for a template includes a template part with attribute align: full in the markup, that we would not be able to resolve that setting?

Correct.

I am a bit more confused here. It seems like we must be able to do this in some cases as attributes like slug and theme are resolved from the block markup to point the template to the correct template part file?

mcsf · 2020-09-09T10:04:08Z

I am a bit more confused here. It seems like we must be able to do this in some cases as attributes like slug and theme are resolved from the block markup to point the template to the correct template part file?

We might be getting our wires crossed. I'd like to understand this better. Can you DM me once you're around today? :)

Addison-Stavlo · 2020-09-10T22:29:50Z

We might be getting our wires crossed.

We definitely were. 😆 Thanks for the chat!

For visibility I summarized the takeaways from the convo on the other PR - #24909 (comment)

Addison-Stavlo added 3 commits July 30, 2020 10:45

added default fallbacks

96991fd

handle default background and gradient more gracefully

228273c

check for explicit no alignment value

5e7ae24

Addison-Stavlo added 7 commits July 30, 2020 16:43

support custom defaults

8e62e67

php format

7100946

fix gradient default

fb57923

whoopsie, thats just for testing

ba6ce07

thought i got rid of you...

881c137

update tests

d101ae0

fix linter errors

1e8963a

Addison-Stavlo self-assigned this Jul 30, 2020

Addison-Stavlo added the [Feature] Full Site Editing label Jul 30, 2020

Addison-Stavlo requested review from apeatling, youknowriad, noahtallen, ockham, vindl and Copons July 30, 2020 22:57

Addison-Stavlo changed the title ~~WIP server side block styles default values support~~ Server side block styles - add default value support Jul 30, 2020

Addison-Stavlo marked this pull request as ready for review July 30, 2020 22:58

Addison-Stavlo requested review from spacedmonkey and TimothyBJacobs as code owners July 30, 2020 22:58

Addison-Stavlo added 2 commits July 30, 2020 19:18

darn php unit linter is a bully

1c332c7

srsly go home evil linter

d1341ea

Addison-Stavlo mentioned this pull request Aug 3, 2020

Fix link color style property name in global styles #24296

Merged

handle merge conflict

5ae664d

apeatling reviewed Aug 4, 2020

View reviewed changes

jeyip reviewed Aug 10, 2020

View reviewed changes

lib/blocks.php Show resolved Hide resolved

Addison-Stavlo added the [Status] In Progress Tracking issues with work in progress label Aug 29, 2020

Addison-Stavlo mentioned this pull request Aug 29, 2020

Add default attribute values on server side. #24909

Closed

6 tasks

Addison-Stavlo closed this Sep 1, 2020

Addison-Stavlo removed the [Status] In Progress Tracking issues with work in progress label Sep 1, 2020

youknowriad deleted the update/server-side-block-styles-default-values-support branch September 1, 2020 09:49

mcsf mentioned this pull request Oct 14, 2020

Try: Reimplement Block Supports #26111

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Server side block styles - add default value support #24300

Server side block styles - add default value support #24300

Addison-Stavlo commented Jul 30, 2020 •

edited

Loading

github-actions bot commented Jul 30, 2020 •

edited

Loading

apeatling left a comment •

edited

Loading

apeatling Aug 4, 2020

Addison-Stavlo commented Aug 4, 2020 •

edited

Loading

Addison-Stavlo commented Aug 6, 2020 •

edited

Loading

youknowriad commented Aug 10, 2020 •

edited

Loading

Addison-Stavlo commented Aug 10, 2020 •

edited

Loading

Addison-Stavlo commented Aug 12, 2020

mcsf commented Aug 13, 2020

Addison-Stavlo commented Aug 13, 2020 •

edited

Loading

Addison-Stavlo commented Aug 17, 2020 •

edited

Loading

Addison-Stavlo commented Sep 1, 2020

mcsf commented Sep 8, 2020

Addison-Stavlo commented Sep 9, 2020

mcsf commented Sep 9, 2020

Addison-Stavlo commented Sep 10, 2020

Server side block styles - add default value support #24300

Server side block styles - add default value support #24300

Conversation

Addison-Stavlo commented Jul 30, 2020 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

github-actions bot commented Jul 30, 2020 • edited Loading

apeatling left a comment • edited Loading

Choose a reason for hiding this comment

apeatling Aug 4, 2020

Choose a reason for hiding this comment

Addison-Stavlo commented Aug 4, 2020 • edited Loading

Addison-Stavlo commented Aug 6, 2020 • edited Loading

youknowriad commented Aug 10, 2020 • edited Loading

Addison-Stavlo commented Aug 10, 2020 • edited Loading

Addison-Stavlo commented Aug 12, 2020

mcsf commented Aug 13, 2020

Addison-Stavlo commented Aug 13, 2020 • edited Loading

Addison-Stavlo commented Aug 17, 2020 • edited Loading

Addison-Stavlo commented Sep 1, 2020

mcsf commented Sep 8, 2020

Addison-Stavlo commented Sep 9, 2020

mcsf commented Sep 9, 2020

Addison-Stavlo commented Sep 10, 2020

Addison-Stavlo commented Jul 30, 2020 •

edited

Loading

github-actions bot commented Jul 30, 2020 •

edited

Loading

apeatling left a comment •

edited

Loading

Addison-Stavlo commented Aug 4, 2020 •

edited

Loading

Addison-Stavlo commented Aug 6, 2020 •

edited

Loading

youknowriad commented Aug 10, 2020 •

edited

Loading

Addison-Stavlo commented Aug 10, 2020 •

edited

Loading

Addison-Stavlo commented Aug 13, 2020 •

edited

Loading

Addison-Stavlo commented Aug 17, 2020 •

edited

Loading