-
Notifications
You must be signed in to change notification settings - Fork 421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
can.observe.delegate only fires once for wildcard selectors, skips subsequent batch changes. #122
Comments
I think it's intended that way? How are you using it? Sent from my iPhone On Oct 19, 2012, at 5:15 AM, Anthony Mann notifications@github.com wrote:
|
For non-wildcard selectors this makes sense, however for wildcard selectors it makes sense to receive all events that match the wildcard, not just the first. I think the fiddle provides an accurate description of this behaviour. Note that the fiddle is intentionally simplified, a stronger use case within that context would be: observeInstance.delegate('*.baz.*.boz', 'change', function(ev, prop, oldVal, newVal){
//expect all events that match selector, only first is fired
}) FYI, commenting out the batchNum check https://github.com/jupiterjs/canjs/blob/master/observe/delegate/delegate.js#L69 allows the selector to be matched for all events. This is obviously not a good solution, but demonstrates correct behaviour for wildcards |
So i would like to fix this issue, but it seems to be in conflict with the following: This test: https://github.com/jupiterjs/canjs/blob/master/observe/delegate/delegate_test.js#L168 is preventing the issue from being fixed. Why should compound sets only be notified of the first change in a set that matches either selector? Surely we should be firing all events that match either selector, even within a batch. I will happily create a pull request that changes compound sets and wildcards to match all events from a single batch. |
I typically don't want this. If a change event happens I typically update the dom, I don't want to update it multiple times. Are you intending to branch in your handler? How? Sent from my iPhone On Oct 23, 2012, at 11:33 AM, Anthony Mann notifications@github.com wrote:
|
Understood, but i don't think this is a strong enough argument to enforce this functionality ubiquitously. I can see how this makes sense for compound sets, but less so for wildcards. There are many use cases for delegates outside of the context of updating the DOM, and even when doing so, you still might need to know all object mutations (e.g animation etc). In this case, i think extending the delegate API to provide this functionality optionally would be a good solution: /**
* @param {String} selector The attributes you want to listen for changes in.
* @param {String} event The event name. One of ("set","add","remove","change")
* @param {Boolean} [matchAll=false] Selector can optionally match all events in a batch change.
* @param {Function} handler(ev,newVal,oldVal,prop) The callback handler
**/
delegate : function(selector, event, matchAll, handler){ ... EDIT: |
@MrNibbles can you ping me on skype (justinbmeyer) or irc? |
Selectors containing wildcards are only firing the first change notification in a single batch:
See fiddle: http://jsfiddle.net/2L74R/2/
The text was updated successfully, but these errors were encountered: