jQuery addClass

Question

I am a self confessed jQuery novice but I am conscious of learning the correct techniques. When I can see repetition like this I just know that there must be a better way.

jsFiddle

$(document).ready(function() {
 var project1 = $('section').eq(0).offset();
 var project2 = $('section').eq(1).offset();
 var project3 = $('section').eq(2).offset();
 var project4 = $('section').eq(3).offset();
 var project5 = $('section').eq(4).offset();
 var project6 = $('section').eq(5).offset();
 var $window = $(window);
 $window.scroll(function() {
 if ( $window.scrollTop() >= project1.top) {
 $("header").removeClass().addClass("project1");
 }
 if ( $window.scrollTop() >= project2.top ) {
 $("header").removeClass().addClass("project2");
 }
 if ( $window.scrollTop() >= project3.top ) {
 $("header").removeClass().addClass("project3");
 }
 if ( $window.scrollTop() >= project4.top ) {
 $("header").removeClass().addClass("project4");
 }
 if ( $window.scrollTop() >= project5.top ) {
 $("header").removeClass().addClass("project5");
 }
 if ( $window.scrollTop() >= project6.top ) {
 $("header").removeClass().addClass("project6");
 }
 }); 
});

• digita1-anal0g: Review the code/run the tests at jsperf.com/caching-jquery-selectors/6 for some refactoring ideas.

Answer 1

• Since section elements are being retrieved in a sequence, a for loop can be used.
• Since the scroll event can happen frequently, store the header selection in a variable outside of the event.
• $window.scrollTop() is called multiple times, making it a good candidate to be stored in a variable.

Condensed version of the code:

$(document).ready(function() {
 var $header = $("header");
 var numberOfSections = 6; 
 var sectionOffsets = [];
 for(var i = 0; i < numberOfSections + 1; i++) {
 sectionOffsets.push($('section').eq(i).offset()); 
 } 
 $(window).scroll(function() {
 var scrollTop = $(this).scrollTop(); 
 for(var i = 0; i < numberOfSections + 1; i++) {
 if(scrollTop > sectionOffsets[i].top) {
 $header.removeClass().addClass("project" + (i + 1)); 
 }
 }
 });
});

• 2ne: Is it necessary to name the number of sections? Could that be removed so that it could be any number?
• 2ne: I did it by using section.length jsfiddle.net/o44yy3mg/3
• 2ne: I am still gettin an error that says that it cannot read top of undefined
• digita1-anal0g: Hello, I edited the code so that the undefined error would no longer occur in the fiddle provided. See the updated version at jsfiddle.net/o44yy3mg/3. To summarize, you don't have to add 1 to the number of iterations the loop will run when selecting by the tag 'section'.

Answer 2

This should do the trick.

$(document).ready(function() {
 var $window = $(window), 
 $sections = $('section'), 
 $header = $('header');
 var elementMatchesViewport = function(data, index, element) { 
 return (data.scrollTop >= $(element).offset().top) ? true : false;
 };
 var setHeaderClassFromElementIndex = function(data, index, element) {
 var classString = 'project' + parseInt($(element).index() + 1, 10);
 data.$header
 .removeClass()
 .addClass(classString);
 };
 var handleScroll = function(data, event) {
 var scrollTop = $(this).scrollTop();
 data.$sections
 .filter($.proxy(elementMatchesViewport, this, {scrollTop: scrollTop}))
 .each($.proxy(setHeaderClassFromElementIndex, this, {$header: $header}));
 };
 $window.on('scroll', $.proxy(handleScroll, this, {$sections: $sections}));
});

Possible further enhancements:

• Use a zero-based index for your header classes.
• Debounce the scroll event / use request animation frame, as it's performance intensive.
• Refactor the condition of elementMatchesViewport() as it matches too many elements.

jsFillde