2
\$\begingroup\$

I'm using jquery and cookie.jquery to remember the scroll position. Is there anything I can do to improve my code? Thanks!

$(document).ready(function() {

    // convert cokkie variable for easy access
    var prev_scroll_position = $.cookie('prev_scroll_position');

    // scroll to position
    $(window).scrollTop(prev_scroll_position);

    // update cookie when user scrolls
    $(window).scroll(function (event) {
        var scroll_positon = $(window).scrollTop();
        $.cookie('prev_scroll_position', scroll_positon, { expires: 7, path: '/' });
    });

});
\$\endgroup\$
2
\$\begingroup\$

@EthanBierlein Made a really good point, but he forgot something important.

It's always a good idea to wrap your code on an anonymous function:

(function ($) {
    $(document).ready(function () {
        [code]
    });
})(window.jQuery);

This prevents problems in case you run jQuery.noConflict() before, but you forget it.


You have the following code block:

$(window).scroll(function (event) {
    var scroll_positon = $(window).scrollTop();
    $.cookie('prev_scroll_position', scroll_positon, { expires: 7, path: '/' });
});

Instead of this, consider wrapping the $.cookie call in a setTimeout:

$(window).scroll(function (event) {
    var scroll_positon = $(window).scrollTop();
    setTimeout(function () {
        $.cookie('prev_scroll_position', scroll_positon, { expires: 7, path: '/' });
    }, 10);
});

This won't block the scroll event, which may improve UI responsiveness.

Why is that? The scroll event may be triggered multiple times a second, and that setTimeout schedules the cookie writting to a time that the browser is free.

Or that has some time for writing the cookie.

You should experiment with it.


Update!

As @Pevara pointed out in the comment section, this is not 100% a good idea.

He suggests to use a clearTimeout. From there, I could infer what he was trying to say.

Still follow the advice to use a setTimeout, but with a bigger delay (as he suggested: 250ms).

But now, the twist:

var timeout;
$(window).scroll(function (event) {
    var scroll_positon = $(window).scrollTop();

    clearTimeout(timeout); //clear it to avoid crazy writing

    //and create a new interval to write it later
    timeout = setTimeout(function () {
        $.cookie('prev_scroll_position', scroll_positon, { expires: 7, path: '/' });
    }, 250);
});

This will eliminate all the craziness of writing cookies like mad! And will write it only once, saving the browser from re-re-re-re-writing it.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ While I basically agree with your answer, and believe a debounce function should definitely be used here, yours is missing a (critical!) clearTimeout as far as I can tell. Not much debouncing going on here, just a little delay before you start writing cookies like crazy. Also 10ms is hardly a delay... I would at least go for 250ms here \$\endgroup\$ – Pevara Aug 3 '15 at 20:19
  • \$\begingroup\$ @Pevara I see what you mean with the clearTimeout. That would be a great idea! Only write the cookie if you want n milliseconds. That really is amazing! I will look into it in 10 minutes (I'm eating now) \$\endgroup\$ – Ismael Miguel Aug 3 '15 at 20:33
  • \$\begingroup\$ @Pevara Fixed it! Hope it is what you meant (or better). \$\endgroup\$ – Ismael Miguel Aug 3 '15 at 20:50
  • \$\begingroup\$ Much better! Not something I invented btw, it's a well known technique called Debouncing, it is even build right into popular js toolboxes like underscore.js \$\endgroup\$ – Pevara Aug 3 '15 at 21:30
  • \$\begingroup\$ @Pevara I had no idea of the name, but at the time I didn't remembered of it. I know it is quite used and I've used it myself a few times before. Thank you for the reminder. I'm sure I can use it somewhere again. \$\endgroup\$ – Ismael Miguel Aug 3 '15 at 21:32
0
\$\begingroup\$

There's really not much to improve here. The only thing I'd recommend would be to remove $(document).ready(function() { ... }). See this Stackoverflow answer for more detail.

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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