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 should support current-menu-item #19993

Closed
Bowriverstudio opened this issue Jan 31, 2020 · 11 comments · Fixed by #20076
Closed

Navigation Block should support current-menu-item #19993

Bowriverstudio opened this issue Jan 31, 2020 · 11 comments · Fixed by #20076
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Status] In Progress Tracking issues with work in progress

Comments

@Bowriverstudio
Copy link

In Gutenberg 7.3 I've added the navigation block.

When I'm on the Current Page the navigation block does not add the css class "current-menu-item".

Describe alternatives you've considered
In my case I've got four pages. So I just created a navigation block for each page, and added custom css.

Thank you.

@aristath
Copy link
Member

aristath commented Jan 31, 2020

I was thinking of the exact same thing for a couple of weeks... Right now the nav block is nothing more than a styled links list.
In order to become a true navigation, it needs to show the current page (which is also important for a11y).

Wrote this today, didn't have time to test it yet but it's a start:

add_filter( 'render_block_data', function( $block, $source ) {

	// Early exit if innerBlocks doesn't exist.
	if ( ! isset( $block['innerBlocks'] ) ) {
		return $block;
	}

	foreach ( $block['innerBlocks'] as $key => $innerAttrs ) {

		// Skip if we're not in a nav-link.
		if ( 'core/navigation-link' !== $innerAttrs['blockName'] ) {
			continue;
		}

		// Skip if ID is defined and not the same as the current page.
		if ( isset( $innerAttrs['attrs']['id'] ) && get_the_ID() !== $innerAttrs['attrs']['id'] ) {
			continue;
		}

		// Skip if URL is defined and not the same as the current URL.
		if ( isset( $innerAttrs['attrs']['url'] ) && get_permalink() !== $innerAttrs['attrs']['url'] ) {
			continue;
		}

		// If we got this far, we need to add the classname.
		// First make sure it is defined.
		if ( ! isset( $block['innerBlocks'][ $key ]['attrs']['className'] ) ) {
			$block['innerBlocks'][ $key ]['attrs']['className'] = '';
		}

		// Add the classname.
		$block['innerBlocks'][ $key ]['attrs']['className'] .= ' active';
	}

	return $block;
}, 10, 2 );

The logic is pretty simple:

  1. Go through the block params, looping innerBlocks
  2. If the inner-block is a navigation-link, check if it has the same ID and/or URL as the current page we're on.
  3. If the above tests pass then add an active class to the navigation-link.

I'll work on this a bit during the weekend, the above is just an idea (I don't even know if it works at this point).


EDIT: The above code wasn't working because of a bug, see updated comment with code below.

@aristath
Copy link
Member

aristath commented Feb 1, 2020

This solution is a bit hacky, but it works:

add_filter( 'render_block_data', function( $block ) {

	// Early exit if not a nav block.
	if ( 'core/navigation' !== $block['blockName'] ) {
		return $block;
	}

	foreach ( array_keys( $block['innerBlocks'] ) as $key ) {

		// Skip if ID is defined and not the same as the current page.
		if ( isset( $block['innerBlocks'][ $key ]['attrs']['id'] ) && get_the_ID() !== $block['innerBlocks'][ $key ]['attrs']['id'] ) {
			continue;
		}

		// Skip if URL is defined and not the same as the current URL.
		if ( isset( $block['innerBlocks'][ $key ]['attrs']['url'] ) && get_permalink() !== $block['innerBlocks'][ $key ]['attrs']['url'] ) {
			continue;
		}

		// If we got this far, we need to add the classname.
		// First make sure it is defined.
		if ( ! isset( $block['innerBlocks'][ $key ]['attrs']['className'] ) ) {
			$block['innerBlocks'][ $key ]['attrs']['className'] = '';
		}

		// Add the classname.
		$block['innerBlocks'][ $key ]['attrs']['className']  = trim( $block['innerBlocks'][ $key ]['attrs']['className'] . ' active-nav-item' );
	}

	return $block;
} );

