-
Notifications
You must be signed in to change notification settings - Fork 27.4k
feat($rootScope): implement event.stopPropagation() for $broadcast-ed events #12672
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it as just-boris@hotmail.com |
I signed it! |
if (!(next = ((current.$$listenerCount[name] && current.$$childHead) || | ||
(current !== target && current.$$nextSibling)))) { | ||
if (!(next = ((current.$$listenerCount[name] && !stopPropagation && current.$$childHead) || | ||
(current !== target && ((stopPropagation = false) || current.$$nextSibling))))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it should split that complicated expression into more if
s?
But then it will be completely different than traversal in $digest
CLAs look good, thanks! |
I am not sure what the reasons were for not having stopPropagation on broadcast. However, what's the use case for using stopPropagation this way? Normally, you use events pretty sparingly in an angular app. |
})); | ||
|
||
|
||
iit('should stop propagation only for one scope chain', inject(function($rootScope) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iit -> it
@Narretz fixed. Now all tests are passing fine. The reasons are similar to stoping I guess that wasn't implemented earlier, because it is more rare case than |
It would more consistent if both ways to trigger an event will have
stopPropagation
feature.Implemented as described in #914
Also there is a related question on stackoverlow.