Size Change: +895 B (+0.05%)

Total Size: 1.96 MB

_{compressed-size-action}

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org>
Co-authored-by: talldan <talldanwp@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: seifeldinio <seifradwane@git.wordpress.org>
Co-authored-by: jhmonroe <jhmonroe@git.wordpress.org>
Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: ndiego <ndiego@git.wordpress.org>
Co-authored-by: colorful-tones <colorful-tones@git.wordpress.org>
Co-authored-by: mtias <matveb@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Flaky tests detected in c4447a3.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18331165365
📝 Reported issues:

• [Flaky Test] should duplicate and activate #71988 in /test/e2e/specs/site-editor/template-activate.spec.js
• [Flaky Test] should refresh the page when stylesheet loading fails #71722 in /test/e2e/specs/interactivity/router-styles.spec.ts

The concern with this PR is that all available blocks are stored as attributes

That's only a problem if users are allowing most blocks, rather than allowing just a few (which from the issue seems to be the most likely scenario). So it's probably fine 😄

This is very cool. I could see this being really useful for patterns, possibly connected to the recent idea mentioned in #71887 about being able to specify a 'Free area' where only content blocks can be inserted.

There are a few of small bits of UI food for thought that I noticed (might be worth getting design feedback):

• I saw some mention in another PR that the block dropdown is getting very long, I wonder if this should live in 'Advanced' within the block inspector.
• It'd be nice to have a way to select all / deselect all blocks in the modal. If a user only wants to select a single block they currently have to unselect every category before selecting the one they want.
• I'm not sure this experience needs the "71 blocks are hidden." message like the preferences panel does.

It'd be great to get a designer's feedback on those things.

Thanks for the feedback!

should we update them all to use the block support instead?

Yes. In this PR, I've added support only to the Group block to let you try out the actual UI. If this UI works well, I plan to add this support to all relevant core blocks in a follow-up.

I saw some mention in another PR that the block dropdown is getting very long, I wonder if this should live in 'Advanced' within the block inspector.

That's right. The initial proposed design recommended the Advanced panel, but as mentioned in this comment, the UI order may be swapped. Do we try to reserve a special slot here for the allowedBlocks UI?

• It'd be nice to have a way to select all / deselect all blocks in the modal. If a user only wants to select a single block they currently have to unselect every category before selecting the one they want.
• I'm not sure this experience needs the "71 blocks are hidden." message like the preferences panel does.

Another thing that occurred to me in testing is that it would be really nice to be able to deselect all blocks with one click, especially for those cases where we only want to allow one or two blocks. Again, not a change for this PR, but something to consider.

I agree, maybe we need to extend the BlockManager component itself. I welcome design feedback!

Update: I created a private slot fill and moved the toggle button there so that it would always be rendered last in the Advanced panel. I also removed the {number} blocks are hidden. section, although it's still available in the preferences modal.

Designwise, looks good at a glance, especially after you added the help text. The partially duplicated title ("Allowed blocks" in the modal, "Manage allowed blocks" inside) is the only slightly uncanny thing. I realize that is based on the original design, the main difference being that the modal shown there has a gray border under its header, which made a bit of a difference. We shouldn't add that border back, and arguably what you have here is solid (so ship it). If we did want to try an alternative, it could be to have the modal be called "Manage allowed blocks", and have a single paragraph of text (not help text) at the top of the content. But that would create its own disconnect with the "Allowed blocks" button title.

In any case, looks good!

@jasmussen Thanks for the review! How about this?

Looks good, and that's what I pictured. But what you had also works. I think it probably boils down to the button that says "Allowed blocks", which is short, concise and a good label to translate without wrapping. If it's okay to keep that button title, but the modal saying something different, then I think it can work. If the modal title should be the same, I'd keep what you had before.

Any preference? Thanks as always.

As for the button, it might be better to match the design with the "Apply globally" button. For now, it looks like this:

I see what you mean, a subheading saying "Allowed blocks", which then beckons a different label below, such as "Manage", or "Manage allowed blocks". Note if we do the latter, I'd make it sentence case, "blocks".

On a separate note, it'd be nice to ensure we have text-wrap: pretty; in place for the helper text, so we avoid typographical widows. Not an action item here, just noting to self.

I decided to go with option "Manage allowed blocks". This might be better than "Manage" in terms of accessibility:

I don't mind the longer title, this is a context in which we can be verbose. The only risk is that translations become so long the text inside might wrap. Might be good to do a spot test of some of the more common long translations, such as German: "Zulässige Blöcke verwalten". If that can fit, we're probably good?

Might be good to do a spot test of some of the more common long translations, such as German: "Zulässige Blöcke verwalten"

Looks fine. If issues are reported in other locales, let's simplify it to "Manage".

Thanks for iterating! Just a small labelling issue I noticed below, and a question: I thought from the conversation we were matching the button text to the modal title? This is what I'm seeing on this branch:

Sorry, I forgot to push this change about modals. Fixed by 2e1e7b4.

Thanks for the review, everyone!

What do you think about extending this support to other core blocks? If it's required for the 6.9 release, I'd be happy to address it

What do you think about extending this support to other core blocks?

I think it makes sense to extend it, at a minimum, to other generic container blocks that already have the allowedBlocks attribute: Column, Cover, Details, Media & text.

I'd also love to see it in the Quote block because it's the same kind of generic container (the fact that it doesn't have the attribute currently seems like an oversight).