Skip to content
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

Block Locking: Register the 'lock' attribute on the server #40468

Merged
merged 3 commits into from
Apr 21, 2022

Conversation

Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented Apr 20, 2022

What?

Resolves #40461.

PR fixes an error when locking server-side rendered blocks.

Why?

The block-renderer endpoint validates passed attributes, but the lock block attribute was registered on the client-side.

How?

Use register_block_type_args filter to register the lock attribute.

Testing Instructions

  1. Open a Post or Page.
  2. Insert the Latest Comments or any block that uses the ServerSideRender component.
  3. Lock the block using block Options.
  4. Confirm there's no "This block has encountered an error and cannot be previewed" error.

Screenshots or screencast

Before After
CleanShot 2022-04-20 at 15 15 09 CleanShot 2022-04-20 at 15 15 25

@Mamaduka Mamaduka self-assigned this Apr 20, 2022
@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Feature] Block Locking The API allowing for the ability to lock/unlock blocks labels Apr 20, 2022
@Mamaduka
Copy link
Member Author

I'll create the WP core patch today or tomorrow 👍

@gziolo
Copy link
Member

gziolo commented Apr 20, 2022

The lock functionality isn't technically a part of the block supports layer so I would appreciate some sanity check on the idea from @youknowriad, @andrewserong, and @ramonjd who helped with WP 6.0 Beta 1 backports in that area.

@Mamaduka
Copy link
Member Author

@gziolo, the block locking UI can be controlled via block supports, but you're right attributes are registered for every block.

@gziolo
Copy link
Member

gziolo commented Apr 20, 2022

@gziolo, the block locking UI can be controlled via block supports, but you're right attributes are registered for every block.

With the full context, it looks for me like a more legit use case then 👍🏻

@youknowriad
Copy link
Contributor

I think we should include it by default in the "register" function like I said before, this is backend and frontend if needed as it's part of the "framework" and not an extension (which both hooks and block supports are).

So yeah, I think we should add it. I'd prefer it was baked in though but it's minor.

@gziolo
Copy link
Member

gziolo commented Apr 20, 2022

I think we should include it by default in the "register" function like I said before, this is backend and frontend if needed as it's part of the "framework" and not an extension (which both hooks and block supports are).

I guess you are referring to this function call:
https://github.com/WordPress/wordpress-develop/blob/8ad5aec839679804a81f29252b1307869d3d6d01/src/wp-includes/class-wp-block-type.php#L348-L371

We could also simplify a lot of code by assuming that syle and className should be always set as well.

@Mamaduka
Copy link
Member Author

I will update PR to use register_block_type_args and use :: set_props for core patch. We can move styles and className to use this new method as a follow-ups.

@Mamaduka Mamaduka force-pushed the fix/register-lock-attribute-on-server branch from 9d1cb2c to 1212c56 Compare April 20, 2022 19:24
@Mamaduka
Copy link
Member Author

@gziolo, updated registration method.

Copy link
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.

Yes, I think it's a good way to handle this attribute 👍🏻

lib/compat/wordpress-6.0/blocks.php Outdated Show resolved Hide resolved
@gziolo
Copy link
Member

gziolo commented Apr 21, 2022

I will update PR to use register_block_type_args and use :: set_props for core patch. We can move styles and className to use this new method as a follow-ups.

Yes, it's going to look different in WP core. styles and className is something that should be coordinated in WP 6.1 cycle with the ongoing work on the Style Engine.

@Mamaduka
Copy link
Member Author

I don't think React Native test failure is related to this change, so I'll merge after the Performance test is green.

@Mamaduka Mamaduka merged commit 2f0f6de into trunk Apr 21, 2022
@Mamaduka Mamaduka deleted the fix/register-lock-attribute-on-server branch April 21, 2022 10:22
@github-actions github-actions bot added this to the Gutenberg 13.2 milestone Apr 21, 2022
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Apr 26, 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.

Props mamaduka, peterwilsoncc.
See #55567.




git-svn-id: https://develop.svn.wordpress.org/trunk@53268 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Apr 26, 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.

Props mamaduka, peterwilsoncc.
See #55567.



Built from https://develop.svn.wordpress.org/trunk@53268


git-svn-id: http://core.svn.wordpress.org/trunk@52857 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this pull request Apr 26, 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.

Props mamaduka, peterwilsoncc.
See #55567.



Built from https://develop.svn.wordpress.org/trunk@53268


git-svn-id: https://core.svn.wordpress.org/trunk@52857 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Locking The API allowing for the ability to lock/unlock blocks [Type] Bug An existing feature does not function as intended
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Locking: ServerSideRendered block error
3 participants