add_filter( 'render_block', function( $content, $block ) {
	// Early exit if not a nav block.
	if ( 'core/navigation' !== $block['blockName'] ) {
		return $content;
	}

	// Check if we have an active item in this nav.
	$active_item_url = false;
	foreach ( $block['innerBlocks'] as $innerBlock ) {
		if ( isset( $innerBlock['attrs']['className'] ) && false !== strpos($innerBlock['attrs']['className'], 'active-nav-item' ) ) {
			$active_item_url = $innerBlock['attrs']['url'];
		}
	}

	// Early exit if this nav doesn't have an active item.
	if ( ! $active_item_url ) {
		return $content;
	}

	// Add the "active-nav-item" class to the active item.
	$parts = explode( 'wp-block-navigation-link"', $content );
	foreach ( $parts as $part_key => $part ) {
		$part = ( 0 === $part_key ) ? $part : '"' . $part;
		if ( false !== strpos( $part, 'href="' . $active_item_url . '"' ) ) {
			$part = ' active-nav-item' . $part;
		}
		$parts[ $part_key ] = $part;
	}

	return implode( 'wp-block-navigation-link', $parts );
}, 10, 2 );

The above code consists of 2 hooks:

  • render_block_data - Check the innerBlocks of the navigation and add a className to the active item.
  • render_block - Normally this shouldn't be necessary, but the problem with the nav block is that className for nav-items doesn't get printed (see Additional CSS Class(es) not working on Navigation Block #19858), so this is the "hacky" part that goes through the HTML and adds our class to the <li> item in the HTML.

@aristath
Copy link
Member

aristath commented Feb 1, 2020

Another solution via JS:

add_action( 'wp_footer', function() {
	?>
	<script>
	document.body.querySelectorAll( '.wp-block-navigation' ).forEach( function( navBlock ) {
		navBlock.querySelectorAll( '[href="' + window.location.href + '"]' ).forEach( function( navActiveLink ) {
			navActiveLink.parentNode.classList.add( 'active-nav-item' );
		});
	});
	</script>
	<?php
} );

This just adds a script in the footer of the site by hooking in wp_footer. We're then going through all nav-blocks, and we check if there's a link in there that goes to window.location.href. If we find such a link, we add a class to its parent node (the <li>).

Both this and the PHP solution posted above are imperfect, so only useful as workarounds until this gets fixed in the core block 👍

@Bowriverstudio
Copy link
Author

Just tested the PHP version and this works perfectly. I'll use this workaround until it is fixed in core. Thank you @aristath

Not 100% sure of the etiquette here - should I close this issue now that there is a workaround?

@jorgefilipecosta jorgefilipecosta added the [Block] Navigation Affects the Navigation Block label Feb 3, 2020
@jorgefilipecosta
Copy link
Member

Hi @Bowriverstudio thank you for reporting this issue. @aristath thank you for researching this and providing a workaround.

Hi @draganescu, @getdave, @retrofox, any thoughts on this? Is this something we should address before 5.4?

@draganescu
Copy link
Contributor

I can test this and see if it raises any issues. If it is straightforward we could release this as well.

@draganescu draganescu self-assigned this Feb 4, 2020
@aristath
Copy link
Member

aristath commented Feb 5, 2020

Highlighting the currently active menu item is a common practice that helps sighted users understand where they are in a site, and also a best practice for a11y.
Though not strictly speaking a requirement, not highlighting the currently active item breaks the mental pattern we all have when using a navigation on any site.
It would be great if this could be implemented ❤️

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Feb 6, 2020
@draganescu
Copy link
Contributor

Hi @aristath can you take #20076 for a spin and see if it solves the problem correctly?

@aristath
Copy link
Member

aristath commented Feb 6, 2020

Confirmed. Classes get added to menu items for posts/pages etc, and it looks like menu-item classes also get echoed. 👍

@timnicholson
Copy link

This has been rolled into core WordPress, but doesn't seem to work for the blog when the home page is set to a static page. i.e. If the blog itself is actually a page, the navigation menu doesn't get the current-menu-item class added to it.

@madfcat
Copy link

madfcat commented Jan 9, 2024

Confirmed. Classes get added to menu items for posts/pages etc, and it looks like menu-item classes also get echoed. 👍

_I use Wordpress 6.4.2 for building a custom theme. Inserting core navigation block does not have any indication of current class.

I have checked TwentyTwentyFour theme and current-menu-item is indeed added to li of the menu. It does not work when I am developing the theme from scratch?!_

UPD: Ok. It seems to me that I had to add my menu items to the existing pages. I did it before creating pages just writing the links, and probably, it was the issue. It would be nice if it worked this way to. I usually start building themes from header and footer and only after that add the pages.
But it still does not support submenus as I can not assign a page directly to it? Please, prove me wrong 😵‍💫

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 [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants