This repository was archived by the owner on Apr 12, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
transclude implicitly creates new scope.
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.
But this is creating an "isolated" scope
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.
read this:
angular.js/src/ng/compile.js
Line 242 in 727b232
transclude is a sibling scope of directive scope, but it inherits from parent scope and it does not matter what type(inherit or isolate) directive scope is.
look here:
angular.js/src/ng/compile.js
Line 929 in 727b232
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.
The example "doesn't work" without the "scope: {}" line, it renders "Check out the contents, Jeff!".
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.
@benol - I think you are wrong here. It does work without that line. And as @matjaz says, a new child scope will be created because of the transclusion, which means that changing$scope.name
inside the directive will not affect the$scope.name
in the scope passed to the transcluded content.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.
@benol : sorry I got confused by the two commits. Yes, you are right that we need a new scope here. Otherwise the directive does not create a scope of its own and so writing
in the link function will change the scope outside.
@matjaz - you are right that the transclusion scope is a new child scope of the outerscope but since it inherits from this scope, changing the outerscope will change the inner scope.
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.
you're right. unless directive has new scope, name gets overridden. but the new scope doesn't have to be isolate.
so there are two things in master to fix
create new scope
link function arguments
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.
We don't need to drop element parameter as this is not fundamentally wrong
leaving it in.
Regarding the scope. It is very rare to have transclusion without an
isolated scope, so again, keeping it thus will be less confusing in the
long run.
On 5 Nov 2013 17:32, "Matjaž Lipuš" [email protected] wrote: