views:

109

answers:

4

Here's what I'm trying to achieve:

I have a div that switches between fixed and absolute positioning depending on the scroll position. I have an example that works but I noticed that it's a little slow because it constantly changes the position on each and every pixel of scroll. So I thought that adding an additional if statement (as kind of a on and off switch) would remedy it a bit. But of course it broke.

I rarely use jquery/javascript so to my eyes this seems right but it's not.. any help? Maybe there's even a better way of doing this than if statements.

var top = blah;
var bottom = blah;
var ison=0;
$(window).scroll(function (event) {
    var y = $(this).scrollTop();
    if (y >= top && y <= bottom) {
        if (ison===0) {
            $('#float-contain').addClass('fixed');
            var ison = 1;
        }
    } else {
        if (ison===1) {
            $('#float-contain').removeClass('fixed');
            var ison = 0;
        }
    }
});
A: 

Most likely the problem is that the value of ison is always undefined when you try accessing it, because you are defining it inside the function, but only after trying to read it.

Whenever you define a variable inside a function, even if it is not declared until later in the code, it automatically overrides any variables with the same name outside the scope - thus inside the function, the value of ison is undefined until it's defined, and the code never reaches that because of your if clauses.

Try removing var from the var ison = n statements inside the function and that may help. This would make it access the variable you declare in global scope.

Jani Hartikainen
A: 

Here,

    if (ison===0) {
        $('#float-contain').addClass('fixed');
        var ison = 1;
    }

Delete the var because you're redeclaring ison with it. (do the same for the other branch).

KennyTM
+2  A: 

Try removing the "var" inside each if statement like this:

if (y >= top && y <= bottom) {
    if (ison===0) {
        $('#float-contain').addClass('fixed');
        ison = 1;
    }
} else {
    if (ison===1) {
        $('#float-contain').removeClass('fixed');
        ison = 0;
    }
}

You're already declaring var ison globally above all of this. By redeclaring the variable inside a function you're creating new local instances of it, which causes some undesirable results. I'm not sure if this is you're only problem, but it's definitely part of it.

By the way, here's a good overview of global and local variable in Javascript. They even include an example of identically named global and local variables (which I try to avoid).

Steve Wortham
Thanks, funny thing is that I was looking at that very same thing. I'm definitely going to go through javascript more.
A: 

I think the slowness is due to repeated document.getElementById calls due to $('#float-contain'). You can cache the jQuery object so that you don't do it. addClass/removeClass/toggleClass doesn't do anything if the element already has/doesn't have the class.

This should work just fine:

var top = blah;
var bottom = blah;
var $elem = $('#float-contain');
$(window).scroll(function (event) {
    var y = $(this).scrollTop();
    $elem.toggleClass('fixed', (y >= top && y <= bottom));
});

You can also make this code run when the user has finished scrolling instead of running continuously or you can 'throttle' it to run only once per say 100ms. Here's a way to achieve the first technique - http://www.matts411.com/post/delaying_javascript_event_execution/

Chetan Sastry
Someone else solved the original problem but I feel that your advice will give me a better alternative. Thanks!