Scrolling event being fired too many times

Question

This code works and does the job, but it's poor, rudimentary and has some issues with the scrolling event. Can somebody help me to optimize the scrolling event?

Prototype

Structure and desired behaviour:

• There are two containers, .layout__left (red) and .layout__right (green).
• The green container will have the main content and will scroll normally.
• The red container will have the navigation so it will have less content and should scroll up to a certain point only, until the menu reaches the border of the page.

This is what I am doing:

1. First both containers scroll normally.
2. When scrolling down the red container, if the scroll position + window size is bigger than the li:last child position (couldnt think of anyway to detect the "bottom"), then the container has a position fixed.
3. When scrolling up, the fixed position is disabled and when the menu reaches the top of the document, the position is fixed again, and then removed.

As I said, it does the job, but, it's slow and has glitches while scrolling. I think it's because I am doing on the scroll event on every pixel change.

I tried throttling the function call, but the code doesn't function at all:

    $(window).on('scroll', function() {
    $.throttle(100, sectionScroll));
    });

    sectionScroll = function() {
    var lastScrollTop = 0, delta = 5;
    var left = $('.layout__left');
    var right = $('.layout__right');
    var scroll = $(this).scrollTop();
    var viewport = $(window).height();
    var lastChild = $('.navigation > ul > li:last-child').offset().top;

    if(Math.abs(lastScrollTop - scroll) <= delta)
    return;

    if (scroll > lastScrollTop){
    // SCROLL DOWN
    left.removeClass('top absolute');

    // if the last item of the left is visible
    if (  (scroll + viewport) > (lastChild + 30)  ) {
    // make the left fixed
    left.addClass('fixed bottom');
    console.log("bottom!");
    }

    // SCROLL UP
    } else {
    // remove fixed class from left
    left.removeClass('fixed bottom');
    left.addClass('absolute bottom');

    if ( scroll <= left.position().top   ) {
    left.removeClass('bottom absolute');
    left.addClass('top fixed');
    left.removeClass('top fixed');
    console.log("top!");
    }
    }

    lastScrollTop = scroll;
    };

Any feedback?

Answer 1

Bug!

You have a small bug in your code:

$.throttle(100, sectionScroll));

See it? You have one-too-many parentheses. This line should be:

$.throttle(100, sectionScroll);

---

Debounce

You say in your title that the scrolling event is being fired too many times.

Well, this makes perfect sense: as long as the user has their mouse over the scroll bar and is moving the bar ever-so-slightly, the scroll event is going to try and fire as quick as it can.

Even for the tiniest movement, this event could fire over 50 times.

However, there is a simple way to prevent your code from trying to run that many times: a debounce.

A debounce is when you setup a variable to prevent code (typically an event) from running while another piece of code (often the same code) is still running.

Every time this scroll event fires, a new thread is going to be created with the event code running in it; a debounce would prevent these from running at the same time.

It is very simple to create a debounce; you just need a variable and a conditional.

Here is what the basic design would look like:

var debounce = true;
function foo() {
    if(debounce) {
        debounce = false;
        [code]
        debounce = true;
    }
}

The first line in the conditional would stop any other thread from running because they are all sharing a single debounce variable.

Now, all you need to do to apply this to your code would be to treat foo in the example as your sectionScroll function, create the debounce variable, and then encase the function code in the if statement.

Then, after adding the lines for setting and un-setting the debounce, you should be good to go.

---

The above is really the meat of my review, everything else is just miscellaneous.

---

Indentation

You have none! Which, in my opinion, is better than having bad indentation.

Luckily for you there is a handy-dandy tool that you probably already know about: JSFiddle. At the very top of the page is a button labeled "TidyUp".

1. Paste your code into the "JavaScript" code box.
2. Give that "TidyUp" button a smack.
3. ???
4. Profit!

It's really as simple as that: using that "TidyUp" button will fully and properly indent your code.

However, don't get too used to this: you want to be able to tell right from wrong without having to consult something else.

Study the changes that that button made to your code. You may want to go back and forth a couple of times. And, read other JavaScript code that has proper indentation. Over time, you should be able to develop a strong sense of what is right, and what is wrong.

Note: I actually had to use this tool in order to review your code; I couldn't read it otherwise!

---