classList usage in dom.{add|remove|toggle}CssClass + minor changes#1237
classList usage in dom.{add|remove|toggle}CssClass + minor changes#1237nightwing merged 5 commits intoajaxorg:masterajaxorg/ace:masterfrom
Conversation
There was a problem hiding this comment.
Why not just test el.textContent ?
There was a problem hiding this comment.
i'd say el.textContent || el.innerText || "" would be ok unless it throws on ie8 or something
There was a problem hiding this comment.
Can you please explain what do you mean?
There was a problem hiding this comment.
I thought to replace whole thing with one line, but it can't be done, because el.textContent can be "", maybe use if ("textContent" in el)?
There was a problem hiding this comment.
Can we do this way:
if ("textContent" in document.documentElement) {
exports.setInnerText = function(el, innerText) {
el.textContent = innerText;
};
exports.getInnerText = function(el) {
return el.textContent;
};
}
else {
exports.setInnerText = function(el, innerText) {
el.innerText = innerText;
};
exports.getInnerText = function(el) {
return el.innerText;
};
}or at least:
exports.setInnerText = function(el, innerText) {
if ("textContent" in el)
el.textContent = innerText;
else
el.innerText = innerText;
};
exports.getInnerText = function(el) {
if ("textContent" in el)
return el.textContent;
else
return el.innerText;
};?
There was a problem hiding this comment.
By the way, textContent works on Webkit much slower then innerText. But I'm not sure innerText is future-proof:
http://wiki.whatwg.org/wiki/Specs_todo :
innerText and outerText, if browsers don't remove them entirely
There was a problem hiding this comment.
i like the first one
if ("textContent" in document.documentElement) {
exports.setInnerText = function(el, innerText) {
....There was a problem hiding this comment.
Another way:
var textProperty = ("innerText" in document.documentElement) ? "innerText" : "textContent";
exports.setInnerText = function(el, innerText) {
el[textProperty] = innerText;
};
exports.getInnerText = function(el) {
return el[textProperty];
};
don't know, it is probably code from bespin, not sure if it's worth to change.
why not |
Ok, no need to change this ). But some js libs has on/un methods. or bind/unbind. or addListener/removeListener. I didn't see on/off :) But underscore for _dispatchEvent is unexpected. Would be nice to change this sometime. |
|
i didn't know about |
classList usage in dom.{add|remove|toggle}CssClass + minor changes
I thought this was the joke ) But jQuery uses on/off now. So I think this also is acceptable. |
lib/event_emitter: why do you use underscore for _dispatchEvent ? We have
on | addEventListener, so, we should haveun | removeEventListenerinstead ofremoveListener | removeEventListener