Что в скрипте не так?

Question

[Сергей Миллер]

Скрипт работает, но анимация лагает жестко, особенно на слабых ноутах. Знакомый подсказал что each каждый раз использовать не нужно, не понимаю тогда как должен выглядеть в итоге код. Делаю так: анимированный элемент сначала в css имеет свойство display:none , про достижении скроллинга до нужного места, в jquery делаю fadeIn элементу и добавляю класс, в котором прописана сама анимация в css. Подскажите что в скрипте не правильно и как делается это правильно, пожалуйста.

$(document).ready(function() {
    var windowHeight = $(window).height();
    $(document).on('scroll', function() {
        $('#firstScroll').each(function() {
            var self = $(this),
                height = self.offset().top + self.height() + 50;
            if ($(document).scrollTop() + windowHeight >= height) {
                $('#firstGrid').addClass('fadeInDown').fadeIn(700);
            }
        });
        //цены в слайдере
        $('.strike-price').each(function() {
            var self = $(this),
                height = self.offset().top + self.height() + 100;
            if ($(document).scrollTop() + windowHeight >= height) {
                $('.for-price big').addClass('fadeInDownPrice').fadeIn(700);
            }
        });
        //слайдер отзывов
        $('#box6').each(function() {
            var self = $(this),
                height = self.offset().top + self.height();
            if ($(document).scrollTop() + windowHeight >= height) {
                $('.contacts').addClass('fadeInTop').fadeIn(700);
            }
        });
    });
});

Answer 1

[Stalker_RED]

$(document).on('scroll' ...
эта штука срабатывает при КАЖДОМ событии скролла. А их может быть ОЧЕНЬ много. Чтобы убедиться можете добавить console.log() и посмотреть на флуд.
Почитайте про throttle/debounce

Идем дальше:

$('#firstScroll').each(function() { ...
$('#box6').each(function() { ...

WTF? У вас там множество элементов с одинаковым id?

И каждый раз вы ищете в DOM эти элементы, и для каждого высчитываете одни и те-же данные
$(document).scrollTop() + windowHeight

Почему не найти элементы заранее, не сделать вычисления общие вычисления один раз для всех?

Лезем глубже:

if (скролл больше чего-то там)  {|
  какойтоЭлемент.addClass('fadeInTop').fadeIn(700);
}

Серьезно? То есть доскроллили до нужного места, все красиво появилось, крутим колесо дальше, и продолжаем дальше спамить .addClass().fadeIn() по много раз в секунду? Ну зашибись.

P.S.: сори что немного грубо, на пришлось вашу кашу прогонять через jsbeautifier, а то кровь из глаз начала идти.

Comments

[Сергей Миллер]

подскажите, пожалуйста, как это правильно делается? Есть 10 элементов на странице каждый из них анимируется когда достроил до нужных мест. Как это правильно сделать, а то вообще не соображаю.