HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Rendering HTML for a navigation menu using a class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
classnavigationmenuforusingrenderinghtml

Problem

Below I have implemented a class that will generate HTML to build the structure for a navigation menu.

```
define('BASE_URL', 'http://localhost/site');

class Navigation
{
private $doc;
private $xpath;
private $rootNode;

/**
* Initialize navigation.
*/
public function __construct($menu)
{
$this->doc = new DOMDocument();
$this->doc->appendChild($this->buildMenu($menu));
$this->xpath = new DOMXpath($this->doc);
}

/**
* Un-mark selected menu item.
*/
public function deselect()
{
$liNodeList = $this->xpath->query("//li[contains(@class, 'selected')]");
if ($liNodeList->length) {
$liNode = $liNodeList->item(0);
$classAttr = explode(' ', $liNode->getAttribute('class'));
$key = array_search('selected', $classAttr);
unset($classAttr[$key]);
$liNode->setAttribute('class', implode(' ', $classAttr));
}
}

/**
* Mark first occurrence of menu item containing link with URI as selected.
* @param $uri string
*/
public function select($uri)
{
$liNodeList = $this->xpath->query("//li[a[contains(@href, '" . $uri . "')]]");
if ($liNodeList->length) {
$liNode = $liNodeList->item(0);
$classAttr = explode(' ', $liNode->getAttribute('class'));
$classAttr[] = 'selected';
$liNode->setAttribute('class', trim(implode(' ', $classAttr)));
}
}

/**
* Build menu.
* @param $menu array
* @param $depth int
*/
private function buildMenu($menu, $depth = 0)
{
$divNode = $this->createNav();
$ulNode = $this->doc->createElement('ul');
// set first nav as root
if (!$depth) {
$divNode->setAttribute('id', 'primary-nav');
$this->rootNode = $divNode;
}
// add menu items
foreach ($menu as $index => $menuItem) {

Solution

This is very good code.
Re-usability

BASE_URL and $_SERVER['REQUEST_URI'] should be passed into the constructor as parameters.

I would suggest that this class should focus on building the ` and
  • part of the navigation and leave any container wrapping to the caller. HTML5 provides the nav element. This may be appropriate for the caller to use. The method createNav is misleading it should either create a or it should be renamed to createContainer.



Sometimes just the top level or two of a menu is desired. Having the depth to traverse the menu and build as a parameter could be good.
Style

You have the current depth, but you don't add it to the output. The CSS for styling might be easier if you add the current depth to the

  • items?


Variables and Properties

One variable name was a little confusing.
$depth has a few different meanings. It could mean the current depth or be a limitation on how deep to traverse down the menu.

$this->rootNode` can probably be removed?

The following could be made protected in case you want to extend this class?

private $doc;
private $xpath;

Code Snippets

private $doc;
private $xpath;

Context

StackExchange Code Review Q#117539, answer score: 2

Revisions (0)

No revisions yet.