-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Limit block supports flag support to v2 #26106
Conversation
caa84d3
to
cfd94b5
Compare
lib/block-supports/index.php
Outdated
@@ -37,7 +37,7 @@ function gutenberg_apply_block_supports( $block_content, $block ) { | |||
|
|||
$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] ); | |||
// If no render_callback, assume styles have been previously handled. | |||
if ( ! $block_type || ! $block_type->render_callback ) { | |||
if ( ! $block_type || ! $block_type->apiVersion || ! $block_type->apiVersion < 2 || ! $block_type->render_callback ) { |
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.
The apiVersion
comes from block.json and is camel case (to match js rules), how did we solve this kind of issues in previous block.json properties cc @gziolo @mcsf @jorgefilipecosta
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.
We map everywhere to meet PHP coding style guidelines. It should be api_version
in the block type object instance. I'm not sure about other places though.
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.
So when we call register_block_type
explicitly, we should pass api_version
right?
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.
Correct, this is what regiser_block_type_from_metadata
does for other settings:
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.
Ok so for now, I explicitly added api_version
to the registration call. We should do a patch for Core to add the api_version to that array.
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.
Right, that works, too 👍
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.
I agree with translating to snake case in PHP and with subsequently patching register_block_type_from_metadata
.
Size Change: +17 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
cfd94b5
to
25d6ad5
Compare
@@ -522,6 +523,16 @@ class LatestPostsEdit extends Component { | |||
} | |||
} | |||
|
|||
function LatestPostBlockWrapper( props ) { |
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.
Ideally, we refactor the whole block to be a functional component, but this is better done on a separate PR. This should be enough for the moment.
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.
This makes sense.
I think this one is not needed anymore since the API is explicit now. |
To avoid impacting existing dynamic blocks, this PRs only enable block supports server-side for blocks using apiVersion 2.