views:

143

answers:

2

The issue I am having is when the function is called multiple times for different elements. I believe I need to localize all of the variables such that this function can be used against multiple elements. In case it matters I also call in jquery 1.3.

If I only call pageSlide once, everything is fine, but once I call it multiple times it gets painful. No errors, just erratic behaviour.

The code has been updated

var slideMenu=function(){
  var speed, startWidth, time, menuId, liID, menuLen, menuWidth, globalWidth, openWidth;
  return{

    speed : 0, startWidth : 0, time  : 0, menuId  : 0, liID : 0, menuLen  : 0,        menuWidth  : 0, globalWidth : 0, openWidth : 0,

    build:function(ulID,passStartWidth,passTime,s,passSlideLen,passHeight){
      speed=s;
      startWidth=passStartWidth;
      time=passTime;
      menuId=document.getElementById(ulID);
      liID=menuId.getElementsByTagName('li');
      menuLen=liID.length;
      menuWidth=menuId.offsetWidth;
      globalWidth=menuWidth/menuLen;
      openWidth=Math.floor((menuWidth-startWidth)/(menuLen-1));
      var i=0;
      for(i;i<menuLen;i++){
        s=liID[i];
        s.style.width=globalWidth+'px';
        this.timer(s)
      }
      if(passSlideLen!=null){
        menuId.timer=setInterval(function(){
          slideMenu.slide(liID[passSlideLen-1])
        },time)
      }
    },
    timer:function(s){
      s.onmouseover=function(){
        clearInterval(menuId.htimer);
        clearInterval(menuId.timer);
        menuId.timer=setInterval(function(){
          slideMenu.slide(s)
        },
        time)
      }
      s.onmouseout=function(){
        clearInterval(menuId.timer);
        clearInterval(menuId.htimer);
        menuId.htimer=setInterval(function(){
          slideMenu.slide(s,true)
        },
        time)
      }
    },
    slide:function(s,passChange){
      var changeWidth=parseInt(s.style.width);
      if((changeWidth<startWidth && !passChange) || (changeWidth>globalWidth && passChange)){
        var overallWidth=0;
        var i=0;
        for(i;i<menuLen;i++){
          if(liID[i]!=s){
            var slideObj,openWidth; var opening=0; slideObj=liID[i]; openWidth=parseInt(slideObj.style.width);
            if(openWidth<globalWidth && passChange){
              opening=Math.floor((globalWidth-openWidth)/speed);
              opening=(opening>0)?opening:1;
              slideObj.style.width=(openWidth+opening)+'px';
            }else if(openWidth>openWidth && !passChange){
              opening=Math.floor((openWidth-openWidth)/speed);
              opening=(opening>0)?opening:1;
              slideObj.style.width=(openWidth-opening)+'px'
            }
            if(passChange){
              overallWidth=overallWidth+(openWidth+opening)}else{overallWidth=overallWidth+(openWidth-opening)
            }
          }
        }
        s.style.width=(menuWidth-overallWidth)+'px';
      }else{
        clearInterval(menuId.timer);
        clearInterval(menuId.htimer)
      }
    }
  };
}();

The code above does not error but fails to work. When I use the this keyword, it doesn't get any better.

My question is which variables should be "this.". I have tried various combinations that I thought would work, but I am missing something.

+1  A: 

You get this behavior because you are using the module pattern which allows to hold private static variables in your object. The problem is that those variables are shared between all instances.

  var sp=0;
  var st=0;
  var t=0;
  var m='';
  var sa='';
  var l=0;
  var w=0;
  var gw=0;
  var ot=0;

You have to move these variables into your public instance aka in the return part of your script or in the constructor (but in this case you will have to provide a getter for every variable).

return{

   // Place your variables here
   sp : 0,
   st : 0,

   ...

   build:function(sm,sw,mt,s,sl,h){
      // And then use the this keyword to access the variable
      this.sp=s;
      this.st=sw;
      t=mt;
      m=document.getElementById(sm);
      sa=m.getElementsByTagName('li');
      l=sa.length;     
      w=m.offsetWidth;     
      gw=w/l;
      ...
SleepyCod
Good answer. Do you need to use "this" to access the variables, though? What happens if you just write "sp=s;"?
buti-oxa
It will only work if a private static variable exists with the same name in the constructor or resides in the same scope (local variable) with the same name. Make sure to use the "this" keyword to guarantee the uniqueness of your variables per instance.
SleepyCod
Would I use the this keyword on the lines this.sp=s down to this.gw=this.w/this.l since many of those values are global. The gw=w/l would then become this.gw=this.w/this.l ?Thank you for your help.
When I do put a "this." in front of every refernce to the variables defined at the top (sp,st,t,m,sa,l,w,gw,ot) I end up getting the js error: this.sa has no properties in the line slideMenu.slide(this.sa[sl-1]). I am sure I am over using the this keyword out of frustration and obvious lack of understanding. Thanks for the help.
I updated the code.
+2  A: 

I think you're misunderstanding the overall concept of the module pattern, when you need to have a global static variable shared between all your instances then you can declare or initialize your variable with the keyword "var" just before the return statement.

Assume that whenever you change those variables all your instances will be impacted by the change.

var slideMenu=function(){
    // Declare or initialize your private static variables here    
    var speed, startWidth, time;

    return {
        // Public part of your object
        slide:function(s,passChange){
            //To access your variable        
            speed = 20;
        ...

On the other hand if you want to keep variable safe from the global scope, you have to put your variable into the object returned by your function, this is a reliable way to provide unique per instance properties.

var slideMenu=function(){
    // Declare or initialize your private static variables here    
    var speed, startWidth, time;

    return {
        // Public part of your object
        // Declare internal or public properties here
        menuId  : 0, 
        liID    : 0, 
        menuLen : 0,
        ...
        slide:function(s,passChange){
            //To access your private static variable inside functions        
            speed = 20;
            // To access your public/internal properties
            this.menuId = s; // for example ;) 
        }

To conclude

Sometimes to help distinguish between internal / public properties some folks writes a leading underscore on their internal properties (note that properties will still remain accessible from the outside).

Hope it helps, good luck!

SleepyCod
it helped a ton. Thank you!!