CODEWITHSUNDEEP
javascriptjqueryperformancescroll
user1512126
user1512126

Reputation: 99

Jquery scroll event causing performance issues

I'm trying to use the browser scroll event to place a block of html based on the amount a user has scrolled. The code works but it is causing a huge performance issue which basically forces my browser to freeze.

Any insight as to why and what I could do to resolve this would be greatly appreciated.

<script type="text/javascript">

$('#content').scroll(function () {
    var scroll = $('#content').scrollTop();
    var $controls = $(".controls").clone();
    if (scroll > 200) {
        $(".controls").remove();
        $('#header').append($controls);
    }
    else {
        $(".controls").remove();
        $('.banner').append($controls);
    }
});

</script>

Upvotes: 4

Views: 10544

Answers (4)

Beetroot-Beetroot
Beetroot-Beetroot

Reputation: 18078

First, discovering elements in the DOM is an expensive activity, so cache your jQuery objects.

Second, .append() moves elements around so .clone() and remove() should be unnecessary.

This gives :

var $$ = {//cache of jQuery objects
    content: $('#content'),
    controls: $(".controls"),
    header: $("#header"),
    banner: $('.banner')
};
$('#content').scroll(function() {
    $controls.appendTo(($$.content.scrollTop() > 200) ? $$.header : $$.banner);
});

Now, you can work on reducing the frequency at which the handler is called, which can be achieved as follows :

var $$ = {//cache of jQuery objects
    content: $('#content'),
    controls: $(".controls"),
    header: $("#header"),
    banner: $('.banner')
};

var scrollHandling = {
    allow: true,
    reallow: function() {
        scrollHandling.allow = true;
    },
    delay: 50 //(milliseconds) adjust to the highest acceptable value
};

$('#content').scroll(function() {
    if(scrollHandling.allow) {
        $controls.appendTo(($$.content.scrollTop() > 200) ? $$.header : $$.banner);
        scrollHandling.allow = false;
        setTimeout(scrollHandling.reallow, scrollHandling.delay);
    }
});

Upvotes: 14

Marc Baumbach
Marc Baumbach

Reputation: 10473

The scroll function is called for every movement of the scrollbar. That can potentially be a lot of times, so you need to be careful how much code you are running and certainly how much manipulation of the DOM you are doing.

In this case, you'll be repeating a lot of the same actions (clone, append, remove) as the scrolling is occurring, but it appears that you only want to flip between two states as you cross back and forth over that 200 scroll value. You could potentially solve most of the performance issues with something like this:

<script type="text/javascript">

var thresholdCrossed = false;

$('#content').scroll(function () {
    var scroll = $('#content').scrollTop();
    var threshold = 200;
    if (scroll > threshold && !thresholdCrossed) {
        var controls = $(".controls").clone();
        $(".controls").remove();
        $('#header').append(controls);
    } else if (scroll <= threshold && thresholdCrossed) {
        var controls = $(".controls").clone();
        $(".controls").remove();
        $('.banner').append(controls);
    }
    thresholdCrossed = scroll > threshold;
});

</script>

You can do additional work that some of the other answers describe to help reduce wasted resources, but this should give you a general idea of how to hopefully help the main performance concern of constantly modifying the DOM as you scroll. I would probably suggest some combination of this along with the answer provided by @Kolink so that you are truly limiting the DOM manipulation to the least amount necessary.

Upvotes: 3

Niet the Dark Absol
Niet the Dark Absol

Reputation: 324650

You are cloning all .controls elements every tick of scrolling, even when it is not needed.

I would suggest cloning the controls on ready and setting it to display:none. Then, just toggle the display based on the scroll position.

On re-reading your question, it looks like you're just moving the controls element from the header to the banner? In that case, you don't even need a clone. However I strongly suggest adding id="controls" to the controls element, and id="banner" - use IDs instead of classes in general if there is only one.

document.getElementById('content').onscroll = function() {
    document.getElementById(this.scrollTop > 200 ? "banner" : "header")
       .appendChild(document.getElementById('controls'));
};

Upvotes: 2

AgnosticDev
AgnosticDev

Reputation: 1853

Every time the scroll bar moves jQuery has to go into the DOM to get the variables you references. To start cache the variables so jQuery doesnt have to do twice the work.

          var content = $('#content');

          content.scroll(function(){


          });

Upvotes: 0

Related Questions