More efficient way to write this Javascript (waypoints)?

Question

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

});

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
– Qantas 94 Heavy, Commented Jun 9, 2013 at 1:21
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
– Jenn, Commented Jun 9, 2013 at 7:37

Ringo · Accepted Answer · 2013-06-09 20:59:41Z

0

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

edited Jun 9, 2013 at 20:59

answered Jun 9, 2013 at 1:38

Ringo

5,5333 gold badges35 silver badges52 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Jenn Over a year ago

Hi Ringo. Thanks so much for the reply. For some reason, this isn't working for me. But I get what you're suggesting, I think. I like the array idea, though I was hoping that I could accomplish this in a way that was even more automated (since its a Wordpress menu that may change with added pages, etc.) Appreciate your help though!

Ringo Over a year ago

If you post the html and js to a jsfiddle or pastebin, I can help figure out the issue. What is the error that you're seeing? You can definitely automate the code more. Instead of that first line, you could just do "var sections = $('section')"

Ringo Over a year ago

There was a syntax error above. Sorry about that. The $.each block was missing the closing parens after the curly brace. I've edited the answer above. Let me know if that works better for you. I'll also edit the "var sections" so it'll be more automated.

Jenn Over a year ago

Wow...cool! I just need to offset in a different (+1) direction for the downward transition, and it works. Way to go, Ringo! You rock! And I even think I understand it. I'll adjust to a class name rather than the id, too. That's an excellent point about possible additional sections. JS Fiddle in the question above if you want to see it in action. I added your code already. Thank you!

Jenn Over a year ago

By the way, this code will be included in my personal work portfolio, so I'm happy to credit you in the code and link to your profile (unless you'd rather I not).

|

Collectives™ on Stack Overflow

More efficient way to write this Javascript (waypoints)?

1 Answer 1

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related