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

Navigation block: allow extending the list of allowed inner blocks #31387

Closed
Tracked by #35521
Aljullu opened this issue Apr 30, 2021 · 16 comments · May be fixed by #39492
Closed
Tracked by #35521

Navigation block: allow extending the list of allowed inner blocks #31387

Aljullu opened this issue Apr 30, 2021 · 16 comments · May be fixed by #39492
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Feature] Extensibility The ability to extend blocks or the editing experience [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@Aljullu
Copy link
Contributor

Aljullu commented Apr 30, 2021

The Navigation block has a hard-coded list of allowed inner blocks:

const ALLOWED_BLOCKS = [
'core/navigation-link',
'core/search',
'core/social-links',
'core/page-list',
'core/spacer',
];

But plugins might want to add more blocks to that list. For example, WooCommerce could add a Product Search block or a minicart block (ie: a link to the Cart which shows the number of products in the cart).

If I'm not wrong, there is no way plugins can do that right now, so this issue is about making the ALLOWED_BLOCKS list filterable or introducing some other API so plugins can extend the list of allowed blocks inside the Navigation block.

@Aljullu Aljullu added [Feature] Extensibility The ability to extend blocks or the editing experience [Block] Navigation Affects the Navigation Block labels Apr 30, 2021
@jasmussen
Copy link
Contributor

Good ticket. There's some overlap with #23745, perhaps specifically it's sort of blocked until we can take a stab at that one since the interface and rendered markup needs to be able to handle the added blocks available.

Here's an exploration of what happens if we just allow other blocks inside, and here's a mockup of future menu editing.

@mtias
Copy link
Member

mtias commented Oct 6, 2021

Would it make sense for the mini-cart case to define parent: 'core/navigation'?

@Aljullu
Copy link
Contributor Author

Aljullu commented Oct 6, 2021

Would it make sense for the mini-cart case to define parent: 'core/navigation'?

I wasn't aware that this option would work, thanks! It seems to work with Gutenberg trunk, but for some reason I couldn't get it to work on the latest released version (11.6.0).

The only issue of this approach, is that it doesn't allow blocks to be added inside the navigation and outside of it. Ie: a Product Search block could be displayed as part of the navigation or as a widget in the sidebar.

@gziolo
Copy link
Member

gziolo commented Oct 7, 2021

The only issue of this approach, is that it doesn't allow blocks to be added inside the navigation and outside of it. Ie: a Product Search block could be displayed as part of the navigation or as a widget in the sidebar.

It's an array so you could define several parent blocks where it can be inserted:

"parent": [ "core/navigation", "something/else" ],

We discussed that the current implementation is very strict, and it would be great to provide a way to support also "grandparents, "grandgrandparent", and so on 😄 Related issue: #30679.

@gziolo
Copy link
Member

gziolo commented Oct 7, 2021

By the way, this is how allowed blocks are handled for the Columns block with attributes:

"allowedBlocks": {
"type": "array"
},

Related PR: #35342.

@Aljullu
Copy link
Contributor Author

Aljullu commented Oct 7, 2021

Thanks for the links @gziolo!

It's an array so you could define several parent blocks where it can be inserted:

"parent": [ "core/navigation", "something/else" ],

That wouldn't still work for the usecase of the a block that I want to make available inside the Navigation block + anywhere else like the Product Search block, right? Or is there a way to say that any parent is allowed (something like "parent": [ "core/navigation", "*" ])?

@gziolo
Copy link
Member

gziolo commented Oct 7, 2021

Or is there a way to say that any parent is allowed (something like "parent": [ "core/navigation", "*" ])?

No, you need to list all parent blocks you want to support. It should be more flexible.

@kubiqsk
Copy link

kubiqsk commented Jun 2, 2023

Can we just add a filter hook?
Using parent is very limitating...
Imagine language switchers, dynamically generated category lists, more complex elements like minicarts, etc...
This is really needed, something like:

...
allowedBlocks: wp.hooks.applyFilters( 'navigation_allowed_blocks', ALLOWED_BLOCKS ),
...

in https://github.com/WordPress/gutenberg/blob/44e3a64e6e3e453cae644ff8ebd3ffd0f2963caa/packages/block-library/src/navigation/edit/inner-blocks.js

But it would be cool to have full control over the whole menu and stop limiting block, because then you can create megamenus and more complex structures...

@gziolo
Copy link
Member

gziolo commented Jun 2, 2023

We discuss finding a general solution to customize the list of allowed blocks for inner blocks with block attributes in #15682. So far, the handling was added whenever it was requested for blocks with inner blocks. You can see it in action for the following core blocks:

It should be fine to replicate the same approach for the Navigation block, too. It's something we definitely should support by default whenever there are inner blocks involved, as we do it for the template lock. I'm sure there are more features that would benefit from similar pattern.

@getdave
Copy link
Contributor

getdave commented Jul 10, 2023

It should be fine to replicate the same approach for the Navigation block

The Nav block has some additional complications in that it actively manages the output of it's inner blocks in the PHP server render of the block.

foreach ( $inner_blocks as $inner_block ) {

@fabiankaegy
Copy link
Member

@gziolo I tried your approach of defining parent on a custom block which works great for allowing you to insert that custom block into the navigation block.

However the issue we are now running into is what @getdave described above. The navigation block does some magic to move some inner blocks into a special wrapper. The list of the blocks that get this treatment is again hardcoded in PHP.

Ideally I would love for this Array in PHP to either be filterable or for the solution to not check for the block name but instead the markup produced by the block. If the merkup is an li element then it should get moved into the ul. If not, it shouldn't :)

@getdave
Copy link
Contributor

getdave commented Oct 23, 2023

@fabiankaegy I think a PR which utilises the Tag Processor to find the <li> (rather than rely on the hardcoded list of blocks) might work well as you suggest.

@fabiankaegy
Copy link
Member

@getdave I created a draft PR for that here: #55551 doing some final testing on my end but wanted to add it to your radar still :) Thanks for confirming

@annezazu
Copy link
Contributor

annezazu commented Feb 5, 2024

Is this solved by this recent PR that adds an allowedBlocks field to block.json to specify allowed children? #58262

@fabiankaegy
Copy link
Member

@annezazu I would say so 👍 Though as mentioned in other places there still is an issue that I think needs adressing regarding the feature: #55551

@Aljullu
Copy link
Contributor Author

Aljullu commented Feb 5, 2024

Oh yes, that's solved for our use case. I'm going to close this issue. Thanks for the heads-up, @annezazu!

@Aljullu Aljullu closed this as completed Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Feature] Extensibility The ability to extend blocks or the editing experience [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
Status: Done
9 participants