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

add template url argument to $includeContentRequested event #8454

Closed
wants to merge 3 commits into from

Conversation

stephenbunch
Copy link
Contributor

@mary-poppins
Copy link

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.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@stephenbunch
Copy link
Contributor Author

@mary-poppins Okay, you should be good to go!

@mary-poppins
Copy link

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

@petebacondarwin
Copy link
Member

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?

@petebacondarwin petebacondarwin added this to the 1.3.0 milestone Aug 2, 2014
@stephenbunch
Copy link
Contributor Author

Yeah, I'll work on that.

@stephenbunch
Copy link
Contributor Author

@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);
}));
Copy link
Member

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');
  }));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, check it out!

@stephenbunch
Copy link
Contributor Author

@petebacondarwin ping

@petebacondarwin
Copy link
Member

Thanks @stephenbunch - I added the param to all three events in the end and tweaked the tests.

@stephenbunch
Copy link
Contributor Author

@petebacondarwin Sweet thanks!

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.

None yet

4 participants