-
Notifications
You must be signed in to change notification settings - Fork 27.3k
feat($rootScope): Implement stopPropatagion for $broadcast events #15877
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.
googlebot
commented
Apr 1, 2017
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!) and we'll verify. Thanks.
- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
- If you signed the CLA as a corporation, please let us know the company's name.
1 similar comment
googlebot
commented
Apr 1, 2017
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!) and we'll verify. Thanks.
- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
- If you signed the CLA as a corporation, please let us know the company's name.
StrangelyTyped
commented
Apr 1, 2017
I signed it!
googlebot
commented
Apr 1, 2017
CLAs look good, thanks!
1 similar comment
googlebot
commented
Apr 1, 2017
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:
- This change brings $broadcast and it's dispatched event object into line with $emit and it's event.
- $broadcast-ed events and $emit-ed events are both dispatched through $on, but at present only one has a stopPropagation method, meaning $on needs to be aware of whether or not the event came from above via $broadcast or below via $emit.
- Any arguments for needing stopPropagation on $emit-ed events also applies to $broadcast.
- Given the angular movement towards component-heirarchy-based applications it is likely to become more common to need to intercept broadcast events to handle them at a higher level and either suppress it or broadcast a modified event instead.
- The established 'workaround' for lack of stopPropagation is to use preventDefault, but this only works if the author has control over both event handlers to check preventDefault which is not always the case. It also affects all subsequent receivers not just the ones below the one that called preventDefault.
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
Narretz
commented
Apr 19, 2017
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: