Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have this code for a website I'm working on. It uses window.scroll events to parallax some items, It's running a bit slow on average machines. Is there any way I could improve it to make it run faster?

$(function() {

    // Tell the DOM that JS is enabled
    $('html').removeClass('no-js');

    // Navigation Waypoints
    $('.waypoint').waypoint(function(event, d) {
        var a = $(this);
        if (d === "up") a = a.prev();
        if (!a.length) a = a.end();
        $('.active').removeClass('active');
        a.addClass('active');
        $('a[href=#'+a.attr('id')+']').addClass('active');
    }, {offset: '40%'});

    // Parallax Effects
    $(window).scroll(function() {
        var s = $(window).scrollTop(),
            b = ($('body').height()/100)*20;
            $('.container').each(function () {
                $(this).css('top', Math.round((($(this).closest('.waypoint').offset().top-s)+b)/$(this).attr('data-speed')));
            });
            $('#home hgroup').css('bottom', '-'+Math.round(s/6)+'px');
    });

    // FAQs
    $(document).on('click', '.faqs a', function () {
        $('.faqs .open').slideToggle().removeClass('open');
        $(this).siblings().slideToggle('fast').addClass('open');
        return false;
    });

    // Kinetics
    $('.counter').kinetic({
        y: false
    });

    // Smooth Scroll
    $(document).on('click', 'nav a', function() {
        var t = $($(this).attr('href')),
            o = t.offset().top,
            b = ($('body').height()/100)*10;
        if($(this).attr('href') === "#home") {
            $('html, body').animate({scrollTop: o}, 'slow');
        } else {
            $('html, body').animate({scrollTop: o+b}, 'slow');
        }
        return false;
    });

    // Set Margin
    function setMargin () {
        var t = $('#about'),
            o = t.offset().top,
            c = $('.container'),
            b = ($('body').height()/100)*20,
            m = Math.round(parseInt(t.css('margin-top'))),
            a = Math.round((((o/2)+b)+m)/2);
        c.css('top', a);
    }

    // Calculate Padding
    function calcPadding() {
        var p = Math.round(($(window).width()-700)/2);
        $('.pr').css('padding-right', p);
        setWidth();
    }

    // Calculate Width
    function setWidth () {
        $('.draggable').each(function () {
            var w = 0;
            $(this).children().each(function () {
                w = w + $(this).outerWidth() + parseInt($(this).css('margin-left')) + parseInt($(this).css('margin-right'));
            });
            $(this).css('width', w);
        });
    }

    // Window Resize
    $(window).on('resize', calcPadding);

    // Initialize Functions
    calcPadding();
    setMargin();

});
share|improve this question
2  
Except for the idiomatic i in for cycles, I suggest you avoid one-letter variables. var a = Math.round((((o/2)+b)+m)/2) is indeed compact, but not very nice to read. –  ANeves Dec 14 '12 at 15:48
    
I see your point, it was mainly for speed as I needed to get it done ASAP. –  Simon Sturgess Dec 14 '12 at 16:21
add comment

2 Answers

up vote 2 down vote accepted

I don't know what the "waypoint" plugin is, but you might be able to optimize your scroll event handler by caching the jQuery objects, data-speed values, and the results of the .closest('.waypoint') calls:

// Parallax Effects
$(window).scroll(
    (function() {
        // cache the queries
        var $body = $(document.body),
            $containers = $('.container'),
            $hgroup = $('#home hgroup'),
            speeds = [],
            waypoints = [];

        // save the values for data-speed attributes 
        // and the waypoints / container relationships
        // based on the index in $containers
        $containers.each( function(idx) {
            var $this = $(this);
            // if the waypoints never change, you could just put
            // the offset().top value in the waypoints array
            waypoints[idx] = $this.closest('.waypoint');
            speeds[idx] = $this.data('speed'); 
        });

        return function() {

            var s = window.scrollY,
                diff = ($body.height() / 100 * 20) - s;

            // you don't need the Math.round(...)
            $containers.each(function(idx) {
                $(this).css('top', (waypoints[idx].offset().top + diff ) / speeds[idx]);
            });
            $hgroup.css('bottom', s / -6);
        };
    })()
);

If the results of waypoints[idx].offset().top don't change, you could store those values instead of the result of the .closests('.waypoint') in the waypoints array.

Lastly, I don't think you need the 'px' Math.round() when setting the CSS properties with jQuery.

I haven't tested this as you didn't provide a link to a live page or a jsFiddle, but in theory, it should work.

share|improve this answer
    
hey man thanks for the response. the staging site is up at: staging.lightenink.com I'll have a go at implementing your advice tonight! –  Simon Sturgess Dec 18 '12 at 12:44
    
I just implemented your code and result wise is identical, I will try it on a slow machine in a bit to see if the performance has increased. Thanks so much for your help. Will try and get my head around what you did! –  Simon Sturgess Dec 18 '12 at 12:50
    
This code seems to perform pretty well on slowish machines. Thanks so much for all your help! If you have any other advice on how I can improve the way I write my code it would be greatly appreciated! Thanks guys! –  Simon Sturgess Dec 18 '12 at 13:25
add comment

The jQuery documentation offers the best advice:

Event performance

In most cases, an event such as click occurs infrequently and performance is not a significant concern. However, high frequency events such as mousemove or scroll can fire dozens of times per second, and in those cases it becomes more important to use events judiciously. Performance can be increased by reducing the amount of work done in the handler itself, caching information needed by the handler rather than recalculating it, or by rate-limiting the number of actual page updates using setTimeout.

Obviously it would be ideal if you could move the calculations outside the event handler to reach the desired performance, but that may not be possible in this case. So to expand on the rate-limiting approach, there are 2 ways you could go about it:

  1. Call event handler after the user has finished scrolling (hasn't scrolled for some set amount of time):

    var timer = null;
    $(window).scroll(function () {
      if (timer) {
        clearTimeout(timer);
      }
    
      timer = setTimeout(function() {
        timer = null;
    
        // your event handling logic here
      }, 50);
    });
    
  2. Call event handler immediately after scrolling, with a minimum delay between scroll events:

    var justExecuted = false;
    $(window).scroll(function() {
      if(justExecuted) {
        return;
      }
    
      // your event handling logic here
    
      justExecuted = true;
      setTimeout(function() {
        justExecuted = false;
      }, 50);
    });
    
share|improve this answer
    
Hey man, thanks for the response, unfortunately I am updating the css top position of an element using the distance scrolled from the top as the user scrolls so I can't fire the function after the user has scrolled. –  Simon Sturgess Dec 18 '12 at 12:45
    
I don't know if there is a better way of parallaxing my elements tho. –  Simon Sturgess Dec 18 '12 at 12:47
    
@SimonSturgess: My recommendation would be the second approach I listed, where you execute your event handling logic immediately after scrolling, with a minimum delay between each scroll event. You only need to try this if caching the values (as in tiffon's answer) isn't fast enough. –  seand Dec 18 '12 at 13:12
    
I've tested tiffons code on a slowish machine and it seems to perform pretty well. Thanks so much for your responses. I'm an intermediate jquery user, just need to work improving the quality and performance of the code I write. –  Simon Sturgess Dec 18 '12 at 13:24
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.