Scrolling elements into view

Question

On scroll, I am adding a class (current) to particular elements.

The code looks like it can be made better. For example, it seems like I am reusing the same things over and over again (i.e. the nth-child(1), nth-child(2), etc.).

I appreciate any and all comments and advice.

 function isScrolledIntoView(elem) {
 if (elem.length > 0) {
 var docViewTop = $(window).scrollTop();
 var docViewBottom = docViewTop + $(window).height();
 var elemTop = $(elem).offset().top;
 return ((elemTop <= docViewBottom) && (elemTop >= docViewTop));
 }
 }
 $(document).scroll(function () { // Measure scroll To Top
 // @TODO Fix it so that there is only 1 if block
 if (isScrolledIntoView($('.section .main-content:nth-child(1)')) === true) {
 $('.events-widget-menu li').removeClass('current');
 $('.events-widget-menu li:nth-child(1)').addClass('current');
 }
 if (isScrolledIntoView($('.section .main-content:nth-child(2)')) === true) {
 $('.events-widget-menu li').removeClass('current');
 $('.events-widget-menu li:nth-child(2)').addClass('current');
 }
 if (isScrolledIntoView($('.section .main-content:nth-child(3)')) === true) {
 $('.events-widget-menu li').removeClass('current');
 $('.events-widget-menu li:nth-child(3)').addClass('current');
 }
 if (isScrolledIntoView($('.section .main-content:nth-child(4)')) === true) {
 $('.events-widget-menu li').removeClass('current');
 $('.events-widget-menu li:nth-child(4)').addClass('current');
 }
 });

• Tushar: Try $('.section .main-content').filter(function() { return isScrolledIntoView($(this)); }).last().addClass('current'); in ready().

Answer 1

I don't understand why :nth-child selector is even in play here. It seems all you are wanting to do is add a class to an li with same index within its parent as what the content's index within it's parent is.

I might suggest something along these lines to achieve that:

// Cache jQuery collections somewhere in scope that `scroll()` inherits.
// This prevents you from having to continually re-query the DOM every time
// there is a scroll event.
var $mainContent = $('.section .main-content');
var $eventLi = $('.events-widget-menu li');
$(document).scroll(function () {
 $eventLi.removeClass('current');
 // find content elements in view
 var $currentElements = $mainContent.filter(function(index) {
 return isScrolledIntoView(this);
 });
 // apply class to LI with corresponding index
 $currentElements.each(function() {
 var index = $mainContent.index(this);
 $eventLi.eq(index).addClass('current');
 });
}

This solution is not hard-coded to X number of elements.

Answer 2

You probably want something like:

$(document).scroll(function () {
 for (i = 1; i <= 4; i++) {
 var curElem = $('.section .main-content:nth-child(' + i.toString() + ')');
 if (isScrolledIntoView(curElem) === true) {
 $('.events-widget-menu li').removeClass('current');
 curElem.addClass('current');
 }
 }
});

I didn't test it, but that should be it.

• Sᴀᴍ Onᴇᴌᴀ: Without a declaration for i using var or let a global variable would be created, which won't lead to an error but typically frowned upon