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

Highlight the active link in a navigation menu

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

Problem

I wonder if my script is all right. Please review my code:


Menu

ul {list-style-type:none; margin:0; padding:0;}
li {display:inline;}
a.active {background:red;}

Home
News
Contact
About

for (var i = 0; i 


It should highlight the current page link in the navigation bar.

Solution

querySelectorAll

Here's another approach, if you don't mind older browsers. You can use querySelectorAll instead of document.links. That way, you end up with a smaller result set rather than all links, and save you CPU cycles for running the loop.

In this case, we get only those links that have the same href value as the current document url:

var links = document.querySelectorAll('a[href="'+document.URL+'"]');


However, note the browser compatibility and there are quirks across implementations.

Not all links need active

Now if you think about it, not all links with the document.URL need to have active. Say you have active set the font to 2em. If you get all links that point to the same page, they'd be 2em in size, regardless of where they are in the page!

Most likely, you only need it for the primary navigation. Consider adding a class to further refine the result set.


    Home
    News
    Contact
    About

// Getting only the links that are in .navigation
var links = document.querySelectorAll('.navigation a[href="'+document.URL+'"]');

// More specific CSS
.navigation a.active {background:red;}


className

Now you have to note that setting className replaces it with the value, like this example. If the links you have happen to have existing classes (and styles that come with them), your script will unintentionally remove their styles due to the replaced className.

What you can do is get the existing className value, split them by spaces (multiple class names are separated by spaces), append your class name, join them back and then change the value, like I did here:

var element = document.getElementById('change');
var classNames = element.className.split(' ');
classNames.push('huge');
element.className = classNames.join(' ');

Code Snippets

var links = document.querySelectorAll('a[href="'+document.URL+'"]');
<ul class="navigation">
    <li><a href="http://www.example.com/home">Home</a></li>
    <li><a href="http://www.example.com/news">News</a></li>
    <li><a href="http://www.example.com/contact">Contact</a></li>
    <li><a href="http://www.example.com/about">About</a></li>
</ul>

// Getting only the links that are in .navigation
var links = document.querySelectorAll('.navigation a[href="'+document.URL+'"]');

// More specific CSS
.navigation a.active {background:red;}
var element = document.getElementById('change');
var classNames = element.className.split(' ');
classNames.push('huge');
element.className = classNames.join(' ');

Context

StackExchange Code Review Q#38413, answer score: 6

Revisions (0)

No revisions yet.