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.

Sign up for GitHub

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

(Optionally) rendering classic navigation data source in Navigation B… #30852

Closed
wants to merge 13 commits into from
Closed

(Optionally) rendering classic navigation data source in Navigation B… #30852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9567968
(Optionally) rendering classic navigation data source in Navigation B…
pbking Apr 14, 2021
1d0e4c6
Refactored to use 'block-nav-menus'
pbking Apr 15, 2021
7f4d146
Removed the 'wrapping' of navigation block (since it's done via 'bloc…
pbking Apr 15, 2021
8decddd
Passed block attributes through to the generated block.
pbking Apr 15, 2021
0cb51ac
Removed 'source' from attributes passed to renderer
pbking Apr 16, 2021
26d665a
Fix PHP linting errors
pbking Apr 16, 2021
2046285
Refactored to use 'location'
pbking Apr 22, 2021
c96541d
Tinkering with editor interface with location set
pbking Apr 22, 2021
e56e59b
Add real menu locations from the theme
scruffian Apr 23, 2021
a4cdf72
Select the correct menu using the dropdown
scruffian Apr 23, 2021
30b12aa
Added 'edit' link to navigation
pbking Apr 23, 2021
b9147cb
Added 'edit' link to navigation
pbking Apr 23, 2021
71be90c
Added controls to switch navigation block to and from classic target …
pbking Apr 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/navigation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,14 @@ function gutenberg_output_block_nav_menu( $output, $args ) {
$menu_items_by_parent_id[ $menu_item->menu_item_parent ][] = $menu_item;
}

$block_attributes = array();
if ( isset( $args->block_attributes ) ) {
$block_attributes = $args->block_attributes;
}
Comment on lines +303 to +305
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably just do this inline with the array below?


$navigation_block = array(
'blockName' => 'core/navigation',
'attrs' => array(),
'attrs' => $block_attributes,
'innerBlocks' => gutenberg_convert_menu_items_to_blocks(
isset( $menu_items_by_parent_id[0] )
? $menu_items_by_parent_id[0]
Expand Down
3 changes: 3 additions & 0 deletions packages/block-library/src/navigation/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
"name": "core/navigation",
"category": "design",
"attributes": {
"location": {
"type": "string"
},
"orientation": {
"type": "string"
},
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/navigation/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ function Navigation( {
return (
<div { ...blockProps }>
<NavigationPlaceholder
location={ attributes.location }
setAttributes={ setAttributes }
onCreate={ ( blocks, selectNavigationBlock ) => {
setIsPlaceholderShown( false );
updateInnerBlocks( blocks );
Expand Down
37 changes: 36 additions & 1 deletion packages/block-library/src/navigation/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,41 @@ function block_core_navigation_build_css_font_sizes( $attributes ) {
return $font_sizes;
}

/**
* Renders a Navigation Block derived from data from the theme_location assigned
* via the block attribute 'location'.
*
* If the theme doesn't explicity support 'block-nav-menus' or no location was provided
* as a block attribute then an empty string is returned.
*
* @param array $attributes Navigation block attributes.
* @return string HTML markup of a generated Navigation Block.
*/
function get_classic_navigation_elements( $attributes ) {

if ( ! current_theme_supports( 'block-nav-menus' ) ) {
return '';
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the role of checking for this here? The expected use of this supports is kind of the opposite, as here the result is rendering a classic menu, while the supports is meant to enable rendering of a block menu.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose isn't to render a classic menu, the purpose is to render a BLOCK menu with a classic data source. That flag ensures that what comes out of the call to render the menu is navigation block markup. Ideally it would be preferred to not require that theme supports and simply (in this situation) ALWAYS render block markup. But this was the cleanest path to done at the moment.


if ( ! array_key_exists( 'location', $attributes ) ) {
return '';
}

$block_attributes = $attributes;
unset( $block_attributes['location'] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow the swapping values and the unset. It's probably related to the updates in wp_nav_menu but at least it needs a comment, and ideally we don't need to do it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When crafting a Navigation Block that leverages a classic navigation data source you will still be able to apply stylistic attributes (justification, font size, etc). These attributes should be passed in when it actually renders. HOWEVER if the 'location' attribute is set when the rendering is attempted, and there are no menu items to be rendered, the block enters a loop. This ensures that the attributes passed to actually be rendered exclude the 'location' attribute preventing the loop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a problem to be fixed in the block then :)


return wp_nav_menu(
array(
'theme_location' => $attributes['location'],
'block_attributes' => $block_attributes,
'container' => '',
'items_wrap' => '%3$s',
'fallback_cb' => false,
'echo' => false,
)
);
}

/**
* Returns the top-level submenu SVG chevron icon.
*
Expand Down Expand Up @@ -122,7 +157,7 @@ function render_block_core_navigation( $attributes, $content, $block ) {
unset( $attributes['rgbTextColor'], $attributes['rgbBackgroundColor'] );

if ( empty( $block->inner_blocks ) ) {
return '';
return get_classic_navigation_elements( $attributes );
}

$colors = block_core_navigation_build_css_colors( $attributes );
Expand Down
173 changes: 132 additions & 41 deletions packages/block-library/src/navigation/placeholder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ function convertMenuItemsToBlocks( menuItems ) {
return mapMenuItemsToBlocks( menuTree );
}

function NavigationPlaceholder( { onCreate }, ref ) {
function NavigationPlaceholder( { location, onCreate, setAttributes }, ref ) {
const [ selectedMenu, setSelectedMenu ] = useState();

const [ isCreatingFromMenu, setIsCreatingFromMenu ] = useState( false );
Expand All @@ -108,6 +108,7 @@ function NavigationPlaceholder( { onCreate }, ref ) {
isResolvingPages,
hasResolvedPages,
menus,
menuLocations,
isResolvingMenus,
hasResolvedMenus,
menuItems,
Expand All @@ -118,6 +119,7 @@ function NavigationPlaceholder( { onCreate }, ref ) {
getEntityRecords,
getMenus,
getMenuItems,
getMenuLocations,
isResolving,
hasFinishedResolution,
} = select( coreStore );
Expand Down Expand Up @@ -167,6 +169,7 @@ function NavigationPlaceholder( { onCreate }, ref ) {
menuItemsParameters
)
: false,
menuLocations: getMenuLocations(),
};
},
[ selectedMenu ]
Expand Down Expand Up @@ -216,6 +219,131 @@ function NavigationPlaceholder( { onCreate }, ref ) {
isPrimary: true,
className: 'wp-block-navigation-placeholder__actions__dropdown',
};

const locationPlaceholder = () => {
if ( ! location || ! menuLocations ) {
return;
}

const selectedMenuLocation = menuLocations.find(
( menuLocation ) => menuLocation.name === location
);

if ( ! selectedMenuLocation ) {
return;
}

return (
<>
{ ' ' }
<DropdownMenu
text={ selectedMenuLocation.description }
icon={ chevronDown }
toggleProps={ toggleProps }
>
{ ( { onClose } ) => (
<MenuGroup>
{ menuLocations.map( ( { name, description } ) => {
return (
<MenuItem
onClick={ () => {
setAttributes( { location: name } );
onClose();
} }
isSelected={ name === location }
onClose={ onClose }
key={ name }
>
{ description }
</MenuItem>
);
} ) }
</MenuGroup>
) }
</DropdownMenu>
<Button
isTertiary
href={ `/wp-admin/nav-menus.php?action=edit&menu=${ selectedMenuLocation.menu }` }
>
{ __( 'Edit' ) }
</Button>
<Button isTertiary onClick={ convertToBlocks }>
{ __( 'Convert to Blocks' ) }
</Button>
</>
);
};

const convertToBlocks = () => {
setAttributes( { location: null } );
};

const convertToClassic = ( newLocation ) => {
setAttributes( { location: newLocation.name } );
};

const defaultPlaceholder = () => {
if ( location ) {
return;
}

const defaultClassicMenuLocation = menuLocations?.[ 0 ];

return (
<>
{ hasMenus ? (
<DropdownMenu
text={ __( 'Existing menu' ) }
icon={ chevronDown }
toggleProps={ toggleProps }
>
{ ( { onClose } ) => (
<MenuGroup>
{ menus.map( ( menu ) => {
return (
<MenuItem
onClick={ () => {
setSelectedMenu( menu.id );
onCreateFromMenu();
} }
onClose={ onClose }
key={ menu.id }
>
{ menu.name }
</MenuItem>
);
} ) }
</MenuGroup>
) }
</DropdownMenu>
) : undefined }

{ hasPages ? (
<Button
isPrimary={ hasMenus ? false : true }
isTertiary={ hasMenus ? true : false }
onClick={ onCreateAllPages }
>
{ __( 'Add all pages' ) }
</Button>
) : undefined }
<Button isTertiary onClick={ onCreateEmptyMenu }>
{ __( 'Start empty' ) }
</Button>
{ !! defaultClassicMenuLocation ? (
<Button
isTertiary
onClick={ () => {
convertToClassic( defaultClassicMenuLocation );
} }
>
{ __( 'Use classic' ) }
</Button>
) : null }
</>
);
};

return (
<div className="wp-block-navigation-placeholder">
<PlaceholderPreview />
Expand All @@ -234,46 +362,9 @@ function NavigationPlaceholder( { onCreate }, ref ) {
<div className="wp-block-navigation-placeholder__actions__indicator">
<Icon icon={ navigation } /> { __( 'Navigation' ) }
</div>
{ hasMenus ? (
<DropdownMenu
text={ __( 'Existing menu' ) }
icon={ chevronDown }
toggleProps={ toggleProps }
>
{ ( { onClose } ) => (
<MenuGroup>
{ menus.map( ( menu ) => {
return (
<MenuItem
onClick={ () => {
setSelectedMenu(
menu.id
);
onCreateFromMenu();
} }
onClose={ onClose }
key={ menu.id }
>
{ menu.name }
</MenuItem>
);
} ) }
</MenuGroup>
) }
</DropdownMenu>
) : undefined }
{ hasPages ? (
<Button
isPrimary={ hasMenus ? false : true }
isTertiary={ hasMenus ? true : false }
onClick={ onCreateAllPages }
>
{ __( 'Add all pages' ) }
</Button>
) : undefined }
<Button isTertiary onClick={ onCreateEmptyMenu }>
{ __( 'Start empty' ) }
</Button>

{ locationPlaceholder() }
{ defaultPlaceholder() }
</div>
) }
</div>
Expand Down