-
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
[WIP] Allow setting allowedBlocks on Navigation block via attribute #52467
Conversation
This is an interesting PR. Searching through Gitbug we have a slew of issues about how allowed blocks in inner blocks should be improved:
Also I am unsure opening up the navigation block like in this PR is a good idea for another reason I remember stumbling upon recently: the rendered expects only certain kinds of blocks to be available and adds HTML wrappers depending on block name. Some plugin authors complained that their custom block would not get a wrapper. So if we want to allow customising the inner blocks of the navigation block I think we should lean more into changing how innerblocks themselves work - and also coming up with a renderer for inner blocks that always works. |
Size Change: +22 B (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
Yes there is a fair amount of demand for this.
Yes exactly what I discovered (see |
Related Issue to refactor PHP render code for the block should help here. |
Flaky tests detected in 347a3f0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5507037347
|
I think allowed blocks should be extendable as a root prop in block json not as an attribute. That would be a solution that solves for all blocks and something which must be supported 1st class by block renders (or explicitly opted out of). |
I don't have bandwidth to pursue this PR at this time. Moreover it sounds like this will be solved at a more systemic level. I will close out but feel free to reopen if anyone feels able to take it on. |
What?
PoC for allowing setting a set of allowed blocks on the Nav block using an attribute `allowedBlocks.
Closes #31387
Why?
Because people want to customise the Nav block. See #31387.
How?
Following existing examples such as #49128.
Problems
The current issue with this approach is that unlike many blocks, the Nav block actively manages it's inner blocks on server side render.
So instead of just "render these inner blocks" the Nav block will loop over the inner blocks and actively manage their markup. I believe this is for accessibility reasons.
There's probably a good case for moving away from this approach but that would need to be a seperate PR.
Testing Instructions
Problems
above.Testing Instructions for Keyboard
Screenshots or screencast