Changed event parameters to always include collection; "diff" hash for MultiSelect #29
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deselectAll is more consistent with the rest of the API; selectNone is kept around for compatibility.
If a selectable model belongs to more than one selectable collection then only the initial collection will have it's select() method called. Also, if a model is initially added to a non-selectable collection, and then later added to a selectable collection the 2nd collection will not have it's select() method called, and a error will be thrown as the initial collection will not have a select() method.
Backbone 1.0.0, Underscore 1.5.0. Using the listenTo and stopListening methods requires at least Backbone 0.9.9.
The extended model or collection is passed to the mixin as an argument now.
Events still need to be improved, those tests are failing.
Plus trivial changes in the tests to get rid of some warnings when committing.
…e in spec) Previously, selectAll fired a select:all event even if all models had already been selected beforehand. Similarly, deselectAll fired a select:none event even if no model had been selected in the first place. No-ops like this don't trigger an event any more.
Handlers named onSelect, onDeselect etc are run without having to be wired up first.
Passed as the second parameter, options are now third. The collection info is required by event handlers listening to more than one collection. Also, some overly narrow tests have been fixed.
Breaking change: MultiSelect `select:*` events provide the "diff" hash as the second parameter. The collection is now third parameter (used to be second). Providing the diff as third parameter would have been backward compatible. But then the API would have been inconsistent with SingleSelect (arg pattern: change, context (collection), options). Choosing consistency over compatibility (probably no big deal, args hadn't even been documented). Also, some overly narrow tests have been fixed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR provides additional info to event handlers.
SingleSelect events now pass the collection as the second parameter. The collection info is required by event handlers listening to more than one collection.
(That's a side effect of multi-collection support, #16 / #23.
model.collectionused to be enough, but it just gives access to the first collection a model is assigned to.)MultiSelect
select:*events provide a "diff" hash,{selected: [...], deselected: [...]}, as the second parameter. The collection is now passed as the third parameter (used to be second).The MultiSelect change could have been backward compatible by providing the diff as third parameter, keeping the collection second. But then the API would have been inconsistent with SingleSelect (arg pattern: modification (model), context (collection), options). I reckon the break in compatibility is not a big deal, though, the args hadn't even been documented so far.
Based on my previous commits. Tests are adjusted, documentation updated.