2
votes

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}); 

});
1
Hi, welcome to Stack Overflow. You might find that since this is simply about improving a working piece of code, that Code Review is a more appropriate location for your question. Having said that though, I like the way that you've set out your post - keep it up! - Qantas 94 Heavy
Oops! Sorry about that. Don't think I ever realized that option existed. I'll shift the question over there. Thank you for the encouragement! - Jenn

1 Answers

0
votes
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');
}