Navigation: Handle block menu items #24846
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,13 @@ | ||
| /** | ||
| * External dependencies | ||
| */ | ||
| import { some } from 'lodash'; | ||
| import classnames from 'classnames'; | ||
|
|
||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { createBlock, parse } from '@wordpress/blocks'; | ||
| import { | ||
| Button, | ||
| CustomSelectControl, | ||
|
|
@@ -76,26 +77,51 @@ function getSelectedMenu( selectedCreateOption ) { | |
| /** | ||
| * A recursive function that maps menu item nodes to blocks. | ||
| * | ||
| * @param {Object[]} menuItems An array of menu items. | ||
| * @return {WPBlock[]} An array of blocks. | ||
| */ | ||
| function mapMenuItemsToBlocks( menuItems ) { | ||
| return menuItems.map( ( menuItem ) => { | ||
| if ( menuItem.type === 'block' ) { | ||
| const [ block ] = parse( menuItem.content.raw ); | ||
|
|
||
| if ( ! block ) { | ||
| return createBlock( 'core/freeform', { | ||
| content: menuItem.content, | ||
| } ); | ||
| } | ||
|
|
||
| return block; | ||
| } | ||
|
|
||
| const attributes = { | ||
| label: ! menuItem.title.rendered | ||
| ? __( '(no title)' ) | ||
| : menuItem.title.rendered, | ||
| opensInNewTab: menuItem.target === '_blank', | ||
| }; | ||
|
|
||
| if ( menuItem.url ) { | ||
| attributes.url = menuItem.url; | ||
| } | ||
|
|
||
| if ( menuItem.description ) { | ||
| attributes.description = menuItem.description; | ||
| } | ||
|
|
||
| if ( menuItem.xfn?.length && some( menuItem.xfn ) ) { | ||
| attributes.rel = menuItem.xfn.join( ' ' ); | ||
| } | ||
|
|
||
| if ( menuItem.classes?.length && some( menuItem.classes ) ) { | ||
| attributes.className = menuItem.classes.join( ' ' ); | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Ideally we'd also set There was a problem hiding this comment. I'm unclear on the reasoning behind dropping these attributes, what's the background? There was a problem hiding this comment. So the existing code in
Instead of fixing these issues, though, I added a note to address them when we address #18345 as there's some thinking to be done around what attributes are needed for dynamically rendering a link. IMO we should be storing three things with the block: the menu item type, the object type, the object ID. See #18345 (comment). There was a problem hiding this comment. Thanks for the explanation, sounds like it's not something to tackle here. It'd be good to rewrite the description on the issue with these details. 👍 |
||
| const innerBlocks = menuItem.children?.length | ||
| ? mapMenuItemsToBlocks( menuItem.children ) | ||
| : []; | ||
|
|
||
| return createBlock( 'core/navigation-link', attributes, innerBlocks ); | ||
| } ); | ||
| } | ||
|
|
||
|
|
||
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.
While this seems to work in testing, I think it'd be safest to also add
core/freeformto the allowedBlocks of the nav blockinnerBlockscomponent.In testing you can actually hack any block into the inner blocks of another block using the code editor (is this a bug?), but probably best not to rely on this behavior.
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 do wonder if, instead, we should simply fail (loudly?) in this case. It's undefined behaviour for the menu item to have 0 or > 1 blocks in it and probably indicates a bug elsewhere. Thoughts?
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.
Oh right, I think I've misunderstood what it's for. It actually looks like
parsegives you a freeform block anyway if it's valid HTML that doesn't contain a block.I think you're right, it should probably fail, but in a way that still makes the navigation recoverable. Not sure what the best option would be.