search

component / reactive

Tiny reactive template engine
382 stars 48 forks source link

Adding missing operations for Arrays #142

Closed Thomas-P closed 10 years ago

Thomas-P commented 10 years ago

Hi,

I added some missing features for work with reactive. Now you could remove and sort Items in the array or reverse them an it would copied to the view.

Hope it will help,

Thomas

PS: by copying the code, i killed line 155 of each.js if you want to keep the line, please notify me.

defunctzombie commented 10 years ago

Awesome. I will review.

One quick item style item is missing braces for if statements.

Thomas-P commented 10 years ago

I commited a fix for the brackets. Hope I have understood this correctly.

defunctzombie commented 10 years ago

Please squash into one commit.

Why does the property need to be configurable?

Thomas-P commented 10 years ago

Ok, I have squash them into one commit. I don't know why it should be configurable, i think it should't, but it is in the actual version at https://github.com/component/reactive/blob/master/lib/each.js#L155

I removed my changes for this in the commit. There are actually only the missing functions for array operations and some semicolons i added. Hope it will help.

defunctzombie commented 10 years ago

In addition to the comments, this PR needs tests for all the functions added. You don't have to go crazy but some basic tests and edge cases would be nice.

Thomas-P commented 10 years ago

Hi,

i have fixed the styles and added the test for the functions i implemented. Don't wonder about the return of void 0 on empty arrays. It's the same as undefined, you could check this with 

[].pop()=== void 0;

So i hope i've done it right and it's now easy to merge.