views:

46

answers:

3

I'm trying to make a simple in in-page popup called like this:

var test = new popObject({}); //JSON options

and I'm having trouble because when I create two in a row, and call show() on the first one, the second one always shows. Both are created, but they aren't separate somehow, despite being called with new. What am I doing wrong here? I've included my code, but I have removed out the irrelevant functions for compactness.

function popObject(options) {   
//functions

show = function() {
    console.log(boxselector);
    jQuery(boxselector).css("display", "block");
    return jQuery(boxselector);
}

var hide = function() {...}
var update = function(updateOptions) {...}
var calcTop = function(passedHeight) {...}
var calcLeft = function(passedWidth) {...}
var calcHeight = function(passedHeight) {...}
var stripUnits = function(measure, auto) {...}
var destroy = function() {...}

//public functions

this.show = show;
this.hide = hide;
this.update = update;
this.destroy = destroy;

//constants

name = options.name; //name should never be changed.
boxselector = ".boxcontainer[name=" + options.name + "]";
boxbodyselector = ".boxbody[name=" + options.name + "]";
boxtitleselector = ".boxcontainer[name=" + options.name + "]"
boxboxselector = ".boxbox[name=" + options.name + "]"
title = options.title;
content = options.content;  
width = options.width;
height = options.height;

this.name = name;
this.selectors = [boxselector, boxbodyselector, boxtitleselector, boxboxselector]
this.title = title;
this.content = content;
this.width = width;
this.height = height;

//variables

popupHtml = ...

//init code

jQuery("#dropzone").append(popupHtml); this.init = null;
jQuery(".boxbox[name=" + name + "]").css("top", calcTop(width));
jQuery(".boxbox[name=" + name + "]").css("left", calcLeft(height));
jQuery(".boxbody[name=" + name + "]").css("height", calcHeight(height));
}
+4  A: 

This is because you're declaring a lot of variables in the global scope. Try the following code instead:

function popObject(options) {   
//functions

this.show = function() {
    console.log(boxselector);
    jQuery(boxselector).css("display", "block");
    return jQuery(boxselector);
}

var hide = function() {...}
var update = function(updateOptions) {...}
var calcTop = function(passedHeight) {...}
var calcLeft = function(passedWidth) {...}
var calcHeight = function(passedHeight) {...}
var stripUnits = function(measure, auto) {...}
var destroy = function() {...}

//public functions

this.show = show;
this.hide = hide;
this.update = update;
this.destroy = destroy;

//constants

var name = options.name; //name should never be changed.
var boxselector = ".boxcontainer[name=" + options.name + "]";
var boxbodyselector = ".boxbody[name=" + options.name + "]";
var boxtitleselector = ".boxcontainer[name=" + options.name + "]"
var boxboxselector = ".boxbox[name=" + options.name + "]"
var title = options.title;
var content = options.content;  
var width = options.width;
var height = options.height;

this.name = name;
this.selectors = [boxselector, boxbodyselector, boxtitleselector, boxboxselector]
this.title = title;
this.content = content;
this.width = width;
this.height = height;

//variables

var popupHtml = ...

//init code

jQuery("#dropzone").append(popupHtml); this.init = null;
jQuery(".boxbox[name=" + name + "]").css("top", calcTop(width));
jQuery(".boxbox[name=" + name + "]").css("left", calcLeft(height));
jQuery(".boxbody[name=" + name + "]").css("height", calcHeight(height));
}

Note all the vars that weren't there before. This defines them as local to the function, and thus local to your object (and also, essentially, private... use this. instead of var to make public members).

Anything that isn't declared with a var or a this. is considered global. So, when you called show(), it used the global show, which referenced the object that was created later.

Ryan Kinal
+1 Beat me to it
Jamie Wong
This is so important to do it cannot be stressed enough. Always use var unless you are absolutely sure you want a global variable, and even then, you probably shouldn't be using a global variable!
Russell Leggett
even if you DO want a global variable, you should probably be doing `window.variable_name` to make it clear it's not a reassignment of a local variable.
Jamie Wong
Still feel like I'm just getting started with javascript. I had no idea about that. Thanks a bunch.
Reactor5
Always glad to help!
Ryan Kinal
A: 

What is boxselector? If it's a generic selector then it would select all elements on the page, regardless if its inside of that unique object.

meder
`boxselector` is declared later in the object. In the original code, it's an implied global. In my solution, it's a local variable.
Ryan Kinal
A: 

When you declare something without var or this within a function definition, such as

boxselector = ".boxcontainer[name=" + options.name + "]";

It creates it in the global namespace (attaches it to window)

Try changing this line to

var boxselector = ".boxcontainer[name=" + options.name + "]";

Jamie Wong