Sync packages for WP 6.1 RC 1 (with Fluid Typography) #3437
Conversation
There was a problem hiding this comment.
I think this code is mostly OK as-is, aside from updating the function prefix from gutenberg to wp.
My comments on the inline styles aren't a blocker for 6.1 RC 1, but I think this code should be refactored. There's actually a lot of this pattern in(at least) the search block code, i.e. check if value for specific key in array has a value, and, if so, a similar task to several other keys.
| $font_sizes['inline_styles'] = sprintf( 'font-size: %s;', $context['style']['typography']['fontSize'] ); | ||
| $font_sizes['inline_styles'] = sprintf( | ||
| 'font-size: %s;', | ||
| gutenberg_get_typography_font_size_value( |
There was a problem hiding this comment.
| gutenberg_get_typography_font_size_value( | |
| wp_get_typography_font_size_value( |
| $font_sizes['inline_styles'] = sprintf( 'font-size: %s;', $context['style']['typography']['fontSize'] ); | ||
| $font_sizes['inline_styles'] = sprintf( | ||
| 'font-size: %s;', | ||
| gutenberg_get_typography_font_size_value( |
There was a problem hiding this comment.
| gutenberg_get_typography_font_size_value( | |
| wp_get_typography_font_size_value( |
| $font_sizes['inline_styles'] = sprintf( 'font-size: %s;', $context['style']['typography']['fontSize'] ); | ||
| $font_sizes['inline_styles'] = sprintf( | ||
| 'font-size: %s;', | ||
| gutenberg_get_typography_font_size_value( |
There was a problem hiding this comment.
| gutenberg_get_typography_font_size_value( | |
| wp_get_typography_font_size_value( |
| $typography_styles[] = sprintf( 'font-size: %s;', esc_attr( $attributes['style']['typography']['fontSize'] ) ); | ||
| $typography_styles[] = sprintf( | ||
| 'font-size: %s;', | ||
| gutenberg_get_typography_font_size_value( |
There was a problem hiding this comment.
| gutenberg_get_typography_font_size_value( | |
| wp_get_typography_font_size_value( |
| if ( ! empty( $attributes['style']['typography']['fontFamily'] ) ) { | ||
| $typography_styles[] = sprintf( 'font-family: %s;', esc_attr( $attributes['style']['typography']['fontFamily'] ) ); | ||
| $typography_styles[] = sprintf( 'font-family: %s;', $attributes['style']['typography']['fontFamily'] ); | ||
| } | ||
|
|
||
| if ( ! empty( $attributes['style']['typography']['letterSpacing'] ) ) { | ||
| $typography_styles[] = sprintf( 'letter-spacing: %s;', esc_attr( $attributes['style']['typography']['letterSpacing'] ) ); | ||
| $typography_styles[] = sprintf( 'letter-spacing: %s;', $attributes['style']['typography']['letterSpacing'] ); | ||
| } | ||
|
|
||
| if ( ! empty( $attributes['style']['typography']['fontWeight'] ) ) { | ||
| $typography_styles[] = sprintf( 'font-weight: %s;', esc_attr( $attributes['style']['typography']['fontWeight'] ) ); | ||
| $typography_styles[] = sprintf( 'font-weight: %s;', $attributes['style']['typography']['fontWeight'] ); | ||
| } | ||
|
|
||
| if ( ! empty( $attributes['style']['typography']['fontStyle'] ) ) { | ||
| $typography_styles[] = sprintf( 'font-style: %s;', esc_attr( $attributes['style']['typography']['fontStyle'] ) ); | ||
| $typography_styles[] = sprintf( 'font-style: %s;', $attributes['style']['typography']['fontStyle'] ); | ||
| } | ||
|
|
||
| if ( ! empty( $attributes['style']['typography']['lineHeight'] ) ) { | ||
| $typography_styles[] = sprintf( 'line-height: %s;', esc_attr( $attributes['style']['typography']['lineHeight'] ) ); | ||
| $typography_styles[] = sprintf( 'line-height: %s;', $attributes['style']['typography']['lineHeight'] ); | ||
| } | ||
|
|
||
| if ( ! empty( $attributes['style']['typography']['textTransform'] ) ) { | ||
| $typography_styles[] = sprintf( 'text-transform: %s;', esc_attr( $attributes['style']['typography']['textTransform'] ) ); | ||
| $typography_styles[] = sprintf( 'text-transform: %s;', $attributes['style']['typography']['textTransform'] ); |
There was a problem hiding this comment.
I feel like this code could be simplified to one loop over all elements in $attributes['style']['typography'].
Inside the loop, we could abstract out the building of the inline styles to another function. Also, if there are more style props added, this code would only need to be changed if there's some sort of unique transform needed on the value, e.g. fontSize key above on line 447 passes its value through wp_get_typography_font_size_value.
Example:
$allowed_keys = array(
'fontFamily',
'letterSpacing',
'fontWeight',
'fontStyle',
'lineHeight',
'textTransform',
);
if ( isset( $attributes['style']['typography'] ) ) {
foreach ( $attributes['style']['typography'] as $key => $value ) {
if ( in_array( $key, $allowed_keys, true ) && ! empty( $attributes['style']['typography'][ $key ] ) ) {
$typography_styles[] = sprintf(
'%1$s: %2$s;',
_wp_to_kebab_case( $key ),
$attributes['style']['typography'][ $key ]
);
}
}
}
There was a problem hiding this comment.
Edited to add an allowlist of known keys, to avoid parsing bad ones.
Good catch! Since this is dynamic block PHP, we'll have to do that in Gutenberg, as that code is sync'ed from the |
|
Merged into core in https://core.trac.wordpress.org/changeset/54483. |
I believe @dream-encode merged the version without the Fluid Typography changes (#3434). Reopening 😅 |
|
Ah, looks like it was actually this one that was merged! Apologies for the confusion. Closing again; #3439 is the follow-up to prevent the |
Alternative to #3434. It's also possible to merge #3434 first, and then merge this PR, which contains all the changes from #3434, plus the Fluid Typography fixes (see WordPress/gutenberg#44758 and #3431).
Includes the latest round of Gutenberg bugfix PRs that were approved for inclusion during yesterday’s triage session. The following Gutenberg PRs were cherry-picked:
Furthermore, this includes the Fluid Typography fixes listed in WordPress/gutenberg#44758.
Needed for today's WP 6.1 RC 1 release.
Trac ticket: https://core.trac.wordpress.org/ticket/56467
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.