-
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
Group: Add allowedBlocks attribute and pass it to innerBlockProps #49128
Group: Add allowedBlocks attribute and pass it to innerBlockProps #49128
Conversation
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @sque89! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
304ad9a
to
5e8b1f4
Compare
Any news on this PR? Do i have to do something to get it reviewed? Just to be sure i am not missing something. Thanks! |
This is quiet a large and time consuming step 1 in order to test the PR:
Can you link to a premade plugin that does this? |
@carolinan thanks for reaching out. I don't have one in the plugin store but here i am attaching one as a zip file. It includes a block "Test" which has the proper configuration to test the positive scenario above (it only allows the core/paragraph block to be added inside the group of it). |
@carolinan sorry i don't want to stress on this too much, but it would help a lot in our current project. Do you think this can be merged soon? Thanks a lot. |
In my brief testing, the pull request works in the sense that with the plugin, the test block displays the templated heading and paragraph, and only allows inserting new paragraphs. I am not sure this is the correct solution or if the expectation is that developers should be able to add this via a filter. The PR needs to be rebased and the conflict needs to be solved. |
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 this is a great addition, and the mechanism is correct. Usually, we wouldn't want to change the allowed block list within the group globally. But only on a per-instance basis. Like in a pattern, only text, lists, and buttons should be allowed to be inserted.
But of course I will leave the final approach up to someone that is closer to the codebase
Hi, @sque89 I just returned from a little break and am still catching up with notifications. I'll try to review/test the PR properly this week. Meanwhile, let's resolve the failing static analysis check @carolinan suggested. If you have ESLint or Prettier add-on enabled in your code editor, they should handle similar issues automatically on save. See: https://developer.wordpress.org/block-editor/contributors/code/getting-started-with-code-contribution/#eslint Current error:
|
I see a similar approach used with the Cover and Columns blocks, so it should be fine if it works the same way. It'd be great to find out why there isn't a more general way available that all blocks with inner blocks could benefit from. |
@Mamaduka, it seems like it is close conceptually with block locking. Do you think it could become one day one of the options that you can set with UI when creating block patterns? Does it make sense to implement it similarly, as it works internally for block locking? |
I'm linking for reference an old issue #15682 where it wasn't possible to filter the list of allowed blocks. If we come up with a general solution here, we would gain options to address that limitation. |
Hi @Mamaduka so sorry to tag you here again, but since this change would help us so much in our current project, i just wanted to ask if you had the time to review this change and if i can support you in any way to go on with this. Thanks a lot for your time. |
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.
Thank you, @sque89. The changes here work as expected!
@gziolo, UI should be done when there's a "native" flow/screen for pattern creation. I also commented on the issue you've referenced regarding the general implementation.
@carolinan, you can also provide the allowedBlocks
attribute to any Group block for testing. Here's the markup I used for my tests:
<!-- wp:group {"allowedBlocks":["core/paragraph","core/heading","core/image"],"layout":{"type":"flex","orientation":"vertical"}} -->
<div class="wp-block-group"></div>
<!-- /wp:group -->
Screenshot
Congratulations on your first merged pull request, @sque89! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts: https://profiles.wordpress.org/me/profile/edit/ And if you don't have a WordPress.org account, you can create one on this page: https://login.wordpress.org/register Kudos! |
@sque89 If you have a moment, could you connect your GitHub account to your wordpress.org one? This change is slated to be included in the upcoming WordPress 6.3 release on August 8th, and it would be great if we could give you attribution by mentioning you on the credits screen. You can find more details on how contributions are tracked and how connecting GitHub accounts helps in this post on the Make WordPress Core blog. |
@desrosj thanks for reaching out. My Github wordpress account should be connected now. |
Great! Just confirmed. Thanks again for your contribution! |
What?
Adding the allowedBlocks attribute to the core/group block and pass it into the innerBlockProps to be considered by the InnerBlocks component
Why?
Being able to constraint the allowedBlocks for a group from an InnerBlocks template is crucial to provide a convenient yet failsafe editing environment
How?
By adding support of the allowedBlocks attribute to the core/group block a theme and/or block developer can then constraint the allowed blocks for the group
Testing Instructions
Positive Test
Negative Test
Testing Instructions for Keyboard
Screenshots or screencast