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

Navigation Block: Add missing menu item attributes to core/navigation #35634

Merged
merged 11 commits into from
Nov 2, 2021
Merged

Navigation Block: Add missing menu item attributes to core/navigation #35634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
08a0601
Navigation Block: add target, rel & title attributes
mikachan Oct 14, 2021
dae69cf
Nav block: change title to attr_title
mikachan Oct 15, 2021
00ec38e
Nav block: add classes attribute
mikachan Oct 15, 2021
d006b41
Nav block: add menu item ID
mikachan Oct 20, 2021
ce01a7f
Merge branch 'trunk' into fix/navigation-menu-item-attrs
mikachan Oct 20, 2021
691c15a
Nav block: replace className with classes
mikachan Oct 20, 2021
e0d8ae5
Nav block: add description, kind & type attributes
mikachan Oct 20, 2021
10181ce
Nav block: improve id attribute
mikachan Oct 20, 2021
376f071
Nav block: remove array to string conversion from navigation-link
mikachan Oct 22, 2021
198297d
Nav block: change classes to className
mikachan Oct 22, 2021
095c880
Merge branch 'trunk' into fix/navigation-menu-item-attrs
mikachan Oct 22, 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
6 changes: 0 additions & 6 deletions packages/block-library/src/navigation-link/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,6 @@ function render_block_core_navigation_link( $attributes, $content, $block ) {
$has_submenu = count( $block->inner_blocks ) > 0;
$is_active = ! empty( $attributes['id'] ) && ( get_the_ID() === $attributes['id'] );

$class_name = ! empty( $attributes['className'] ) ? implode( ' ', (array) $attributes['className'] ) : false;

if ( false !== $class_name ) {
$css_classes .= ' ' . $class_name;
}

Copy link
Contributor

@talldan talldan Oct 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@mikachan Just double-checking, should this have been removed?

I was thinking that the navigation-link/index.php would be able to remain unchanged from trunk.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was seeing some of the class names being duplicated, which is why I removed this. I can't see where they're being added originally though.

With $attributes['className']:

image

Without $attributes['className']:

image

faq-class (a custom test class I've added from Appearance > Menus), menu-item, menu-item-type-post_type, and menu-item-object-page are all duplicated when $attributes['className'] is used here. If I change the attribute name to something else (in both navigation-link/index.php and navigation/index.php), then the class names are only included once, so I'm assuming something else is picking up on className and adding them. If I var_dump $attributes['className'], it only includes unique classes, and I'm struggling to find where the others are added.

Ideally, I'd like to leave navigation-link/index.php unchanged and fix the duplication problem. If you have time, it would be great if you have any ideas around this!

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yep, you're right, good catch. It seems like a bug in trunk that a custom class name is applied twice (happens when using the navigation block normally in the block editor too), so this would be a fix.

I can't see where they're being added originally though.

I think it must be via the get_block_wrapper_attributes function, which is defined in WordPress core:
https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/class-wp-block-supports.php#L179

Since the block supports both generated and custom class names, those values get merged in like this:
https://github.com/WordPress/wordpress-develop/blob/b45c85a405ab0b1968380a79f7a76dc9293adc0f/src/wp-includes/block-supports/custom-classname.php#L44-L56

In that case it looks good to me, will approve the PR

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh brilliant, I understand, thanks so much for explaining. Nice, unexpected bug fix!

Thanks for your help with this! 🎉

$wrapper_attributes = get_block_wrapper_attributes(
array(
'class' => $css_classes . ' wp-block-navigation-item' . ( $has_submenu ? ' has-child' : '' ) .
Expand Down
18 changes: 16 additions & 2 deletions packages/block-library/src/navigation/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,25 @@ function gutenberg_parse_blocks_from_menu_items( $menu_items, $menu_items_by_par
$blocks = array();

foreach ( $menu_items as $menu_item ) {
$class_name = ! empty( $menu_item->classes ) ? implode( ' ', (array) $menu_item->classes ) : null;
$id = ( null !== $menu_item->object_id && 'custom' !== $menu_item->object ) ? $menu_item->object_id : null;
$opens_in_new_tab = null !== $menu_item->target && '_blank' === $menu_item->target;
$rel = ( null !== $menu_item->xfn && '' !== $menu_item->xfn ) ? $menu_item->xfn : null;
$kind = null !== $menu_item->type ? str_replace( '_', '-', $menu_item->type ) : 'custom';

$block = array(
'blockName' => 'core/navigation-link',
'attrs' => array(
'label' => $menu_item->title,
'url' => $menu_item->url,
'className' => $class_name,
'description' => $menu_item->description,
'id' => $id,
'kind' => $kind,
'label' => $menu_item->title,
'opensInNewTab' => $opens_in_new_tab,
'rel' => $rel,
'title' => $menu_item->attr_title,
'type' => $menu_item->object,
'url' => $menu_item->url,
),
);

Expand Down