CODEWITHSUNDEEP
jquery
Zera42
Zera42

Reputation: 2692

jquery scroll, change navigation active class as the page is scrolling messed up, and making wrong links active?

http://jsfiddle.net/5Cfm6/1/

So whenever I click on watch, it goes to the div with the video, but if I click any other link on the nav bar, it sometimes highlights the active link, and it's always the wrong link. For example, if I click on about, it will highlight links. Could someone help me to get it to highlight the correct div that I'm currently at?

This is what I have so far. 

$(window).scroll(function () {
    var windscroll = $(window).scrollTop();
    if (windscroll >= 100) {
        $('.container div').each(function (i) {
            if ($(this).position().top <= windscroll - 20) {
                $('nav a.active').removeClass('active');
                $('nav a').eq(i).addClass('active');
            }
        });
    } else {
        $('nav a.active').removeClass('active');
        $('nav a:first').addClass('active');
    }`enter code here`

}).scroll();

Upvotes: 2

Views: 3530

Answers (2)

gkalpak
gkalpak

Reputation: 48212

I would like to point out that (at least in your fiddle) the "sponsors" div is outside of div.container, so it will probably not work for sponsors (e.g. if you add more sections after "sponsors") and even if it accidentaly does now, you might run into other unexpected problems in the future.

So, what you have to do for your code to (not accidentaly) work correctly is:

  1. Place the "sponsors" div inside the div.container.
  2. Change:
    $('.container div').each(function (i) { to
    $('.container > div').each(function (i) {
  3. Change:
    if ($(this).position().top <= windscroll + 20) { to
    if ($(this).position().top <= windscroll + 34) {,
    since this is the scrollTop offset you set in line 4 (scrollPoint = $('...').offset().top - 34).

Working demo can be found here.

EDIT: One last case you might want to address is when the last section's height is smaller than the current windows height, so that it is impossible for scrollTop to ever reach the last section's position().top. In that case, it still makes sense to hightlight the last element in nav-bar, when the user has scrolled all the way to the bottom. This can be achieved by means of this small addition:

$(window).scroll(function () {
    ...
    var fromBottom = $(document).height() 
                     - ($(window).scrollTop() + $(window).height());
    if (fromBottom == 0) {     // <-- scrolled to the bottom
        $('nav a.active').removeClass('active');
        $('nav a:last').addClass('active');
    } else if ...

(The working demo has been updated to illustrate that too.)

(NOTE: Not relevant to the current question, but you might want to replace the </br>s in your code with <br/> :))

Upvotes: 2

Eli
Eli

Reputation: 14827

Try to target direct child of .container div instead of every descendants as well as you need to increase the value of i by 1 since it's 0-based:

$('.container > div').each(function (i) {
    var index = i + 1;
    if ($(this).position().top <= windscroll - 20) {
        $('nav a.active').removeClass('active');
        $('nav a').eq(index).addClass('active');
    }
});

Updated Fiddle

Upvotes: 0

Related Questions