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

Update directive.ngdoc #4774

Closed
wants to merge 2 commits into from
Closed

Update directive.ngdoc #4774

wants to merge 2 commits into from

Conversation

bendowski
Copy link
Contributor

The example about transclusion and scopes worked only because the order of scope and element arguments is reversed. To really work, the directive has to define its own scope (at least that's my understanding of it).

The example about transclusion and scopes worked only because the order of scope and element arguments is reversed. To really work, the directive has to define its own scope (at least that's my understanding of it).
templateUrl: 'my-dialog.html',
link: function (element, scope) {
link: function (scope, element) {
Copy link

Choose a reason for hiding this comment

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

element is not needed.

@petebacondarwin
Copy link
Contributor

After the bit about scope: {} was fixed this leaves the change: "removing the element parameter". It is not "wrong" to include this parameter, since the function will be called with this value. It is just unnecessary since it is not used. That being said, since it is common practice in link functions to include the parameters that will be passed in even if they are not used, it is better to leave it in here. A good minifier, will be able to identify that this parameter is not used and remove it from the function declaration.

petebacondarwin pushed a commit that referenced this pull request Nov 5, 2013
The example about transclusion and scopes worked only because the order of `scope` and `element`
arguments is wrong, which means that the `name' property of the scope is not really being updated.
To really work, the directive has to define its own scope, either a new child scope or, as is more
common with transclusion, an isolated scope.

Closes #4774
@bendowski bendowski deleted the patch-1 branch November 17, 2013 17:22
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
The example about transclusion and scopes worked only because the order of `scope` and `element`
arguments is wrong, which means that the `name' property of the scope is not really being updated.
To really work, the directive has to define its own scope, either a new child scope or, as is more
common with transclusion, an isolated scope.

Closes angular#4774
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
The example about transclusion and scopes worked only because the order of `scope` and `element`
arguments is wrong, which means that the `name' property of the scope is not really being updated.
To really work, the directive has to define its own scope, either a new child scope or, as is more
common with transclusion, an isolated scope.

Closes angular#4774
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.

3 participants