На мой взгляд, задумка в целом правильная, но реализация нерациональна. Кроме очевидной ошибки синтаксиса $("#home, #services", #price)
и обращения ко всем ссылкам на странице $("a")
есть некоторые претензии. $(window).scroll
вызывается мильён раз за время прокрутки и при каждом вызове вы перебираете свои дивки, получаете их position().top
, ищете на странице все 'a' (ну или все 'а' с определенным href, не важно), при том, что меняется только прокрутка страницы, а эти переменные постоянны.
Почему бы не собрать все это один раз, а в колбеке скролла только проверять, причем проверять с конца.
В процессе понял, что проще написать все с начала) Вот что получилось:
Пример на jsfiddle: https://jsfiddle.net/ipshenicyn/owhLpuv9/
Дальше ключевой код из примера, очищенный от лишнего
HTML
Ссылкам добавил класс .scroll-to
, чтобы удобнее было с ними работать. "Якорным" блокам добавил класс .anchor
, чтобы сделать гибко и не лазить в js-код каждый раз, когда на странице появляется новый блок.
<a href="#div1" class="scroll-to">div1</a>
<a href="#div2" class="scroll-to">div2</a>
<a href="#div3" class="scroll-to">div3</a>
<a href="#div4" class="scroll-to">div4</a>
<div id="div1" class="anchor"></div>
<div id="div2" class="anchor"></div>
<div id="div3" class="anchor"></div>
<div id="div4" class="anchor"></div>
JS
var positions = [], //сюда сложим на загрузке страницы позиции наших "якорных" блоков, чтобы не считать их каждый раз. и сюда же положим ссылки на соответствующие a.scroll-to
currentActive = null, //здесь будет храниться id текущего блока, чтобы не менять классы по 100 раз за одну прокрутку
links = $('.scroll-to'); //сохраним массив всех a.scroll-to
$(".anchor").each(function(){ //перебираем блоки, сохраняем позиции и ссылки на пункты меню
positions.push({
top: $(this).position().top - 100,
a: links.filter('[href="#'+$(this).attr('id')+'"]')
});
});
//делаем реверс массива, чтобы при скролле перебирать его с конца и выходить из цикла при нахождении
//зачем нам проверять каждый блок, если прокрутка уже ниже последнего, верно?
positions = positions.reverse();
$(window).on('scroll',function() {
var winTop = $(window).scrollTop();
for(var i = 0; i < positions.length; i++){
if(positions[i].top < winTop){ //если прокрутка страницы ниже нашего блока
if(currentActive !== i){ //и если мы еще не добавили класс текущему блоку
currentActive = i;
links.filter('.active').removeClass('active'); //снимаем класс .active с текущего пункта меню
positions[i].a.addClass("active");
}
break; //выходим из цикла, не нужно проверять блоки, которые выше
}
}
});
$("#home, #services", #price)
->$("#home, #services, #price")
. 2.$("a")
- выбирает все анкор таги на странице. 3. посмотрите описание/параметры функции$.each
- api.jquery.com/each.