Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add .empty #20

Merged
merged 4 commits into from Mar 13, 2013

Conversation

Projects
None yet
2 participants
Owner

vendethiel commented Jan 12, 2013

I think we can import some more methods (allow .html() to set innerHTML, for example) and I can do a bigger PR if you want.
Maybe use component/css ?

@tj tj and 1 other commented on an outdated diff Feb 23, 2013

test/dom.js
@@ -246,6 +246,18 @@ describe('.filter(fn)', function(){
})
})
+describe('.empty()', function(){
+ it('should return itself for chaining', function(){
+ var list = dom('<div></div>');
+ assert(list == list.empty());
+ })
+
+ it('should empty the element(s)', function(){
+ var list = dom('<div></div>');
+ assert('' == list.empty().html());
@tj

tj Feb 23, 2013

Owner

we should match jquery here, with jquery if you had <div>Hello</div> you'd end up with <div></div>, also we could probably get away with .innerHTML = '' to speed things up

@vendethiel

vendethiel Feb 23, 2013

Owner

will do when I get home. does this needs a rebase too ?

@tj

tj Feb 23, 2013

Owner

nope I think this PR is fine

Owner

vendethiel commented Feb 23, 2013

@visionmedia I used removeChild() because that's what jquery is doing. I have no idea why OTTOMH, do you ?

Owner

tj commented Feb 24, 2013

weird maybe it is faster then (havent benched, too early for that), sounds good then!

@vendethiel vendethiel closed this Feb 28, 2013

Owner

vendethiel commented Feb 28, 2013

actually this one was the ok one - no need for the other pr since you did it yourself

@vendethiel vendethiel reopened this Feb 28, 2013

Owner

tj commented Mar 1, 2013

yup this is all good, just needs a rebase

Owner

vendethiel commented Mar 1, 2013

Voilà, the conflict was on readme

tj added a commit that referenced this pull request Mar 13, 2013

@tj tj merged commit 997d6be into component:master Mar 13, 2013

@vendethiel vendethiel deleted the vendethiel:patch-3 branch Mar 13, 2013

Owner

vendethiel commented Mar 13, 2013

thanks!

@tj tj commented on the diff Mar 13, 2013

@@ -616,7 +616,7 @@ List.prototype.find = function(selector){
// TODO: real implementation
var list = new List([], this.selector);
var el, els;
- for (var i = 0; i < this.els.length; ++i) {
+ for (var i = 0, len = this.els.length; i < len; ++i) {
@tj

tj Mar 13, 2013

Owner

no need for this browsers can optimize that away

@vendethiel

vendethiel Mar 13, 2013

Owner

I've learnt it that way, will remember from now on :).

Owner

vendethiel commented Apr 14, 2013

weird maybe it is faster then (havent benched, too early for that), sounds good then!

Definitely faster.

http://jsperf.com/emptify-element/2
innerHTML = null : 1,890Ops
firstChild.remove() : 61,250,827Ops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment