-
Notifications
You must be signed in to change notification settings - Fork 27.6k
feat($rootScope): Implement stopPropatagion for $broadcast events #15877
base: master
Are you sure you want to change the base?
feat($rootScope): Implement stopPropatagion for $broadcast events #15877
Conversation
$scope.$broadcast dispatches an event to all child scopes by traversing the scope tree in a depth-first manner. This change allows any scope to prevent it's children from receiving that event by calling stopPropagation on the event object. Other listeners on the scope which called stopPropagation will continue to receive the event, as will siblings of that scope and any children of those siblings.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, 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.
|
1 similar comment
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, 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! |
CLAs look good, thanks! |
1 similar comment
CLAs look good, thanks! |
I'm aware (though didn't spot it until after I'd already done mine) that there has been a previous PR for this functionality here. I don't care which of these is preferred - mine or just-boris' - but I would like to see one of them accepted for the following reasons:
|
// (though it differs due to having the extra check for $$listenerCount) | ||
if (!(next = ((current.$$listenerCount[name] && current.$$childHead) || | ||
// (though it differs due to having the extra check for $$listenerCount and stopPropagation) | ||
if (!(next = ((current.$$listenerCount[name] && !stopPropagation && current.$$childHead) || | ||
(current !== target && 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.
PR #12672 has an additional check here, why not this PR?
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.
Actually it doesn't, it has an assignment here, since it's not part of the condition and sin e the condition is already confusing enough i put mine on line 1310
Personally, I'm kind of indifferent about this change, with the following caveat: The scope event system in AngularJS is kind of bolted on the scope system and should not be a cornerstone of application design. Componentized app architecture should imo rely on the bindings as clearer interfaces - events rely on scopes, component bindings abstract them. So I don't think component archtitecture creates a big need for this - otherwise it would have been requested before and more loudly. |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
$scope.$broadcast events cannot be cancelled and do not have a stopPropagation() function/method
What is the new behavior (if this is a feature change)?
This change allows any scope to prevent it's children from receiving that
event by calling stopPropagation on the event object. Other listeners on the scope which called
stopPropagation will continue to receive the event, as will siblings of that scope and any
children of those siblings.
Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information: