3
\$\begingroup\$

Using Bootstrap 3 and PHP, I create my navigation bars in the following way:

<?php
$page_url = $_SERVER['REQUEST_URI'];
$active1 = (strpos($page_url, "one.php") !== false ? " class=\"active\"" : "");
$active2 = (strpos($page_url, "two.php") !== false ? " class=\"active\"" : "");
$active3 = (strpos($page_url, "three.php") !== false ? " class=\"active\"" : "");
$active4 = (strpos($page_url, "four.php") !== false ? " class=\"active\"" : "");
?>
<div class="navbar navbar-default">
  <div class="container">
    <div class="navbar-header">
      <button class="navbar-toggle" data-toggle="collapse" data-target=".navbar-collapse">
        <i class="icon-bar"></i>
        <i class="icon-bar"></i>
        <i class="icon-bar"></i>
      </button>
      <a class="navbar-brand" href="index.php">Company Name</a>
    </div>
    <div class="collapse navbar-collapse">
      <ul class="nav navbar-nav">
        <li<?=$active1?>><a href="one.php">Link One</a></li>
        <li<?=$active2?>><a href="two.php">Link Two</a></li>
        <li<?=$active3?>><a href="three.php">Link Three</a></li>
        <li<?=$active4?>><a href="four.php">Link Four</a></li>
      </ul>
    </div>
  </div>
</div>

Then, depending on which page you're on, a link within the navigation has class="active" applied to it. However, this feels like a very messy way of building this functionality to the <navbar>. Lots of empty variables at one time, and a lot of almost identical code for each navigation element.

How best can I improve on this code?

\$\endgroup\$
  • \$\begingroup\$ Using templating engines can help in this process. But using a function as suggested in the answer is a good first step \$\endgroup\$ – Pinoniq May 7 '15 at 17:21
3
\$\begingroup\$

Use an array and a function

You've got quite a few pieces of code that look very similar. The more menu items you have, the worse this gets. This is almost always a sign you should start using a function. Here's how to do that:

<?php

  $menuItems = array('Link One'   => 'one.php',
                     'Link Two'   => 'two.php',
                     'Link Three' => 'three.php',
                     'Link Four'  => 'four.php');

  function navigationItem($title,$page)
  {
    $class = (strpos($_SERVER['REQUEST_URI'],$page) !== false) ? ' class="active"' : '';
    return '        <li'.$class.'><a href="'.$page.'">'.$title.'</a></li>'.PHP_EOL;
  }

?>
<div class="navbar navbar-default">
  <div class="container">
    <div class="navbar-header">
      <button class="navbar-toggle" data-toggle="collapse" data-target=".navbar-collapse">
        <i class="icon-bar"></i>
        <i class="icon-bar"></i>
        <i class="icon-bar"></i>
      </button>
      <a class="navbar-brand" href="index.php">Company Name</a>
    </div>
    <div class="collapse navbar-collapse">
      <ul class="nav navbar-nav">
<?php

  foreach ($menuItems as $title => $page) echo navigationItem($title,$page);

?>      </ul>
    </div>
  </div>
</div>

As you can see I've put both lines, which you repeat four times, inside the function and simply call that function four time. I could have done that by simply writing:

<?php
  echo navigationItem('Link One','one.php');
  echo navigationItem('Link Two','two.php');
  echo navigationItem('Link Three','three.php');
  echo navigationItem('Link Four','four.php');
?> 

but I prefer to have the menu at the top of the file in an array. This makes it easier to find and edit.

This was probably the next step you were looking for. But it should not be your last.

You could easily generalize the navigationItem() function for all list items you will make. And the foreach loop basically builds a list, why not make a function out of that?

After that you could make a class which could build a whole menu bar for you. The functions will become methods of that class.

Why not put the array in a seperate file with the other stuff that defines your site? Perhaps eventually create a database table with menu items?

There are many ways in which you can improve this tiny piece of code.

What you should definately NOT do is: Build a whole site with Bootstrap HTML code interspersed by piece of loose PHP code. That will be chaos, and very difficult to maintain and expand. Be systematic about it.

Preferably try to avoid putting piece of PHP code inside HTML pages. It's just so ugly. Personally I prefer to write in pure PHP, and generate the HTML using objects similar to the class I suggested for your menu bar.

|improve this answer|||||
\$\endgroup\$
1
\$\begingroup\$

I have encountered this issue multiple times and have found what I consider a simple solution.

There are two prerequisites to my solutions. I always attempt to separate my HTML (view markup in general) and PHP. And since a link (or the affected tag) can have more than the iconic active class, the solution must make it easy to add/remove classes without affecting it.

My solutions consists of a simple function:

/**
 * Checks if the specified $target page is the current page.
 * 
 * @param string       $target A string declaring the page to match against.
 * @param string|array $class  A string declaring one or more classes you use if the current page matches.
 *
 * @return string|null
 */
function isActive($target, $class) {
    if(strpos($_SERVER['REQUEST_URI'], $target)) { // This line can be changed to better suit your application.

        if(is_array($class)) {
            return implode(' ', $class);
        }
        return (string) $class;

    }
    return null;
}

With this I can specify one or more classes that should be included if the link is the current page. The usage is as follows:

<a class="<?= isActive('awesomeness.php' 'active'); ?> another-unrelated-class" href="awesomeness.php"> This is a link to awesomeness!</a>

If you need more than one classes to be included you pass an array as second argument instead. Now I can also add/remove css classes. I can also change the markup or more importantly reuse the function elsewhere as no markup is generated inside it.

Regarding links: I also think this solution suits HTML markup better as the actual href link and the link-text are not considered. You can then write markup which closer resembles standard HTML, but still keep the behavior.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ The line isActive('awesomeness.php' 'active') requires a comma in there, I think, or at least copying it verbatim seemed to produce an error that was overcome with the comma. \$\endgroup\$ – Mike Cordingley Nov 27 '15 at 18:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.