Make block supports server-side explicit #26192
Conversation
|
Size Change: +3.51 kB (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
youknowriad
changed the title
youknowriad
requested review from
ajitbohra,
mkaz,
Soean,
spacedmonkey and
talldan
as code owners
youknowriad
added
[Feature] Block API
lib/compat.php
Outdated
| /** | ||
| * Shim that hooks into `pre_render_block` so as to override `render_block` with | ||
| * a function that assigns block context. | ||
| * | ||
| * This can be removed when plugin support requires WordPress 5.5.0+. | ||
| * FIXME: THIS IS NO LONGER ACCURATE: This can be removed when plugin support |
There was a problem hiding this comment.
If we keep $current_parsed_block, then this needs to be updated to reflect that:
- It needs backporting to core
- Then, it needs to live in compat.php until 5.6 is required by the plugin
|
|
||
| // This is hardcoded on purpose. | ||
| // We only support a fixed list of attributes. | ||
| $attributes_to_merge = array( 'style', 'class' ); |
There was a problem hiding this comment.
I don't remember off the top of my head how params that are objects/arrays are documented, but this seems worth adding in the PHPDoc. Even something as minimal as @param array $extra_attributes Optional. Extra classes or styles to render on the block wrapper. Use class or style as the array key, respectively..
There was a problem hiding this comment.
Hm, good catch. Is this filtering out any elements in $extra_attributes that aren't style or class? I think we should fix that. Even if keep that restriction for $new_attributes (which makes sense) I think we should lift it for $extra_attributes, otherwise it IMO goes against developer expectations.
(edit: I had added this comment in wrong place)
There was a problem hiding this comment.
The reason it's limited to a set of known attributes is because merging attributes is different from one to another. For example the id should overwrite while the style should concat.
There was a problem hiding this comment.
mmm, I was thinking only about documentation expectations. Is there a need for this function to pass-through attributes other than style and class? My thinking is that it'd be ok if any other element attributes the block author needs is handled by its own mechanisms in the render_callback function (so not in this function). As long as it's documented, I believe this is fine as it is.
| $wrapper_attributes = get_block_wrapper_attributes( | ||
| array( | ||
| 'class' => implode( ' ', $classes ), | ||
| 'style' => $colors['inline_styles'] . $font_sizes['inline_styles'], |
There was a problem hiding this comment.
TIL that NULL . NULL => ''. Nice simplification here!
| $icon = block_core_social_link_get_icon( $service ); | ||
| $wrapper_attributes = get_block_wrapper_attributes( array( 'class' => 'wp-social-link wp-social-link-' . $service . $class_name ) ); | ||
|
|
||
| return '<li .' . $wrapper_attributes . '><a href="' . esc_url( $url ) . '" aria-label="' . esc_attr( $label ) . '" ' . $attribute . '> ' . $icon . '</a></li>'; |
There was a problem hiding this comment.
There's a typo here, note the dot within the string <li ..
There was a problem hiding this comment.
Good catch, thanks for the PR
Builds on top of #26111
This PR introduces a new
get_block_wrapper_attributesfunction you need to call explicitly in render_functions in order to make sure the block supports work in the dynamic blocks. More details on the reasoning here #26111