Liveupdating subsets #1

Merged
merged 3 commits into from Jan 26, 2012

Conversation

Projects
None yet
3 participants
@latentflip
Contributor

latentflip commented Jan 25, 2012

Hi guys.

I found subset last night, and it's looking pretty awesome. For my use however, I wanted subsets to be automatically updated if a model's attributes change. i.e:

tasks.reset([{archived: true}, {archived: false}]);

assert.equal(tasks.length, 2);
assert.equal(archivedTasks.length, 1);

tasks.first().set({archived: false});
assert.equal(tasks.length, 2);
assert.equal(archivedTasks.length, 0);

This makes it really easy to keep my views in sync with any changes made to models by the user/app. So here's a patch that does just that.

I've added a liveupdate_keys attribute to the subset. If set to 'all', the subsets will be updated on all model changes. If set to an array of attributes (liveupdate_keys = ['archived']), the subset will only be updated when those keys change. If set to 'none' or '' the subset will not be live updated.

Before merging, I'd love some discussion/feedback on:

  • is this a good idea?
  • should liveupdating be the default behaviour (currently liveupdate_keys defaults to 'all' in my branch).
  • is liveupdate_keys an appropriate name for the property

Thanks!
Phil

latentflip added some commits Jan 25, 2012

Add a liveupdate_keys option to keep the subset updated, when model a…
…ttributes change that would make them now fail/pass the sieve.

By default subset's will be liveupdated (liveupdate_keys = 'all').

To limit the model keys on which the subset will be liveupdated, set liveupdate_keys to an array of model attributes. liveupdate_keys = ['archived', 'dated_on'].
backbone.subset.js
@@ -25,6 +25,7 @@
this.model = this.parent().model;
this.comparator = this.comparator || options.comparator || this.parent().comparator;
+ this.liveupdate_keys = options.liveupdate_keys || 'all';

This comment has been minimized.

Show comment Hide comment
@saimonmoore

saimonmoore Jan 26, 2012

Collaborator

I'd say default to liveupdating not being active by default.

@saimonmoore

saimonmoore Jan 26, 2012

Collaborator

I'd say default to liveupdating not being active by default.

This comment has been minimized.

Show comment Hide comment
@latentflip

latentflip Jan 26, 2012

Contributor

Yeah I think that's a good idea so that it won't affect existing implementations that use subset. I'll update the pull request.

@latentflip

latentflip Jan 26, 2012

Contributor

Yeah I think that's a good idea so that it won't affect existing implementations that use subset. I'll update the pull request.

@saimonmoore

This comment has been minimized.

Show comment Hide comment
@saimonmoore

saimonmoore Jan 26, 2012

Collaborator

Looks good...Thanks...

Collaborator

saimonmoore commented Jan 26, 2012

Looks good...Thanks...

saimonmoore added a commit that referenced this pull request Jan 26, 2012

@saimonmoore saimonmoore merged commit b0c29c4 into masylum:master Jan 26, 2012

@masylum

This comment has been minimized.

Show comment Hide comment
@masylum

masylum Jan 29, 2012

Owner

Good pull request. Thanks.

Owner

masylum commented Jan 29, 2012

Good pull request. Thanks.

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