Author: williampatton

Covering days 16 through 20 and culminating in 1 month and 1/5th of the way through 100 days.

This week I had spent a substantial portion of my time working on a project I've supported for last 5+ years – the WP Bootstrap Navwalker.

I've used the walker since version 2 of Bootstrap and been a contributer all of BS3 lifespan. With Bootstrap 4 finally in a stable release (after around 3 years in development!) and I have been working through a substantial rework of the entire navwalker.

I've focused on a few things through the rebuild.

• Simplifying the codebase and improving readability.
• Better link modifier (disabled, header, divider) and icon handling.
  
• Handling long standing issues that were difficult to fix or required a large rework in the previous iteration.
  

Simplifying The Codebase And Improving Readability

To simplify things I have focused on a few things.

Reducing the number of possible paths in the start_lvl function has been the first focus. This function has grown to contain lots of conditionals that result in different processing or outputs. At one point in Scrutinizer I seen it say that this function had over 20000 paths!

Lots of paths result in code that becomes hard to follow so to help improve that I decided to externalize some parts of the code into their own functions.

Prime candidates were some decision logic that didn't need to be cluttering up the main thread. A single call with a well named function does a better job of explaining what is happening than the large conditional code blocks – even through those codeblocks are well commented.

Better Link Modifier and Icon Handling

Bootstrap navigation has always required the use of specific classes on the container, the link and the wrapper elements for the dropdown elements to function. WordPress provides a way to set classes for individual nav items in the Menu Editor UI.

In previous versions the Title Attribute input was used to handle special classes for modifiers and icons.

The complexity here is that most of the time Walkers add the classes to the wrapper element – in this walker that is an <li> – however some of the time you need those classes to get applied directly to the link.

Icon classes need to be placed separately from the wrapper and the link. They are added to their own <i> element with aria-hidden="true" so that screen readers don't try read an icon character.

$icon_classes = array(); // This array would contain valid icon classes.

// Join any icon classes plucked from $classes into a string.
$icon_class_string = join( ' ', $icon_classes );

/**
 * Initiate empty icon var, then if we have a string containing any
 * icon classes form the icon markup with an <i> element. This is
 * output inside of the item before the $title (the link text).
 */
$icon_html = '';
if ( ! empty( $icon_class_string ) ) {
	// append an <i> with the icon classes to what is output before links.
	$icon_html = '<i class="' . esc_attr( $icon_class_string ) . '" aria-hidden="true"></i> ';
}

To make this happen I needed to loop through arrays of classnames and strip specific classes from the main array and save them in a new array for later use. The $icon_classes array is filled with that function.

I decided to handle link modifiers and icon classes separately as the link modifiers are used for setting a typeflag for decision making later in the execution.

/**
 * Find any custom linkmod or icon classes and store in their holder
 * arrays then remove them from the main classes array.
 *
 * Supported linkmods: .disabled, .dropdown-header, .dropdown-divider
 * Supported iconsets: Font Awesome 4/5, Glypicons
 *
 * NOTE: This accepts the linkmod and icon arrays by reference.
 *
 * @since 4.0.0
 *
 * @param array   $classes      an array of classes currently assigned to the item.
 * @param array   $link_classes an array to hold linkmod classes. Passed by reference.
 * @param array   $icon_classes an array to hold icon classes. Passed by reference.
 * @param integer $depth        an integer holding current depth level.
 *
 * @return array  $classes      a maybe modified array of classnames.
 */
function seporate_linkmods_and_icons_from_classes( $classes, &$linkmod_classes, &$icon_classes, $depth ) {
	// Loop through $classes array to find linkmod or icon classes.
	foreach ( $classes as $key => $class ) {
		// If any special classes are found, store the class in it's
		// holder array and and unset the item from $classes.
		if ( preg_match( '/disabled/', $class ) ) {
			// Test for .disabled.
			$linkmod_classes[] = $class;
			unset( $classes[ $key ] );
		} elseif ( preg_match( '/dropdown-header|dropdown-divider/', $class ) && $depth > 0 ) {
			// Test for .dropdown-header or .dropdown-divider and a
			// depth greater than 0 - IE inside a dropdown.
			$linkmod_classes[] = $class;
			unset( $classes[ $key ] );
		} elseif ( preg_match( '/fa-(\S*)?|fas(\s?)|fa(\s?)/', $class ) ) {
			// Font Awesome.
			$icon_classes[] = $class;
			unset( $classes[ $key ] );
		} elseif ( preg_match( '/glyphicons-(\S*)?|glyphicons(\s?)/', $class ) ) {
			// Glyphicons.
			$icon_classes[] = $class;
			unset( $classes[ $key ] );
		}
	}

	return $classes;
}

Unit Testing My Code

While I was going through the code and dealing with long standing issues I also took care of an issue which was opened not long ago on the early dev version of the v4 walker. The issue explained that the fallback method was broken and outputting nothing when it should have been outputting something.

The fix was easy – I had mistakenly used assign instead of append so swapping the operator on all of the markup to be output made it work.

