Skip to content

Commit

Permalink
Refactor and debugging based on code review and browser testing.
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Mathews committed Aug 28, 2009
1 parent 8d7f9f9 commit 6b09b00
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 59 deletions.
94 changes: 54 additions & 40 deletions src/dom/dom.js
View file
Expand Up @@ -79,15 +79,15 @@
@type String
@description The property name added to the DomElement by the NodeList#data method.
*/
dataPropName = "_glowData" + glow.UID,
dataPropName = "_uniqueData" + glow.UID,

/**
@name glow.dom-dataIndex
@private
@type String
@description The value of the dataPropName added by the NodeList#data method.
*/
dataIndex = 0,
dataIndex = 1, // must be a truthy value

/**
@name glow.dom-dataCache
Expand Down Expand Up @@ -2226,24 +2226,33 @@
var ret = [],
i = this.length,
allCloneElms,
allBaseElms
eventIdProp = '__eventId' + glow.UID;

while (i--) {
ret[i] = this[i].cloneNode(true);
// copy data onto the new clone
glow.dom.get(ret[i]).data(glow.dom.get(this[i]).data());
}

// some browsers (ie) also clone node properties as attributes
// we need to get rid of the eventId.
allCloneElms = r.get( ret ).get("*").push( ret );
if (nodePropertiesCloned && !isXml(ret[0])) {
allCloneElms = r.get( ret ).get("*").push( ret );
i = allCloneElms.length;
while(i--) {
allCloneElms[i][eventIdProp] = null;
}
}

// copy data from base elements to clone elements
allBaseElms = this.get("*").push( this );
i = allCloneElms.length;
while (i--) {
allCloneElms[i].removeAttribute(dataPropName);
glow.dom.get(allCloneElms[i]).data(
glow.dom.get(allBaseElms[i]).data()
);
}

// shall we clone events too?
if (cloneListeners) {
// check the stuff we need is hanging around, we don't want
Expand Down Expand Up @@ -3057,45 +3066,47 @@
glow.dom.get("p").data("tea", "milky");
var colour = glow.dom.get("p").data("tea"); // milky
@returns {Object} When setting a value this method can be chained, as in that case it will return itself.
*/
data: function (key, val) { /*debug*///console.log("data()");
data: function (key, val) { /*debug*///console.log("data("+key+", "+val+")");
if (typeof key === "object") { // setting many values
for (var prop in key) { this.data(prop, key[prop]); }
return this; // chainable with ({key: val}) signature
}

var elm = null,
i = this.length,
index = null;
// uses private class-scoped variables: dataCache, dataPropName, dataIndex
var index,
elm;
// uses private class-scoped variables: dataCache, dataPropName, dataIndex

while (i--) {
elm = this[i];

if (elm[dataPropName] === undefined) {
elm[dataPropName] = dataIndex;
dataCache[dataIndex++] = {};
}

index = elm[dataPropName];

// TODO - need to defend against reserved words being used as keys?
switch (arguments.length) {
case 0:
return dataCache[index];
case 1:
return dataCache[index][key];
case 2:
switch (arguments.length) {
case 0: // getting entire cache from first node
if (this[0] === undefined) { return undefined; }
index = this[0][dataPropName] || dataIndex++;
return dataCache[index] || (dataCache[index] = {}); // create a new cache when reading entire cache
case 1: // getting val from first node
if (this[0] === undefined) { return undefined; }
index = this[0][dataPropName]; // don't create a new cache when reading just a specific val
return index? dataCache[index][key] : undefined;
case 2: // setting key:val on every node
// TODO - need to defend against reserved words being used as keys?
for (var i = this.length; i--;) {
elm = this[i];

if ( !(index = elm[dataPropName]) ) { // assumes index is always > 0
index = dataIndex++;

elm[dataPropName] = index;
dataCache[index] = {};
}
dataCache[index][key] = val;
break;
default:
throw new Error("glow.dom.NodeList#data expects 2 or less arguments, not "+arguments.length+".");
}
}

return this; // chainable with (key, val) signature
default:
throw new Error("glow.dom.NodeList#data expects 2 or less arguments, not "+arguments.length+".");
}
},

// TODO - clone() and destroy() need to handle attached data

/**
@name glow.dom.NodeList#removeData
@function
Expand All @@ -3107,11 +3118,11 @@
@param {String} [key] The name of the value in glow's data store for the NodeList item.
*/
removeData: function (key) {
var elm = null,
removeData: function (key) { /*debug*///console.log("removeData("+key+")");
var elm,
i = this.length,
index = null;
// uses private class-scoped variables: dataCache, dataPropName
index;
// uses private class-scoped variables: dataCache, dataPropName

while (i--) {
elm = this[i];
Expand All @@ -3120,23 +3131,26 @@
if (index !== undefined) {
switch (arguments.length) {
case 0:
delete dataCache[index];
dataCache[index] = undefined;
elm[dataPropName] = undefined;
try {
delete elm[dataPropName]; // IE 6 goes wobbly here
}
catch(e) { // remove expando from IE 6
elm[dataPropName] = undefined;
elm.removeAttribute && elm.removeAttribute(dataPropName);
}
break;
case 1:
dataCache[index][key] = undefined;
delete dataCache[index][key];
break;
default:
throw new Error("glow.dom.NodeList#removeData expects 1 or less arguments, not "+arguments.length+".");
}
}
}

return this; // chainable
},

/**
Expand Down
49 changes: 30 additions & 19 deletions test/glow/dom/dom.js
View file
Expand Up @@ -2044,11 +2044,11 @@ t.test("glow.dom.NodeList.css getting", function() {
t.equals(glow.dom.get("#cssTests ul.listTest li").css("list-style-position"), "outside", "list-style-position (on li)");
t.equals(glow.dom.get("#cssTests ul.listTest li").css("list-style-type"), "square", "list-style-type (on li)");

//colour tests
//color tests
t.equals(glow.dom.get("#cssTests div.colourTest").css("color"), "rgb(0, 255, 0)", "color");
t.equals(glow.dom.get("#cssTests div.colourTest div").css("color"), "rgb(0, 255, 0)", "color nested");
//safari gets this one a little wrong, but it's ok
t.ok(/rgb\(12[78], 12[78], 12[78]\)/.test(glow.dom.get("#cssTests div.colourTest").css("background-color")), "background-color (percentage colour)");
t.ok(/rgb\(12[78], 12[78], 12[78]\)/.test(glow.dom.get("#cssTests div.colourTest").css("background-color")), "background-color (percentage color)");
t.equals(glow.dom.get("#cssTests div.colourTest").css("border-left-color"), "rgb(0, 128, 0)", "border-left-color (keyword)");

//background test
Expand Down Expand Up @@ -2265,50 +2265,61 @@ t.test("glow.dom.NodeList#data API", function() {
});

t.test("glow.dom.NodeList#data method", function() {
t.expect(5);
var dataTest = glow.dom.get("#dataTest p");
t.expect(7);

dataTest.data("colour", "red");
t.equals(dataTest.data("colour"), "red", "Can set and get a key:val from NodeList.");
var dataTest = glow.dom.get("#dataTest > p");
var self = dataTest.data("color", "red");
t.equals(dataTest.data("color"), "red", "Can set and get a key:val from NodeList.");
t.ok((dataTest === self), "The call to set a key:val is chainable.");

var data = dataTest.data();
t.equals(data.colour, "red", "Can get the entire data object from NodeList when given no arguments.");
t.equals(data.color, "red", "Can get the entire data object from NodeList when given no arguments.");

data = glow.dom.get("#para1").data();
t.equals(data.colour, "red", "Can get the same data from different NodeLists that refer to the same DomElements.");
var para1 = glow.dom.get("#para1");
t.equals(para1.data("color"), "red", "Can get the same data from different NodeLists that refer to the same DomElements.");

dataTest.data({
self = dataTest.data({
size: "grande",
count: 8
});
t.equals(dataTest.data("size"), "grande", "Can set multiple key:vals at once.");
t.equals(dataTest.data("count"), 8, "All the multiple key:vals are set.");
t.ok((dataTest === self), "The call to multiple key:val is chainable.");

});

t.test("glow.dom.NodeList#removeData method", function() {
t.expect(4);
t.expect(5);
var dataTest = glow.dom.get("#dataTest p");

var data = dataTest.data();
t.equals(data.colour, "red", "Data is already set on the NodeList.");
dataTest.removeData("colour");
t.equals(data.colour, undefined, "Can remove data by key name.");
t.equals(data.color, "red", "Data is already set on the NodeList.");
dataTest.removeData("color");
t.equals(data.color, undefined, "Can remove data by key name.");

t.equals(data.size, "grande", "More data is already set on the NodeList.");

dataTest.removeData();
var self = dataTest.removeData();
data = dataTest.data();
t.equals(data.size, undefined, "can remove all data at once.");
t.equals(data.size, undefined, "Can remove all data at once.");
t.ok( (dataTest === self), "The call to removeData is chainable.");
});

t.test("glow.dom.NodeList#data Clone", function() {
t.expect(1);
t.expect(3);

var testData = glow.dom.get("#dataTest");
testData.data("tea", "milky");

var testClone = glow.dom.get("#dataTest").clone();
t.equals(testClone.data("tea"), "milky", "Cloned nodes have the same data as the nodes they are based on.");
testData.get("#span1").data("biscuits", 2);
var testClone = testData.clone();
t.equals(testClone.get("#span1").data("biscuits"), 2, "A cloned node has a copy of the data from the node it's based on.");

t.equals(testClone.data("tea"), "milky", "A child node in a cloned node has the same data as the node it's based on.");

testData.data("tea", "lemony");
t.equals(testClone.data("tea"), "milky", "Changing the original data doesn't change the cloned data.");

});

t.test("glow.dom.NodeList#data Destroy", function() {
Expand Down

0 comments on commit 6b09b00

Please sign in to comment.