-
Notifications
You must be signed in to change notification settings - Fork 27.3k
fix(Scope): $broadcast and $emit should set event.currentScope to null #7523
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1563,15 +1563,30 @@ describe('Scope', function() { | |
|
|
||
| describe('event object', function() { | ||
| it('should have methods/properties', function() { | ||
| var event; | ||
| var eventFired = false; | ||
|
|
||
| child.$on('myEvent', function(e) { | ||
| expect(e.targetScope).toBe(grandChild); | ||
| expect(e.currentScope).toBe(child); | ||
| expect(e.name).toBe('myEvent'); | ||
| eventFired = true; | ||
| }); | ||
| grandChild.$emit('myEvent'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this assertion is still valid, but it probably doesn't matter a lot
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah. it doesn't add any value, so I removed it. |
||
| expect(eventFired).toBeDefined(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's defined to begin with, should be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch. I was refactoring old test and left in the old assertion. fixed. |
||
| }); | ||
|
|
||
|
|
||
| it("should have it's `currentScope` property set to null after emit", function() { | ||
| var event; | ||
|
|
||
| child.$on('myEvent', function(e) { | ||
| event = e; | ||
| }); | ||
| grandChild.$emit('myEvent'); | ||
| expect(event).toBeDefined(); | ||
|
|
||
| expect(event.currentScope).toBe(null); | ||
| expect(event.targetScope).toBe(grandChild); | ||
| expect(event.name).toBe('myEvent'); | ||
| }); | ||
|
|
||
|
|
||
|
|
@@ -1584,6 +1599,7 @@ describe('Scope', function() { | |
| }); | ||
| event = grandChild.$emit('myEvent'); | ||
| expect(event.defaultPrevented).toBe(true); | ||
| expect(event.currentScope).toBe(null); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
@@ -1698,6 +1714,24 @@ describe('Scope', function() { | |
|
|
||
| describe('listener', function() { | ||
| it('should receive event object', inject(function($rootScope) { | ||
| var scope = $rootScope, | ||
| child = scope.$new(), | ||
| eventFired = false; | ||
|
|
||
| child.$on('fooEvent', function(event) { | ||
| eventFired = true; | ||
| expect(event.name).toBe('fooEvent'); | ||
| expect(event.targetScope).toBe(scope); | ||
| expect(event.currentScope).toBe(child); | ||
| }); | ||
| scope.$broadcast('fooEvent'); | ||
|
|
||
| expect(eventFired).toBe(true); | ||
| })); | ||
|
|
||
|
|
||
| it("should have the event's `currentScope` property set to null after broadcast", | ||
| inject(function($rootScope) { | ||
| var scope = $rootScope, | ||
| child = scope.$new(), | ||
| event; | ||
|
|
@@ -1709,7 +1743,7 @@ describe('Scope', function() { | |
|
|
||
| expect(event.name).toBe('fooEvent'); | ||
| expect(event.targetScope).toBe(scope); | ||
| expect(event.currentScope).toBe(child); | ||
| expect(event.currentScope).toBe(null); | ||
| })); | ||
|
|
||
|
|
||
|
|
||
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.
Nitpick: this line isn't really adding much