-
Notifications
You must be signed in to change notification settings - Fork 27.4k
add template url argument to $includeContentRequested event #8454
Conversation
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
@mary-poppins Okay, you should be good to go! |
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
This sounds like a good idea to me but the PR will need unit tests and a documentation update too. Can you look into that please? |
Yeah, I'll work on that. |
@petebacondarwin Hey, I added a test and updated the docs. Do you want to take a look? |
element = $compile('<div><ng:include src="\'/some/template.html\'"></ng:include></div>')($rootScope); | ||
$rootScope.$digest(); | ||
expect(called).toBe(1); | ||
})); |
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.
I think it is cleaner to use jasmine spies for this kind of test. Something like this:
it('should fire $includeContentLoaded with the template url as the second parameter', inject(function($rootScope, $compile, $templateCache) {
var handlerSpy = jasmine.createSpy('handler');
$templateCache.put('/some/template.html', [200, '', {}]);
$rootScope.$on('$includeContentLoaded', handlerSpy);
element = $compile('<div><ng:include src="\'/some/template.html\'"></ng:include></div>')($rootScope);
$rootScope.$digest();
expect(handlerSpy).toHaveBeenCalledWith(jasmine.any(Object), '/some/template.html');
}));
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.
Ok, check it out!
@petebacondarwin ping |
Thanks @stephenbunch - I added the param to all three events in the end and tweaked the tests. |
02dc2aa
to
fd2d6c0
Compare
@petebacondarwin Sweet thanks! |
#8453