Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($rootScope): implement event.stopPropagation() for $broadcast-ed events #12672

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat($rootScope): implement event.stopPropagation() for $broadcast-ed events #12672

wants to merge 1 commit into from

Conversation

just-boris
Copy link
Contributor

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.

@googlebot
Copy link

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!) 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.

@just-boris
Copy link
Contributor Author

I signed it as just-boris@hotmail.com

@just-boris
Copy link
Contributor Author

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))))) {
Copy link
Contributor Author

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 ifs?
But then it will be completely different than traversal in $digest

@googlebot
Copy link

CLAs look good, thanks!

@Narretz
Copy link
Contributor

Narretz commented Sep 24, 2015

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iit -> it

@just-boris
Copy link
Contributor Author

@Narretz fixed. Now all tests are passing fine.

The reasons are similar to stoping $emit-ed events. If some component on top level already handled this event (showed popover, for example), then children don't need to handle this event again. Now as a workaround I have to use some flags and check that on children and code looks ugly.

I guess that wasn't implemented earlier, because it is more rare case than $emit and harder to implement (you need to stop only one branch in the tree and go on by others).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants