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

Site Title: Add support settings for colors, fonts, and line height #23007

Merged
merged 49 commits into from
Jul 13, 2020
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
805d34b
Add experimental support settings for colors, fonts, and line height.
apeatling Jun 8, 2020
05d22e8
Add initial support for rendering of classes and styles for blocks re…
apeatling Jun 11, 2020
d652a9f
Remove custom item justification attribute.
apeatling Jun 11, 2020
357942c
Remove nav block references
apeatling Jun 11, 2020
a2db6d0
add line height support
Addison-Stavlo Jun 11, 2020
3a3095b
consolodate colors arrays
Addison-Stavlo Jun 12, 2020
3eb8e4d
check support, return early if none
Addison-Stavlo Jun 12, 2020
871a272
add support checks for colors
Addison-Stavlo Jun 12, 2020
fa6567b
support checks for typography
Addison-Stavlo Jun 12, 2020
76bc4f8
update comments
Addison-Stavlo Jun 12, 2020
efc7c7b
update function docs
Addison-Stavlo Jun 12, 2020
7e659a9
fix errors/warnings
Addison-Stavlo Jun 12, 2020
c215ae9
formatting
Addison-Stavlo Jun 12, 2020
0d40bad
started a test
Addison-Stavlo Jun 13, 2020
229dcd6
moved to own file
Addison-Stavlo Jun 15, 2020
e45898d
first test passes
Addison-Stavlo Jun 15, 2020
977384b
styles test
Addison-Stavlo Jun 15, 2020
e1cfeaa
unsupported colors test
Addison-Stavlo Jun 15, 2020
680a694
php format
Addison-Stavlo Jun 15, 2020
65c9763
add existing classes/styles
Addison-Stavlo Jun 15, 2020
9ec8584
add class check for style test
Addison-Stavlo Jun 15, 2020
5ca9b47
link color tests
Addison-Stavlo Jun 15, 2020
a9b1e3e
added gradient tests
Addison-Stavlo Jun 15, 2020
732e255
refactor an assertion function
Addison-Stavlo Jun 15, 2020
160d86f
added fontSize tests
Addison-Stavlo Jun 15, 2020
c958549
lineHeight tests
Addison-Stavlo Jun 15, 2020
e089533
test all supports together
Addison-Stavlo Jun 15, 2020
d493142
update comments
Addison-Stavlo Jun 15, 2020
2395a3e
file rename
Addison-Stavlo Jun 15, 2020
fec7628
fix linter errors
Addison-Stavlo Jun 15, 2020
d62406e
more lint error/warning goo
Addison-Stavlo Jun 15, 2020
32bd8bb
lint ignore our secret sauce
Addison-Stavlo Jun 16, 2020
f909ab2
Remove error suppression and return block content on invalid DOM load…
apeatling Jun 16, 2020
1bf2fc0
Update documentation to add site-title block.
apeatling Jun 16, 2020
bf1e768
helper fn and lint fix
Addison-Stavlo Jun 17, 2020
e81946e
use isset for checking the nested style objects
Addison-Stavlo Jun 17, 2020
8923bd8
Update lib/blocks.php
Addison-Stavlo Jun 17, 2020
a581770
Update lib/blocks.php
Addison-Stavlo Jun 17, 2020
7cb2693
check for ending semi-colon on existing styles
Addison-Stavlo Jun 19, 2020
543fb7e
ensure space for string concat and unique-ify-ing
Addison-Stavlo Jun 22, 2020
806a23f
Remove template part block check, this is no longer needed because we…
apeatling Jun 22, 2020
998f7f6
WordPress coding standards tweaks
apeatling Jun 22, 2020
0559112
refactor per Riad's interface suggestion
Addison-Stavlo Jun 24, 2020
35a5856
merge branch 'master' into add/site-title-fonts-colors
Addison-Stavlo Jun 25, 2020
aa4bbdc
cleanup grossly long 1-liners
Addison-Stavlo Jun 25, 2020
1987286
php format
Addison-Stavlo Jun 25, 2020
ba6a395
update tests for better inline style spacing
Addison-Stavlo Jun 25, 2020
7c17c76
require render_callback
Addison-Stavlo Jun 25, 2020
9e148d1
rebase
Addison-Stavlo Jul 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions docs/designers-developers/developers/themes/theme-json.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ Each block will declare which style properties it exposes. This has been coined
| Group | Yes | Yes | Yes | Yes |
| Columns | Yes | Yes | Yes | Yes |
| Media & text | Yes | Yes | Yes | Yes |
| Site Title | Yes | Yes | - | Yes |

[1] The heading block represents 6 distinct HTML elements: H1-H6. It comes with selectors to target each individual element (ex: core/heading/h1 for H1, etc).

