-
Notifications
You must be signed in to change notification settings - Fork 27.4k
Conversation
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
@IgorMinar - seems to solve the problem http://plnkr.co/edit/AqCHhmudH8XxDwP4oGPM?p=preview |
LGTM, I think this is a correct fix. |
These two commits should be squashed I think as it's fixing a bug introduced in the first one (during refactoring/cleaning up from the original e10bd88). |
If you have two directives that both expect to receive transcluded content the outer directive works but the inner directive never receives a transclusion function. This only failed if the first transclude directive was not the first directive found in compilation. Handles the regression identified in e994259 Fixes angular#7240 Closes angular#7387
|
||
it("should not pick up too many children when transcluding", inject(function($compile, $rootScope) { | ||
var element = $compile('<div my-example></div>')($rootScope); | ||
$rootScope.$digest(); |
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.
why do we need $digest
twice?
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.
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.
@vojtajina This is not needed now I've rewritten the ngif fix. I'll close this and send in a series of prs for all this today
@petebacondarwin I'm running google tests to make sure this is fine, will let you know when you can submit this. |
Closed by #7610 |
No description provided.