Navigation: Allow additional CSS classes #18466
Conversation
obenland
added
[Type] Bug
obenland
requested review from
ajitbohra,
jorgefilipecosta,
Soean and
talldan
as code owners
|
It'll be empty if there are no custom colors selected and shouldn't have any detrimental effect on display or performance of the block. |
|
Can't we not have it? Other blocks seem to manage this very well and avoid empty attributes at all times in what I have seen so far :) |
Fixes a bug where additional CSS classes were not passed to the front-end.
obenland
force-pushed
the
fix/additional-class
branch
from
211d52b
to
e8dcabc
Compare
|
Glad to see this being fixed. I was adding in some style variations and found this issue. |
| $colors = build_css_colors( $attributes ); | ||
| $class_attribute = sprintf( ' class="%s"', esc_attr( $colors['css_classes'] ? 'wp-block-navigation-menu ' . $colors['css_classes'] : 'wp-block-navigation-menu' ) ); | ||
| $style_attribute = $colors['inline_styles'] ? sprintf( ' style="%s"', esc_attr( $colors['inline_styles'] ) ) : ''; | ||
| $colors = build_css_colors( $attributes ); |
There was a problem hiding this comment.
Is this function build_css_colors not intended to be namespaced / prefixed specific to the plugin / block / WordPress? (i.e. is it meant to be for general usage?)
There was a problem hiding this comment.
Is there a namespacing standard for PHP functions in Gutenberg?
There was a problem hiding this comment.
Is there a namespacing standard for PHP functions in Gutenberg?
Now that you mention it, not explicitly.
For most functions, we follow best practices in using a gutenberg_ prefix for plugin-specific functions.
It's a little trickier for these blocks though, since they're copied verbatim into core. So the concern is in considering that, should this be merged to core, these become new global WordPress functions.
Most other dynamic blocks follow some convention of render_block_foo and register_block_foo, but there's definitely some (unfortunate) ad hoc naming for similar utility functions (example).
It's definitely not a blocker for merging this pull request, since it wasn't introduced here, but we should see about establishing a convention here that makes sense.
There was a problem hiding this comment.
namespace Gutenberg; 😱
Fixes a bug where additional CSS classes were not passed to the front-end.
How has this been tested?
When creating a navigation menu, add a CSS class in block settings.
Preview the block and make sure the class gets applied to the
navelement.Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: