Skip to content

Register 'lock' attribute for every block on the server#2615

Closed
Mamaduka wants to merge 9 commits into
WordPress:trunkfrom
Mamaduka:fix/register-lock-attribute-on-server
Closed

Register 'lock' attribute for every block on the server#2615
Mamaduka wants to merge 9 commits into
WordPress:trunkfrom
Mamaduka:fix/register-lock-attribute-on-server

Conversation

@Mamaduka
Copy link
Copy Markdown
Member

@Mamaduka Mamaduka commented Apr 21, 2022

Backports changes from WordPress/gutenberg#40468.

The lock attribute needs to be supported by every block, but currently, it is only done on the client site. As a result, it was causing block rendered API requests to fail when blocks are locked.

PR updates the WP_Block_Type class to handle built-in/core attribute registration.

Trac ticket: https://core.trac.wordpress.org/ticket/55567

cc @gziolo, @hellofromtonya

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@Mamaduka Mamaduka changed the title Fix/register lock attribute on server Register 'lock' attribute for every block on the server Apr 21, 2022
Mamaduka
Mamaduka commented Apr 21, 2022
View reviewed changes
Comment thread src/wp-includes/class-wp-block-type.php Outdated
* @since 6.0.0
* @var array
*/
const CORE_ATTRIBUTES = array(
Copy link
Copy Markdown
Member Author

@Mamaduka Mamaduka Apr 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't decide between CORE_ATTRIBUTES and BUILTIN_ATTRIBUTES. I'm open to name suggestions :)

Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't decide between CORE_ATTRIBUTES and BUILTIN_ATTRIBUTES.

How about GLOBAL_ATTRIBUTES or GLOBAL_BLOCK_ATTRIBUTES?

Copy link
Copy Markdown
Member

@gziolo gziolo Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GLOBAL_ATTRIBUTES reads nice 👍🏻

@Mamaduka
Copy link
Copy Markdown
Member Author

Mamaduka commented Apr 21, 2022

The issue is from WP 5.9, but it's more visible now that we have UI.

gziolo
gziolo reviewed Apr 22, 2022
View reviewed changes
Comment thread src/wp-includes/class-wp-block-type.php Outdated
gziolo
gziolo reviewed Apr 22, 2022
View reviewed changes
Comment thread src/wp-includes/class-wp-block-type.php Outdated
@gziolo gziolo requested a review from peterwilsoncc April 22, 2022 16:20
peterwilsoncc
peterwilsoncc requested changes Apr 25, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a few notes inline, mainly in the tests to use more precise assertions and add the ticket number to the docblocks.

Comment thread src/wp-includes/class-wp-block-type.php Outdated
* @since 6.0.0
* @var array
*/
const CORE_ATTRIBUTES = array(
Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't decide between CORE_ATTRIBUTES and BUILTIN_ATTRIBUTES.

How about GLOBAL_ATTRIBUTES or GLOBAL_BLOCK_ATTRIBUTES?

Comment thread src/wp-includes/class-wp-block-type.php Outdated
Comment thread tests/phpunit/tests/blocks/wpBlockType.php
Comment thread tests/phpunit/tests/blocks/wpBlockType.php Outdated
Comment thread tests/phpunit/tests/blocks/wpBlockType.php Outdated
Comment thread tests/phpunit/tests/rest-api/rest-block-type-controller.php Outdated
Comment thread tests/phpunit/tests/rest-api/rest-block-type-controller.php Outdated
Comment thread tests/phpunit/tests/blocks/wpBlock.php Outdated
@gziolo @peterwilsoncc
Apply suggestions from code review
33343ad 
Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>
gziolo
gziolo reviewed Apr 25, 2022
View reviewed changes
Comment thread tests/phpunit/tests/blocks/wpBlockType.php
@gziolo
Copy link
Copy Markdown
Member

gziolo commented Apr 25, 2022 •
edited
Loading

Let's plan to land this patch tomorrow before WP 6.0 Beta 3. I see two remaining tasks:

  • CORE_ATTRIBUTES -> GLOBAL_ATTRIBUTES
  • setup for attributes should happen before the register_block_type_args filter

Mamaduka
Mamaduka commented Apr 26, 2022
View reviewed changes
Comment thread src/wp-includes/class-wp-block-type.php Outdated
gziolo
gziolo approved these changes Apr 26, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, this is in great shape. I will take care of committing to WP core 👍🏻

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Apr 26, 2022

Committed with https://core.trac.wordpress.org/changeset/53268.

@gziolo gziolo closed this Apr 26, 2022
@Mamaduka Mamaduka deleted the fix/register-lock-attribute-on-server branch April 28, 2022 06:56
@Mamaduka Mamaduka mentioned this pull request Oct 3, 2023
Merged
@ockham ockham mentioned this pull request Nov 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peterwilsoncc peterwilsoncc peterwilsoncc requested changes

@gziolo gziolo gziolo approved these changes

@hellofromtonya hellofromtonya Awaiting requested review from hellofromtonya

Assignees

No one assigned

Labels

None yet

Projects

No open projects
WordPress 6.0 Editor Tasks
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Mamaduka @gziolo @peterwilsoncc