Categories block: fix alignment #9829
Conversation
ZebulanStanphill
changed the title
|
Could this be milestoned for 4.1? It's been ready-to-merge (aside from having no reviews) for a month now. Also, the enhancement label is not really correct. This is a bug fix. |
| @@ -16,11 +16,6 @@ function render_block_core_categories( $attributes ) { | |||
| static $block_id = 0; | |||
| $block_id++; | |||
|
|
|||
| $align = 'center'; | |||
| if ( isset( $attributes['align'] ) && in_array( $attributes['align'], array( 'left', 'right', 'full' ), true ) ) { | |||
There was a problem hiding this comment.
I agree we shouldn't want to recreate validation in the render callback. Ideally this is validated at the attribute definition, i.e. via enum.
I expect this is encompassed in part by #2751.
Separately, we can plan to support enum validation in the client implementation. Maybe a near-term compromise is adding expected alignments as an enum in the attribute definition to be ready when this is "turned on". Though, ideally this would be handled automatically when opting into align supports, not the responsibility of each block implementer to define.
There was a problem hiding this comment.
The register_block_type PHP function can accept the same options as what exists in the client today. They are passed down and shallowly† merged with client registration. With #2751, the idea is that the server would be the primary location where blocks be registered.
† All of attributes must be defined in the server or in the client, not fragments amongst both.
There was a problem hiding this comment.
@aduth In that case, I guess registering attributes in PHP is the right move for blocks that already have an index.php. I think there are currently still some blocks that use server-side rendering but define their attributes in JS. I plan on making some PRs to fix that.
One more question, though. These blocks also use getEditWrapperProps in JS, so if you use enum to define the valid values in PHP, then you end up defining the valid alignments in 2 different places. So would it be a good idea to move getEditWrapperProps to index.php so that you can use and reference a constant defining the valid alignments in a single place, or is there a good reason for it to stay in index.js?
There was a problem hiding this comment.
My hope is to actually kill off getEditWrapperProps (example).
I don't think we want to be validating there either. As-is, I think it'd be sufficient to just pass through any non-undefined as data-align, not too much unlike how we've done with the frontend class rendering here?
Description
This PR fixes an issue where the Categories block would always use the
aligncenterclass regardless of what alignment you set. It also cleans up some of the code to make it more readable and consistent.Fixes #7910.