Expand All @@ -180,6 +181,7 @@ Each block will declare which style properties it exposes. This has been coined
| Group | - | - |
| Columns | - | - |
| Media & text | - | - |
| Site Title | Yes | Yes |

[1] The heading block represents 6 distinct HTML elements: H1-H6. It comes with selectors to target each individual element (ex: core/heading/h1 for H1, etc).

Expand Down Expand Up @@ -379,6 +381,19 @@ The list of features that are currently supported are:
"text": <value>
}
}
},
"core/site-title": {
"styles": {
"color": {
"background": <value>,
"gradient": <value>,
"text": <value>
},
"typography": {
"fontSize": <value>,
"lineHeight": <value>
}
}
}
}
```
206 changes: 206 additions & 0 deletions lib/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -296,3 +296,209 @@ function gutenberg_enqueue_editor_block_styles_assets() {
}
add_action( 'enqueue_block_editor_assets', 'gutenberg_enqueue_editor_block_styles_assets' );
}

/**
* Renders the classNames and styles for blocks
*
* @param string $block_content Output of the current block.
* @param array $block Block Object.
* @return string New block output.
*/
function gutenberg_experimental_apply_classnames_and_styles( $block_content, $block ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens with existing blocks that already apply these flags on the frontend on their "save" function? how can we exclude them from this behavior?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens with existing blocks that already apply these flags on the frontend on their "save" function?

They go through this function unnecessarily. They do 'play nice', although this function handling them as well is a bit redundant.

how can we exclude them from this behavior?

Im not sure. There doesn't seem to be anything on the block object passed into the function that signifies anything special about this case. Maybe we can add an extra property on the block object or an attr to signal that it should be ignored by this function? Im open to ideas...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't testing for the presence of "render_callback" enough maybe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to do the trick! Thank you 😄 - 7c17c76

if ( ! isset( $block['attrs'] ) ) {
return $block_content;
}

// Check what style features the block supports.
$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );
if ( ! $block_type ) {
return $block_content;
}

$supports = gutenberg_experimental_global_styles_get_supported_styles( $block_type->supports );

// Return early if nothing is supported.
if ( 0 === count( $supports ) ) {
return $block_content;
}

$colors = gutenberg_experimental_build_css_colors( $block['attrs'], $supports );
$typography = gutenberg_experimental_build_css_typography( $block['attrs'], $supports );

$extra_classes = array_merge(
$colors['css_classes'],
$typography['css_classes'],
isset( $block['attrs']['className'] ) ? array( $block['attrs']['className'] ) : array(),
isset( $block['attrs']['align'] ) ? array( 'has-text-align-' . $block['attrs']['align'] ) : array()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@youknowriad - I'm wondering if it makes sense for us to add the text alignment and className here?

If it does, should we skip the early return for no support flags and apply align and className for all blocks that have those attributes saved? Or should we only apply these attrs if the block is using one of the experimental support flags? (or not at all?)


Other than that, this PR should be ready for another look over once you get a chance. 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My feeling is that this function knows too much about what each support flag (align, classnames, experimental ones)... does.

Ideally it's written in a generic way where we'd have a clear "interface" if want to add a new support flag. Doest that make sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i seem something like

$attributes =  array();
$attributes = gutenberg_experimental_apply_colors_flag(  $attributes, $block['attrs'], $supports  );
$attributes = gutenberg_experimental_apply_lineheight_flag(  $attributes, $block['attrs'], $supports  );
$attributes = gutenberg_experimental_apply-align_flag(  $attributes, $block['attrs'], $supports  );
$attributes = gutenberg_experimental_apply_custom_classname_flag(  $attributes, $block['attrs'], $supports  );


if ( ! count( $attributes ) ) {
     return $post_content;
} 

// Do the magic to parse the html and add  the extra attributes (we could start by supporting class and style).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that makes sense.

However, Im not currently seeing any of the similar experimental supports flags for the text alignment and class name. Should we leave these out for now and expect them to be handled elsewhere for the time being (index.php I'm guessing for these blocks)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have the customClassName for the custom classname, classname for the generated classname (which can also be handled similarly) and align for text alligment. I'm fine leaving them for dedicated PRs though and just get the API right in this initial PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine leaving them for dedicated PRs though and just get the API right in this initial PR.

Sounds good.

We have the customClassName for the custom classname, classname for the generated classname (which can also be handled similarly) and align for text alignment.

I saw these, but the reasons I am confused is that they don't have support keys defined for them like the others (font-size, line-height, colors, etc.) do. Similarly, they aren't checked for in the gutenberg_experimental_global_styles_get_supported_styles function:

$style_features = array(
'color' => array( '__experimentalColor' ),
'background-color' => array( '__experimentalColor' ),
'background' => array( '__experimentalColor', 'gradients' ),
'line-height' => array( '__experimentalLineHeight' ),
'font-size' => array( '__experimentalFontSize' ),
);

So I was a bit confused about how they are supposed to be handled compared to the ones we have here that require the support flags, and if they are intended to be added here or elsewhere.

Are we to assume these ones without support keys are supported by default if the corresponding attributes are saved and present? If that is the case we can handle them that way here.

align for text alignment

I was assuming this is for block alignment since it has settings like 'wide' and 'full' width. Or does this combine both text alignment and block/element alignment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right "align" is for block alignment my bad but it's also a good candidate for server-side generation.

customClassName and className are more "historical" and not part of the global styles project but their behavior is very close. and yes, they are opt-out and I believe className can also receive a string to be used as class (need confirmation)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i seem something like

Updated to adopt this pattern instead. Let me know if this isn't what you had in mind.

);
$extra_styles = (
$colors['inline_styles'] ||
$typography['inline_styles']
) ? esc_attr( $colors['inline_styles'] ) .
esc_attr( $typography['inline_styles'] )
: '';

$dom = new DOMDocument( '1.0', 'utf-8' );

// Suppress warnings from this method from polluting the front-end.
// @codingStandardsIgnoreStart
if ( ! @$dom->loadHTML( $block_content, LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD | LIBXML_COMPACT ) ) {
Comment on lines +248 to +252
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for reference: this introduced #24445, with a likely fix in #24447. I don't find it surprising that we discover quirks in these PHP interfaces as we start doing more server-side manipulation of content. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching it. 👍

// @codingStandardsIgnoreEnd
return $block_content;
}

$xpath = new DOMXPath( $dom );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if regex is more performant. This is definitely stronger and safer but in the long run, performance is important for the frontend. I'm happy starting with this and monitor impact

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a similar thought about performance, but attempting to parse HTML with a regex gets messy really quickly.

FWIW, I found myself in a similar situation a while ago, when adding a new sniff to WPCS. I ended up using XMLReader which out of a number of options seemed to have the smallest resource footprint. However, I only needed parsing at the time -- I didn't have to write HTML.

For writing, there's however XMLWriter -- maybe it could be used in combination with XMLReader.

Here's a StackOverflow post listing PHP's built-in (and external) HTML parsing options.

Here's some more background on RegEx-based parsing (and why it's messy) and alternatives.

$block_root = $xpath->query( '/*' )[0];

if ( empty( $block_root ) ) {
return $block_content;
}

// Some inline styles may be added without ending ';', add this if necessary.
$current_styles = trim( $block_root->getAttribute( 'style' ), ' ' );
if ( strlen( $current_styles ) > 0 && substr( $current_styles, -1 ) !== ';' ) {
$current_styles = $current_styles . ';';
};

// Merge and dedupe new and existing classes and styles.
$new_classes = implode( ' ', array_unique( explode( ' ', ltrim( $block_root->getAttribute( 'class' ) . ' ' ) . implode( ' ', $extra_classes ) ) ) );
$new_styles = implode( ' ', array_unique( explode( ' ', $current_styles . ' ' . $extra_styles ) ) );

// Apply new styles and classes.
if ( ! empty( $new_classes ) ) {
$block_root->setAttribute( 'class', $new_classes );
}

if ( ! empty( $new_styles ) ) {
$block_root->setAttribute( 'style', $new_styles );
}

return $dom->saveHtml();
}
add_filter( 'render_block', 'gutenberg_experimental_apply_classnames_and_styles', 10, 2 );

/**
* Build an array with CSS classes and inline styles defining the colors
* which will be applied to the block markup in the front-end.
*
* @param array $attributes block attributes.
* @param array $supports style features the block attributes.
* @return array Colors CSS classes and inline styles.
*/
function gutenberg_experimental_build_css_colors( $attributes, $supports ) {
$color_settings = array(
'css_classes' => array(),
'inline_styles' => '',
);

// Text Colors.
// Check support for text colors.
if ( in_array( 'color', $supports, true ) ) {
$has_named_text_color = array_key_exists( 'textColor', $attributes );
$has_custom_text_color = isset( $attributes['style']['color']['text'] );

// Apply required generic class.
if ( $has_custom_text_color || $has_named_text_color ) {
$color_settings['css_classes'][] = 'has-text-color';
}
// Apply color class or inline style.
if ( $has_named_text_color ) {
$color_settings['css_classes'][] = sprintf( 'has-%s-color', $attributes['textColor'] );
} elseif ( $has_custom_text_color ) {
$color_settings['inline_styles'] .= sprintf( 'color: %s;', $attributes['style']['color']['text'] );
}
}

// Link Colors.
if ( in_array( 'link-color', $supports, true ) ) {
$has_link_color = isset( $attributes['style']['color']['link'] );
// Apply required class and style.
if ( $has_link_color ) {
$color_settings['css_classes'][] = 'has-link-color';
// If link is a named color.
if ( strpos( $attributes['style']['color']['link'], 'var:preset|color|' ) !== false ) {
// Get the name from the string and add proper styles.
$index_to_splice = strrpos( $attributes['style']['color']['link'], '|' ) + 1;
$link_color_name = substr( $attributes['style']['color']['link'], $index_to_splice );
$color_settings['inline_styles'] .= sprintf( '--wp--style--color--link:var(--wp--preset--color--%s);', $link_color_name );
} else {
$color_settings['inline_styles'] .= sprintf( '--wp--style--color--link: %s;', $attributes['style']['color']['link'] );
}
}
}

// Background Colors.
if ( in_array( 'background-color', $supports, true ) ) {
$has_named_background_color = array_key_exists( 'backgroundColor', $attributes );
$has_custom_background_color = isset( $attributes['style']['color']['background'] );

// Apply required background class.
if ( $has_custom_background_color || $has_named_background_color ) {
$color_settings['css_classes'][] = 'has-background';
}
// Apply background color classes or styles.
if ( $has_named_background_color ) {
$color_settings['css_classes'][] = sprintf( 'has-%s-background-color', $attributes['backgroundColor'] );
} elseif ( $has_custom_background_color ) {
$color_settings['inline_styles'] .= sprintf( 'background-color: %s;', $attributes['style']['color']['background'] );
}
}

// Gradients.
if ( in_array( 'background', $supports, true ) ) {
$has_named_gradient = array_key_exists( 'gradient', $attributes );
$has_custom_gradient = isset( $attributes['style']['color']['gradient'] );

if ( $has_named_gradient || $has_custom_gradient ) {
$color_settings['css_classes'][] = 'has-background';
}
// Apply required background class.
if ( $has_named_gradient ) {
$color_settings['css_classes'][] = sprintf( 'has-%s-gradient-background', $attributes['gradient'] );
} elseif ( $has_custom_gradient ) {
$color_settings['inline_styles'] .= sprintf( 'background: %s;', $attributes['style']['color']['gradient'] );
}
}
Addison-Stavlo marked this conversation as resolved.
Show resolved Hide resolved

return $color_settings;
}

/**
* Build an array with CSS classes and inline styles defining the font sizes
* which will be applied to the block markup in the front-end.
*
* @param array $attributes block attributes.
* @param array $supports style features the block attributes.
* @return array Font size CSS classes and inline styles.
*/
function gutenberg_experimental_build_css_typography( $attributes, $supports ) {
// CSS classes.
$typography = array(
'css_classes' => array(),
'inline_styles' => '',
);

// Font Size.
if ( in_array( 'font-size', $supports, true ) ) {
$has_named_font_size = array_key_exists( 'fontSize', $attributes );
$has_custom_font_size = isset( $attributes['style']['typography']['fontSize'] );

// Apply required class or style.
if ( $has_named_font_size ) {
$typography['css_classes'][] = sprintf( 'has-%s-font-size', $attributes['fontSize'] );
} elseif ( $has_custom_font_size ) {
$typography['inline_styles'] .= sprintf( 'font-size: %spx;', $attributes['style']['typography']['fontSize'] );
}
}

// Line Height.
if ( in_array( 'line-height', $supports, true ) ) {
$has_line_height = isset( $attributes['style']['typography']['lineHeight'] );
// Add the style (no classes for line-height).
if ( $has_line_height ) {
$typography['inline_styles'] .= sprintf( 'line-height: %s;', $attributes['style']['typography']['lineHeight'] );
}
}

return $typography;
}
Addison-Stavlo marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions lib/global-styles.php
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ function gutenberg_experimental_global_styles_get_supported_styles( $supports )
'color' => array( '__experimentalColor' ),
'background-color' => array( '__experimentalColor' ),
'background' => array( '__experimentalColor', 'gradients' ),
'link-color' => array( '__experimentalColor', 'linkColor' ),
'line-height' => array( '__experimentalLineHeight' ),
'font-size' => array( '__experimentalFontSize' ),
);
Expand Down
7 changes: 6 additions & 1 deletion packages/block-library/src/site-title/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
},
"supports": {
"html": false,
"lightBlockWrapper": true
"lightBlockWrapper": true,
"__experimentalColor": {
"gradients": true
},
"__experimentalFontSize": true,
"__experimentalLineHeight": true
apeatling marked this conversation as resolved.
Show resolved Hide resolved
}
}
1 change: 0 additions & 1 deletion phpunit/class-block-context-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,5 +196,4 @@ function test_default_context_is_filterable() {

$this->assertEquals( array( 'example' => 'ok' ), $provided_context[0] );
}

}
Loading