-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Blocks API: Add 'lock' attribute during block registration #35389
Conversation
Size Change: -9 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
@@ -365,6 +366,17 @@ export function registerBlockType( blockNameOrMetadata, settings ) { | |||
return; | |||
} | |||
|
|||
// Handle `lock` as a "build-in" attribute. | |||
// But allow blocks to specify their own with defaults. | |||
if ( ! has( settings.attributes, [ 'lock', 'type' ] ) ) { |
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 noticed while updating unit tests that there's an expectation for empty attributes if the block doesn't have them.
Should I update the condition and only add the lock
attribute if attributes aren't empty?
Changes on the client are not enough if we want to make the |
Good point, @gziolo. Currently, we inject at least nine attributes using filters. I also would prefer a unified way to handle "build-in" attribues. |
The difference is that the attributes added by the "hooks" are not used in other parts of the code other than the "hooks" so even if they live in ( |
I guess the framework could provide a fallback when an attribute is not set, too. I don't have a strong opinion on that though. I don't know why On the PHP side, we will have to change a few things starting from the default value for If I recall correctly, there was a valid reason to use |
@gziolo and @youknowriad is this PR blocked? I see the logic of having this as a built in but why should it be added to the registration process? can't the logic that relies on it just assume if So I'm leaning more toward this:
|
Is this a feature/enhancement that should be added to WordPress 6.0? |
I think we can close this one. The current method for registering the attribute works well. |
We can always reopen if necessary 👍 |
Description
Treat
lock
as a "build-in" attribute and include it during block registration. Removes filter previously used for adding this attribute.Based on #32457 (comment).
Part of #29864.
How has this been tested?
Check that unit tests are passing.
Example block
Types of changes
Enhancement
Checklist:
*.native.js
files for terms that need renaming or removal).