Skip to content

[1.2] Add unit test for ui-view compilation #664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[1.2] Add unit test for ui-view compilation #664

wants to merge 2 commits into from

Conversation

dlukez
Copy link
Contributor

@dlukez dlukez commented Dec 10, 2013

This adds a test that ensures that directives like ng-class are compiled against the ui-view element and update correctly. It fails with the current angular-1.2 branch, but succeeds in the PR #553 (biased much?). I will try and get the test working with the current branch, though.

This issue has been brought up in #320 (comment), #514, #621 and #662.

It also updates the angular libraries to the stable 1.2 release, which I imagine will be the minimum supported version.

@dlukez
Copy link
Contributor Author

dlukez commented Dec 10, 2013

I don't know if it can be fixed in the way the code is currently, because I'm not sure how you can manually clone an element and ensure it's still scope-bound? I think transclusion is needed for this. Anybody - ideas?

@nateabele
Copy link
Contributor

@dlukez Hey, sorry, I've been totally swamped with work the past couple days. I have some updates coming that will affect the dependency changes.

In order to avoid any more botched releases, I've completely revamped the test suite, and am in the process of integrating changes from the 1.2 branch. Instead of transclusion, it's going to use the same compilation strategy as ngInclude, which will hopefully solve any further rendering issues.

I'll push all those changes in a day or two and then we can come back and integrate the test, as well as some of your stuff from the uiView refactor. Thanks again for the help and thanks for being patient with all the back-and-forth.

@dlukez
Copy link
Contributor Author

dlukez commented Dec 12, 2013

@nateabele all good! Sounds like a wise move, let me know when you need a hand.

In regards to ngInclude, It looks like it does use transclusion currently, but through the recently added $transclude function that is passed to the linking function:

https://github.com/angular/angular.js/blob/master/src/ng/directive/ngInclude.js#L197

@dlukez
Copy link
Contributor Author

dlukez commented Feb 13, 2014

Related to #866

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

Successfully merging this pull request may close these issues.

2 participants