-
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
Enable Block Renaming support for (almost) all blocks #54426
Changes from all commits
79d3185
e0002d0
9bc73ec
6aa7286
58042cd
739747a
8a475e4
87a9ca6
1d7184c
c009417
eda762b
90a202f
96bc51c
ebb8bdc
d1509a7
d55ba83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,19 +2,8 @@ | |
* WordPress dependencies | ||
*/ | ||
import { addFilter } from '@wordpress/hooks'; | ||
import { getBlockSupport } from '@wordpress/blocks'; | ||
|
||
const META_ATTRIBUTE_NAME = 'metadata'; | ||
|
||
export function hasBlockMetadataSupport( blockType, feature = '' ) { | ||
// Only core blocks are allowed to use __experimentalMetadata until the fetaure is stablised. | ||
if ( ! blockType.name.startsWith( 'core/' ) ) { | ||
return false; | ||
} | ||
const support = getBlockSupport( blockType, '__experimentalMetadata' ); | ||
return !! ( true === support || support?.[ feature ] ); | ||
} | ||
|
||
/** | ||
* Filters registered block settings, extending attributes to include `metadata`. | ||
* | ||
|
@@ -29,39 +18,18 @@ export function addMetaAttribute( blockTypeSettings ) { | |
return blockTypeSettings; | ||
} | ||
|
||
const supportsBlockNaming = hasBlockMetadataSupport( | ||
blockTypeSettings, | ||
'name' | ||
); | ||
|
||
if ( supportsBlockNaming ) { | ||
blockTypeSettings.attributes = { | ||
...blockTypeSettings.attributes, | ||
[ META_ATTRIBUTE_NAME ]: { | ||
type: 'object', | ||
}, | ||
}; | ||
} | ||
Comment on lines
-32
to
-44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change |
||
blockTypeSettings.attributes = { | ||
...blockTypeSettings.attributes, | ||
[ META_ATTRIBUTE_NAME ]: { | ||
type: 'object', | ||
}, | ||
}; | ||
getdave marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return blockTypeSettings; | ||
} | ||
|
||
export function addSaveProps( extraProps, blockType, attributes ) { | ||
if ( hasBlockMetadataSupport( blockType ) ) { | ||
extraProps[ META_ATTRIBUTE_NAME ] = attributes[ META_ATTRIBUTE_NAME ]; | ||
} | ||
|
||
return extraProps; | ||
} | ||
|
||
Comment on lines
-49
to
-56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needed to be removed to allow tests to pass otherwise an additional All functionality and tests appears to work without it. I think it was mistakenly included in the original PR. |
||
addFilter( | ||
'blocks.registerBlockType', | ||
'core/metadata/addMetaAttribute', | ||
addMetaAttribute | ||
); | ||
|
||
addFilter( | ||
'blocks.getSaveContent.extraProps', | ||
'core/metadata/save-props', | ||
addSaveProps | ||
); |
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.
Specifically @ntsekouras this is where it is being promoted to non-experimental. This doesn't actually change much other than it's no longer in the
experimental
directory.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 think having the
metadata
support is okay and has been discussed in many other issues, so it could be helpful. Should we add it in a different php file underblock-supports
folder though?Additionally I believe we should update the support docs.
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.
Yeh I was wondering that. However this is no longer a block supports is it? It's just a feature of every block.
If so then maybe it should be somewhere else in a more fundamental place lower down in the code?
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.
@ntsekouras I wonder whether infact it would be better to register the block
metadata
attribure as a global attribute available to every block such as seems to have been done forlock
.https://github.com/WordPress/wordpress-develop/blob/a80165b78f2367f92e2eff679c4e035d1c5bdc7c/src/wp-includes/class-wp-block-type.php#L248-L250
Maybe @Mamaduka can shed some light on the best approach here? 🙏
Remember we are discussing the
metadata
attribute and not the block support forblockRenaming
. I say this because it's a confusing PR in that regard 😅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.
Global attributes seems a good place for this, yes.
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 will handle in #55194
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.
@ockham It looks like you backported this to Core in https://core.trac.wordpress.org/changeset/57068.
I was going to say we can remove this code but would that cause backwards compat with versions of WP Core which don't have the backported code but are running the Plugin?
If so it seems to be gated code:
if ( ! array_key_exists( 'metadata', $args['attributes'] ) ) {
...so we should probably move to a
compat/wordpress-*
directory? What do you think?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.
@getdave I think that makes sense!
compat/wordpress-6.5/
sounds like the right place for this 👍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've been looking everywhere for this comment 😅
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.
#58126