// incorrect use:
$output = 'somehtmlcontent';
// fixed:
$output .= 'somehtmlcontent';

Once I'd fixed it I realised that the project has Unit Tests that can be run locally and are run by Travis but all they do is check that the file, class and methods exist. Those tests could be a lot more useful.

If there was a test in place that made sure the fallback output when it was supposed to and didn't when it wasn't I would have caught this mistake early and fixed it quick. Instead it escaped me till someone else pointed it out.

Testing That Functions Produce The Output They Should

The project has PHPUnit already available so all I had to do was add some tests that could check the output.

This fallback function should produce 2 different types of output. For a logged out user it should output nothing. If a user is logged in with edit_theme_options capability it should output markup with a link. There's also a flag that determines if it echos or returns.

I wrote some tests that do a few things:

• Test that logged out users get empty strings and that both 'echo' and 'return' are the same.
• Test that logged in users get html that looks to be valid and that 'echo' and 'return' are both the same.

When testing output of a function there are a few methods that PHPUnit has available. Those methods are useful but somewhat limited so many times – for output that may be complicated or subject to slight changes over time – you will want to check for traits of the valid output rather than the exact output to save you reworking tests all the time. PHPUnit already runs tests inside of a content buffer so all you need to do is get your output with ob_get_contents;.

One thing that got me stuck for a few minutes was how exactly could I get logged in output when this is run in CLI mode where no user is set. Turns out it's quite easy. Make a new user with the capability we need – I made an admin that does have all core capabilities – and set them to the current user.

// make an admin user and set it to be the current user.
$user_id = $this->factory->user->create( array( 'role' => 'administrator' ) );
wp_set_current_user( $user_id );

The full pair of test ended up coming out like this:

/**
 * Test Fallback method output for logged out users.
 *
 * Expects that for logged out users both echo and return requests should
 * produce empty strings.
 */
function test_fallback_function_output_loggedout() {

	// default is to echo reults, buffer.
	ob_start();
	WP_Bootstrap_Navwalker::fallback( $this->valid_sample_fallback_args );
	$fallback_output_echo = ob_get_clean();

	// empty string expected when not logged in.
	$this->assertEmpty(
		$fallback_output_echo,
		'Fallback output for logged out user created a non-empty string in echo mode.'
	);

	// set 'echo' to false and request the markup returned.
	$fallback_output_return = WP_Bootstrap_Navwalker::fallback( array_merge( $this->valid_sample_fallback_args, array(
		'echo' => false,
	) ) );

	// return and echo should result in the same values (both empty).
	$this->assertEquals(
		$fallback_output_echo,
		$fallback_output_return,
		'Fallback output for logged out user created a non-empty string in return mode.'
	);
}

/**
 * Test Fallback method output for logged in users.
 *
 * Expects strings to be produced with html markup and that they match when
 * requesting either a return or defaulting to echo.
 */
function test_fallback_function_output_loggedin() {

	// make an admin user and set it to be the current user.
	$user_id = $this->factory->user->create( array( 'role' => 'administrator' ) );
	wp_set_current_user( $user_id );

	// default is to echo results, buffer.
	ob_start();
	WP_Bootstrap_Navwalker::fallback( $this->valid_sample_fallback_args );
	$fallback_output_echo = ob_get_clean();

	// rudimentary content test - confirm it opens a div with 2 expected
	// values and ends by closing a div.
	$match = ( preg_match('/^(<div id="a_container_id" class="a_container_class">)(.*?)(<\/div>)$/', $fallback_output_echo ) ) ? true : false;
	$this->assertTrue(
		$match,
		'Fallback method seems to create unexpected html for logged in users in echo mode.'
	);

	// set 'echo' to false and request the markup returned.
	$fallback_output_return = WP_Bootstrap_Navwalker::fallback( array_merge( $this->valid_sample_fallback_args, array(
		'echo' => false,
	) ) );

	// return and echo should both produce the same strings.
	$this->assertEquals(
		$fallback_output_echo,
		$fallback_output_return,
		'Fallback method seems to create unexpected html for logged in users in return mode.'
	);
}

Testing Private Methods of a Class

When I was writing other tests I needed to call private methods and test their output. Private methods are not intended to be called from outside of the function so to make that happen we need to use a technique called Reflection.

We use a reflector to duplicate the class and methods we want and then set the methods to be publicly accessible from the reflector.

$wp_bootstrap_navwalker = $this->walker;

// since we're working with private methods we need to use a reflector.
$reflector = new ReflectionClass( 'WP_Bootstrap_Navwalker' );

// get a reflected method for the opener function and set to public.
$method = $reflector->getMethod( 'linkmod_element_open' );
$method->setAccessible( true );

Once you have the reflected method setup and accessible you can then use invokeArgs on the $method – passing the real class object followed by an array containing the args to pass to it.

// test openers for headers and dividers.
$header = $method->invokeArgs( $wp_bootstrap_navwalker, array( $this->valid_linkmod_typeflags[0], 'stringOfAttributes' ) );
$this->assertNotEmpty( $header, 'Got empty string for opener of ' . $this->valid_linkmod_typeflags[0] );