Highlighting current page item in the nav menu with jQuery

Question

I've read all about this online but everything else was just overly complicated and I just want to implement a simple code that I can read without going into substrings and all that stuff, so i thought this might be a good challenge for as a noob in jquery. I've come up with this code but without success.

HTML

<ul id="nav">
    <li><a href="/" id="home" class="lettering">HOME</a></li>
    <li><a href="/about" id="about" class="lettering">ABOUT</a></li>
    <li><a href="/works" id="works" class="lettering">WORKS</a></li>
    <li><a href="/contact" id="contact" class="lettering">CONTACT</a></li>
</ul>

jQuery

$(document).ready ( function(){
    var path = window.location.pathname;
    $('#nav li a').each(function() {
        if( path = ("/"+this.id+"/") ){
            this.addClass('active');
        } else {
            ('#home').addClass('active');
        }
    })
})

I hope you guys don't flame me, my intentions are purely academic rather than getting results. What is the problem here? I'm not getting any error logs or anything btw.

edit: removed the trailing slashes(thanks Justin), also tried what Phrogz suggested but no luck. If anyone feels up to the challenge, the site is located @ egegorgulu.com

In your js you have a trailing slash appended to the path but not in the href. So unless your server adds a trailing slash these would never match — Justin Lucas, Jan 19 '12 at 00:13
Simpler jQuery: `$('#nav a[href="'+location.pathname+'"]').addClass('active');` — Phrogz, Jan 19 '12 at 00:18
The only way you're going to get flamed on this board is if you pop in with a question and ask everyone to figure it out for you and then supply you with the code. And the flaming is usually nothing more then having your question downvoted to the point where everyone just ignores it. Really, this is a very mature board. — Jagd, Jan 19 '12 at 00:31
OK, so I tried both of what you guys said but no luck. It's just not working, I'm probably doing something wrong but what? anyways, if anyone feels up to the challenge the site is located at egegorgulu.com. — Ege, Jan 19 '12 at 00:36

score 3 · Answer 1 · answered Jan 19 '12 at 00:15

3

In your js you have a trailing slash appended to the path but no trailing slash in the href of your tags. So unless your server adds a trailing slash these paths would never match.

answered Jan 19 '12 at 00:15

Justin Lucas

2,301
14
22

you mean the one after the this.id right? removed it but no change :/ – Ege Jan 19 '12 at 00:19
Both of them actually since path would already include the first slash. I don't see anything else wrong. Phrogz has an alternative snippet that should also work. – Justin Lucas Jan 19 '12 at 00:22

GregL · Answer 2 · 2012-01-19T01:58:08.037

There is a few mistakes in your Javascript that I can spot.

Primarily because you need to know that within .each(), this refers to the DOM element object, so to call any jQuery methods on it, you need to pass this to the jQuery function: $(this).

So try the following instead:

$(document).ready ( function(){
    var path = window.location.pathname;
    $('#nav li a').each(function() {
        if( path === "/"+this.id ){ // EDIT: This was the problem with the if
            $(this).addClass('active'); // "this" is a DOM element, not a jQuery object
        } else {
            $('#home').addClass('active'); // you missed the $
        }
    }); // always get in the habit of being explicit with your semicolons
});

EDIT: The problem with your if was that you used the assignment operator = instead of the comparison operator == (equality with type conversion) / === (equality with same types). I have edited the code above to address that.

EDIT 2: Okay, let's try a new approach that leverages jQuery's filter() function to make use of the href attribute on the <a> elements to find a match:

$(document).ready ( function(){
    var path = window.location.pathname;
    var $navLinks = $('#nav li a');
    $navLinks.removeClass('active'); // start with a blank slate
    $navLinks.filter(function() {
        return path.indexOf(this.href) === 0; // test if the pathname starts with the current link's href attribute
    }).addClass('active'); // find a matching link to add the class to where the href attribute starts with the pathname
    if ($navLinks.filter('.active').length === 0) { // if nothing matches
        $('#home').addClass('active'); // make the home link active as a default
    }
});

As an academic aside, note that you can make use of jQuery's powerful chaining syntax and some JS knowledge to compress this even more if you like, so that it could become as small as:

$(document).ready ( function(){
    if (!$('#nav li a').removeClass('active').filter(function() {
            return window.location.pathname.indexOf(this.href) === 0;
        }).addClass('active').end().filter('.active').length) {
        $('#home').addClass('active'); // make the home link active as a default
    }
});

How's that for a short but hard-to-understand bit of code (which is also untested, btw)?

ok this seems to be the right way but there is something wrong with the "if" I guess cause its adding the class to every item under nav. Any ideas? — Ege, Jan 19 '12 at 01:07
yeah, thanks a lot =] this is definately the way to go, but it only works for the home page right now, which suggests that the if is not working. I know this is a minor thing but I was unable to add a console log for the this.id - console.log(path) returns /about/, /works/, etc. if I could only new what this.id is returning that would do the trick I guess. — Ege, Jan 19 '12 at 01:29
@Ege I came up with a new approach that leverages jQuery a bit more. Give it a try and see if it works. — GregL, Jan 19 '12 at 01:49

score 0 · Accepted Answer · answered Jan 24 '12 at 12:49

If anybody is wandering, I came up with the following function for this. Many thanks to GregL for his pointers, it helped me a lot.

jQuery.noConflict()( function(){
    var $ = jQuery;
    var path = window.location.pathname;
    $('#nav li a').each(function() {
        if( path === "/" + this.id + "/" ){ 
            $(this).addClass('active'); 
        } else if( path === "/" ) {
            $('#home').addClass('active'); 
        }
    }); 
});

The reason for the noConflict is because I'm using an embedded contact form on my contact page. And I'm pretty sure that the second if is unnecessary but didn't want to remove it once I got it working properly.

Highlighting current page item in the nav menu with jQuery

3 Answers3

Linked