-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Changes from 42 commits
805d34b
05d22e8
d652a9f
357942c
a2db6d0
3a3095b
3eb8e4d
871a272
fa6567b
76bc4f8
efc7c7b
7e659a9
c215ae9
0d40bad
229dcd6
e45898d
977384b
e1cfeaa
680a694
65c9763
9ec8584
5ca9b47
a9b1e3e
732e255
160d86f
c958549
e089533
d493142
2395a3e
fec7628
d62406e
32bd8bb
f909ab2
1bf2fc0
bf1e768
e81946e
8923bd8
a581770
7cb2693
543fb7e
806a23f
998f7f6
0559112
35a5856
aa4bbdc
1987286
ba6a395
7c17c76
9e148d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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 ) { | ||||||||||||||||
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() | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Other than that, this PR should be ready for another look over once you get a chance. 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i seem something like
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sounds good.
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/lib/global-styles.php Lines 272 to 278 in b98418d
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.
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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for catching it. 👍 |
||||||||||||||||
// @codingStandardsIgnoreEnd | ||||||||||||||||
return $block_content; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
$xpath = new DOMXPath( $dom ); | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They go through this function unnecessarily. They do 'play nice', although this function handling them as well is a bit redundant.
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...
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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