Skip to content

Commit

Permalink
fix($compile): accessing controllers of transcluded directives from c…
Browse files Browse the repository at this point in the history
…hildren

Additional API (backwards compatible)
- Injects `$transclude` (see directive controllers) as 5th argument to directive link functions.
- `$transclude` takes an optional scope as first parameter that overrides the
  bound scope.

Deprecations:
- `transclude` parameter of directive compile functions (use the new parameter for link functions instead).

Refactorings:
- Don't use comment node to temporarily store controllers
- `ngIf`, `ngRepeat`, ... now all use `$transclude`

Closes angular#4935.
  • Loading branch information
tbosch authored and jamesdaily committed Jan 27, 2014
1 parent 470fae9 commit 9fc52c1
Show file tree
Hide file tree
Showing 11 changed files with 434 additions and 73 deletions.
141 changes: 94 additions & 47 deletions src/ng/compile.js

Large diffs are not rendered by default.

6 changes: 2 additions & 4 deletions src/ng/directive/ngIf.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,14 @@ var ngIfDirective = ['$animate', function($animate) {
terminal: true,
restrict: 'A',
$$tlb: true,
compile: function (element, attr, transclude) {
return function ($scope, $element, $attr) {
link: function ($scope, $element, $attr, ctrl, $transclude) {
var block, childScope;
$scope.$watch($attr.ngIf, function ngIfWatchAction(value) {

if (toBoolean(value)) {
if (!childScope) {
childScope = $scope.$new();
transclude(childScope, function (clone) {
$transclude(childScope, function (clone) {
block = {
startNode: clone[0],
endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ')
Expand All @@ -115,7 +114,6 @@ var ngIfDirective = ['$animate', function($animate) {
}
}
});
};
}
};
}];
6 changes: 3 additions & 3 deletions src/ng/directive/ngInclude.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$compile'
priority: 400,
terminal: true,
transclude: 'element',
compile: function(element, attr, transclusion) {
compile: function(element, attr) {
var srcExp = attr.ngInclude || attr.src,
onloadExp = attr.onload || '',
autoScrollExp = attr.autoscroll;

return function(scope, $element) {
return function(scope, $element, $attr, ctrl, $transclude) {
var changeCounter = 0,
currentScope,
currentElement;
Expand Down Expand Up @@ -188,7 +188,7 @@ var ngIncludeDirective = ['$http', '$templateCache', '$anchorScroll', '$compile'
if (thisChangeId !== changeCounter) return;
var newScope = scope.$new();

transclusion(newScope, function(clone) {
$transclude(newScope, function(clone) {
cleanupLastIncludeContent();

currentScope = newScope;
Expand Down
6 changes: 2 additions & 4 deletions src/ng/directive/ngRepeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
priority: 1000,
terminal: true,
$$tlb: true,
compile: function(element, attr, linker) {
return function($scope, $element, $attr){
link: function($scope, $element, $attr, ctrl, $transclude){
var expression = $attr.ngRepeat;
var match = expression.match(/^\s*(.+)\s+in\s+(.*?)\s*(\s+track\s+by\s+(.+)\s*)?$/),
trackByExp, trackByExpGetter, trackByIdExpFn, trackByIdArrayFn, trackByIdObjFn,
Expand Down Expand Up @@ -364,7 +363,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
// jshint bitwise: true

if (!block.startNode) {
linker(childScope, function(clone) {
$transclude(childScope, function(clone) {
clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' ');
$animate.enter(clone, null, jqLite(previousNode));
previousNode = clone;
Expand All @@ -377,7 +376,6 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
}
lastBlockMap = nextBlockMap;
});
};
}
};
}];
Expand Down
16 changes: 7 additions & 9 deletions src/ng/directive/ngSwitch.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ var ngSwitchWhenDirective = ngDirective({
transclude: 'element',
priority: 800,
require: '^ngSwitch',
compile: function(element, attrs, transclude) {
return function(scope, element, attr, ctrl) {
compile: function(element, attrs) {
return function(scope, element, attr, ctrl, $transclude) {
ctrl.cases['!' + attrs.ngSwitchWhen] = (ctrl.cases['!' + attrs.ngSwitchWhen] || []);
ctrl.cases['!' + attrs.ngSwitchWhen].push({ transclude: transclude, element: element });
ctrl.cases['!' + attrs.ngSwitchWhen].push({ transclude: $transclude, element: element });
};
}
});
Expand All @@ -172,10 +172,8 @@ var ngSwitchDefaultDirective = ngDirective({
transclude: 'element',
priority: 800,
require: '^ngSwitch',
compile: function(element, attrs, transclude) {
return function(scope, element, attr, ctrl) {
ctrl.cases['?'] = (ctrl.cases['?'] || []);
ctrl.cases['?'].push({ transclude: transclude, element: element });
};
}
link: function(scope, element, attr, ctrl, $transclude) {
ctrl.cases['?'] = (ctrl.cases['?'] || []);
ctrl.cases['?'].push({ transclude: $transclude, element: element });
}
});
6 changes: 2 additions & 4 deletions src/ngRoute/directive/ngView.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,7 @@ function ngViewFactory( $route, $anchorScroll, $compile, $controller,
terminal: true,
priority: 400,
transclude: 'element',
compile: function(element, attr, linker) {
return function(scope, $element, attr) {
link: function(scope, $element, attr, ctrl, $transclude) {
var currentScope,
currentElement,
autoScrollExp = attr.autoscroll,
Expand All @@ -200,7 +199,7 @@ function ngViewFactory( $route, $anchorScroll, $compile, $controller,

if (template) {
var newScope = scope.$new();
linker(newScope, function(clone) {
$transclude(newScope, function(clone) {
clone.html(template);
$animate.enter(clone, null, currentElement || $element, function onNgViewEnter () {
if (angular.isDefined(autoScrollExp)
Expand Down Expand Up @@ -235,7 +234,6 @@ function ngViewFactory( $route, $anchorScroll, $compile, $controller,
cleanupLastView();
}
}
};
}
};
}
203 changes: 201 additions & 2 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3438,6 +3438,113 @@ describe('$compile', function() {
expect(log).toEqual('pre(); post(unicorn!)');
});
});

it('should copy the directive controller to all clones', function() {
var transcludeCtrl, cloneCount = 2;
module(function() {
directive('transclude', valueFn({
transclude: 'content',
controller: function($transclude) {
transcludeCtrl = this;
},
link: function(scope, el, attr, ctrl, $transclude) {
var i;
for (i=0; i<cloneCount; i++) {
$transclude(cloneAttach);
}

function cloneAttach(clone) {
el.append(clone);
}
}
}));
});
inject(function($compile) {
element = $compile('<div transclude><span></span></div>')($rootScope);
var children = element.children(), i;
expect(transcludeCtrl).toBeDefined();

expect(element.data('$transcludeController')).toBe(transcludeCtrl);
for (i=0; i<cloneCount; i++) {
expect(children.eq(i).data('$transcludeController')).toBeUndefined();
}
});
});

it('should provide the $transclude controller local as 5th argument to the pre and post-link function', function() {
var ctrlTransclude, preLinkTransclude, postLinkTransclude;
module(function() {
directive('transclude', valueFn({
transclude: 'content',
controller: function($transclude) {
ctrlTransclude = $transclude;
},
compile: function() {
return {
pre: function(scope, el, attr, ctrl, $transclude) {
preLinkTransclude = $transclude;
},
post: function(scope, el, attr, ctrl, $transclude) {
postLinkTransclude = $transclude;
}
};
}
}));
});
inject(function($compile) {
element = $compile('<div transclude></div>')($rootScope);
expect(ctrlTransclude).toBeDefined();
expect(ctrlTransclude).toBe(preLinkTransclude);
expect(ctrlTransclude).toBe(postLinkTransclude);
});
});

it('should allow an optional scope argument in $transclude', function() {
var capturedChildCtrl;
module(function() {
directive('transclude', valueFn({
transclude: 'content',
link: function(scope, element, attr, ctrl, $transclude) {
$transclude(scope, function(clone) {
element.append(clone);
});
}
}));
});
inject(function($compile) {
element = $compile('<div transclude>{{$id}}</div>')($rootScope);
$rootScope.$apply();
expect(element.text()).toBe($rootScope.$id);
});

});

it('should expose the directive controller to transcluded children', function() {
var capturedChildCtrl;
module(function() {
directive('transclude', valueFn({
transclude: 'content',
controller: function() {
},
link: function(scope, element, attr, ctrl, $transclude) {
$transclude(function(clone) {
element.append(clone);
});
}
}));
directive('child', valueFn({
require: '^transclude',
link: function(scope, element, attr, ctrl) {
capturedChildCtrl = ctrl;
}
}));
});
inject(function($compile) {
element = $compile('<div transclude><div child></div></div>')($rootScope);
expect(capturedChildCtrl).toBeTruthy();
});

});
});


Expand Down Expand Up @@ -3471,7 +3578,6 @@ describe('$compile', function() {
});
});


it('should only allow one element transclusion per element', function() {
module(function() {
directive('first', valueFn({
Expand Down Expand Up @@ -3620,8 +3726,101 @@ describe('$compile', function() {
]);
});
});
});

it('should allow to access $transclude in the same directive', function() {
var _$transclude;
module(function() {
directive('transclude', valueFn({
transclude: 'element',
controller: function($transclude) {
_$transclude = $transclude;
}
}));
});
inject(function($compile) {
element = $compile('<div transclude></div>')($rootScope);
expect(_$transclude).toBeDefined()
});
});

it('should copy the directive controller to all clones', function() {
var transcludeCtrl, cloneCount = 2;
module(function() {
directive('transclude', valueFn({
transclude: 'element',
controller: function() {
transcludeCtrl = this;
},
link: function(scope, el, attr, ctrl, $transclude) {
var i;
for (i=0; i<cloneCount; i++) {
$transclude(cloneAttach);
}

function cloneAttach(clone) {
el.after(clone);
}
}
}));
});
inject(function($compile) {
element = $compile('<div><div transclude></div></div>')($rootScope);
var children = element.children(), i;
for (i=0; i<cloneCount; i++) {
expect(children.eq(i).data('$transcludeController')).toBe(transcludeCtrl);
}
});
});

it('should expose the directive controller to transcluded children', function() {
var capturedTranscludeCtrl;
module(function() {
directive('transclude', valueFn({
transclude: 'element',
controller: function() {
},
link: function(scope, element, attr, ctrl, $transclude) {
$transclude(scope, function(clone) {
element.after(clone);
});
}
}));
directive('child', valueFn({
require: '^transclude',
link: function(scope, element, attr, ctrl) {
capturedTranscludeCtrl = ctrl;
}
}));
});
inject(function($compile) {
element = $compile('<div transclude><div child></div></div>')($rootScope);
expect(capturedTranscludeCtrl).toBeTruthy();
});
});

it('should allow access to $transclude in a templateUrl directive', function() {
var transclude;
module(function() {
directive('template', valueFn({
templateUrl: 'template.html',
replace: true
}));
directive('transclude', valueFn({
transclude: 'content',
controller: function($transclude) {
transclude = $transclude;
}
}));
});
inject(function($compile, $httpBackend) {
$httpBackend.expectGET('template.html').respond('<div transclude></div>');
element = $compile('<div template></div>')($rootScope);
$httpBackend.flush();
expect(transclude).toBeDefined();
});
});

});

it('should safely create transclude comment node and not break with "-->"',
inject(function($rootScope) {
Expand Down
28 changes: 28 additions & 0 deletions test/ng/directive/ngIfSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,34 @@ describe('ngIf', function () {

});

describe('ngIf and transcludes', function() {
it('should allow access to directive controller from children when used in a replace template', function() {
var controller;
module(function($compileProvider) {
var directive = $compileProvider.directive;
directive('template', valueFn({
template: '<div ng-if="true"><span test></span></div>',
replace: true,
controller: function() {
this.flag = true;
}
}));
directive('test', valueFn({
require: '^template',
link: function(scope, el, attr, ctrl) {
controller = ctrl;
}
}));
});
inject(function($compile, $rootScope) {
var element = $compile('<div><div template></div></div>')($rootScope);
$rootScope.$apply();
expect(controller.flag).toBe(true);
dealoc(element);
});
});
});

describe('ngIf animations', function () {
var body, element, $rootElement;

Expand Down
Loading

0 comments on commit 9fc52c1

Please sign in to comment.