Jquery: functions using the same global variable messing with my code

Question

Based on this awnser: How can I determine the direction of a jQuery scroll event?

I did a function like this:

tempScrollTop=0;
$.fn.cool = function(options){
    windowTop = $(window).scrollTop(); 

    if (tempScrollTop < windowTop ){
        //scroll down
    }
    else if (tempScrollTop > windowTop ){
        //scroll up
    }

    tempScrollTop = windowTop;
};

but each time I try to use my function

 $(window).scroll(function(e) {
    $("#element1").cool();
    $("#element2").cool();
 }

$("#element2") takes the global variable tempScrollTop already modified by $("#element1") and for element 2 tempScrollTop and windowTop has the same value, so my function doesn't work.

Any ideas on what can I do? I dont want to create n functions one for each element.

Why would you need to check the scrolling direction of the window on each element ? — adeneo, Dec 04 '12 at 16:02
Why are you setting a global variable? Shouldn't you be returning the value of `windowTop` from the function call and using that return value? — Joe Green, Dec 04 '12 at 16:07
@adeneo. I need the scrolling direction to move the element up or down depending on the event. — Gaddiel Sadoc Peralta, Dec 04 '12 at 16:15
@JoeGreen. I need the global variable bc of the implementation of this awnser [link](http://stackoverflow.com/questions/4326845/how-can-i-determine-the-direction-of-a-jquery-scroll-event). do you think there is any other better method? — Gaddiel Sadoc Peralta, Dec 04 '12 at 16:16

Christophe · Accepted Answer · 2012-12-04T18:47:56.587

1

It seems that what you actually want to do is:

$("#element1","#element2").cool();

Then in your cool function:

tempScrollTop=0;
$.fn.cool = function(options){
windowTop = $(window).scrollTop(); 

if (tempScrollTop < windowTop ){
    //scroll down
    this.each(function() {
        // do stuff with each selected element $(this)
    });
}
else if (tempScrollTop > windowTop ){
    //scroll down
    this.each(function() {
        // do stuff with each selected element $(this)
    });
}

tempScrollTop = windowTop;
};

More info on plugin authoring here.

Another way would be to separate the scroll calculation from the action on the elements:

$(window).scroll(function(e) {
    var scrollDiff=calculateScroll();
    $("#element1").cool(scrollDiff);
    $("#element2").cool(scrollDiff);
}

edited Dec 04 '12 at 18:47

answered Dec 04 '12 at 16:10

Christophe

27,383
28
97
140

This does not solve the question about using a global variable. This is exactly the same than calling both separated. – Capagris Dec 04 '12 at 16:17
@Capagris certainly not! With my code the scroll is only calculated once. – Christophe Dec 04 '12 at 16:19
You are right if calculated once and always together. Nevertheless, I would still go for a solution that allows using each scrolling element independently. – Capagris Dec 04 '12 at 16:28
@Capagris but the whole point is that they don't scroll independently :-) – Christophe Dec 04 '12 at 16:34
@Christophe I m tring to use your answer but it says "Cannot read property 'top' of undefined" how do you use $(this) in your implementation... and is something like $(this).children(funcion... ??? – Gaddiel Sadoc Peralta Dec 04 '12 at 16:38
@GaddielSadocPeralta I'd need to see more of your code. In my code the selector has more than one element, so you might need to use $().each to loop through them. – Christophe Dec 04 '12 at 16:41
@GaddielSadocPeralta I have added more details, hope this helps. Within a plugin, use this.each() to loop through the selected elements, and $(this) for each element. – Christophe Dec 04 '12 at 17:43
@Christophe let me do the proper modifications and I will tell you if it worked, those details changes lots of things in my real code – Gaddiel Sadoc Peralta Dec 04 '12 at 18:43
@GaddielSadocPeralta I understand. I have added another suggestion that would require less changes. – Christophe Dec 04 '12 at 18:48
@Christophe that less changes addition worked like a charm, thank you very much kind sir – Gaddiel Sadoc Peralta Dec 04 '12 at 20:01

Michael Malinovskij · Answer 2 · 2012-12-04T17:45:43.050

0

var tempScrollTop = 0,
    innerScrollTop;
    $.fn.cool = function(options){
        var $this = $(this);
        innerScrollTop = $this.data('scrollTop') || tempScrollTop;

        windowTop = $(window).scrollTop();

        if (innerScrollTop <= windowTop ){
            //scroll down
        } else {
            //scroll up
        }

        $this.data('scrollTop', windowTop);
    };

edited Dec 04 '12 at 17:45

answered Dec 04 '12 at 16:03

Michael Malinovskij

1,422
8
22

no, sorry it was my error that code is not a copy paste, is an example made for stackoverflow, but thank you on pointing out, (I m removing it from the original question) – Gaddiel Sadoc Peralta Dec 04 '12 at 16:10
1

You would need to modify tempScrollTop in the if to this.tempScrollTop. Apart from that, it is a good answer. Nevertheless, there is no explanation when this page should not just contain code but a way of learning for beginners. What Michael is proposing is to add the information of the current situation of the scroll to the object itself through this. That makes this solution suitable for multiple scrolls, at the same time that we write it only once into an anonymous function which will decide on runtime which object is being affected. – Capagris Dec 04 '12 at 16:11
this code: this.tempScrollTop = this.tempScrollTop || tempScrollTop; somehow this.tempScrollTop is always taking 0 (guess the seccond option) – Gaddiel Sadoc Peralta Dec 04 '12 at 17:09
Changed saving scroll from this.tempScrollTop to $(element).data('scrollTop'); – Michael Malinovskij Dec 04 '12 at 17:46

Jquery: functions using the same global variable messing with my code

2 Answers2