Search code examples
navigationhighlightjquery-waypoints

More efficient way to write this Javascript (waypoints)?


Pardon the JS noob question, but (while my code is working as expected) I'm sure there must be a better/more efficient way to write it. Suggestions are greatly appreciated.

Here's what's happening: I have a Wordpress menu on a one-page vertical scrolling theme (custom). I'm using waypoints.js to highlight the corresponding nav button for the current visible section as the page scrolls up, down,or when a navigation item is clicked.

I've set the home navigation item to a class of "active" on page load, and am highlighting each section manually by adding/removing the "active" class at each waypoint. For the sake of automating this a bit, I tried using $this rather than the div id, but haven't been able to get it right yet.

Thanks in advance for any help. Here's the code in question:

http://jsfiddle.net/vCP4K/

My current clumsy solution:

// Make home button active on page load

$('li.home-btn a').addClass('active');

// Change classes as divs hit the waypoint on the way DOWN or on click

$('section#home').waypoint(function(down) {
$('.nav li a').removeClass('active');
$('li.home-btn a').addClass('active');
}, {offset: -1}); 

$('section#services').waypoint(function(down) {
$('.nav li a').removeClass('active');
$('li.services-btn a').addClass('active');
}, {offset: 1}); 

$('section#work').waypoint(function(down) {
$('.nav li a').removeClass('active');
$('li.work-btn a').addClass('active');
}, {offset: 1}); 

$('section#about').waypoint(function(down) {
$('.nav li a').removeClass('active');
$('li.about-btn a').addClass('active');
}, {offset: 1}); 

$('section#blog').waypoint(function(down) {
$('.nav li a').removeClass('active');
$('li.blog-btn a').addClass('active');
}, {offset: 1}); 

$('section#contact').waypoint(function(down) {
$('.nav li a').removeClass('active');
$('li.contact-btn a').addClass('active');
}, {offset: 1}); 

// Change classes as divs hit the waypoint on the way UP or on click

$('section#home').waypoint(function(up) {
$('.nav li a').removeClass('active');
$('li.home-btn a').addClass('active');
}, {offset: -1}); 

$('section#services').waypoint(function(up) {
$('.nav li a').removeClass('active');
$('li.services-btn a').addClass('active');
}, {offset: -1}); 

$('section#work').waypoint(function(up) {
$('.nav li a').removeClass('active');
$('li.work-btn a').addClass('active');
}, {offset: -1}); 

$('section#about').waypoint(function(up) {
$('.nav li a').removeClass('active');
$('li.about-btn a').addClass('active');
}, {offset: -1}); 

$('section#blog').waypoint(function(up) {
$('.nav li a').removeClass('active');
$('li.blog-btn a').addClass('active');
}, {offset: -1}); 

$('section#contact').waypoint(function(up) {
$('.nav li a').removeClass('active');
$('li.contact-btn a').addClass('active');
}, {offset: -1}); 

});

Solution

  • var sections = [];
    
    // It'd be better if you used a classname for the section.
    // Then you can select by classname rather than element name.
    // E.g., if someone were to add a <section> tag elsewhere in the document,
    // they would experience a bad bug.
    
    $('section').each(function() {
      sections.push($(this).attr('id'));
    });
    $.each(sections, function(index) {
      var sectionDiv = $('#' + sections[index]);
      sectionDiv.waypoint(function(down) {
        activateSection(sections[index]);
      }, {offset: -1});
      sectionDiv.waypoint(function(up) {
        activateSection(sections[index]);
      }, {offset: -1});
    })
    function activateSection(sectionName) {  
        $('.nav li a').removeClass('active');
        $('li.' + sectionName + '-btn a').addClass('active');
    }