From 90f87072e83234ae366cfeb3c281503c31dad738 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Thu, 14 Nov 2013 13:50:36 -0800 Subject: [PATCH] fix($compile): accessing controllers of transcluded directives from children 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 #4935. --- src/ng/compile.js | 141 ++++++++++++------- src/ng/directive/ngIf.js | 6 +- src/ng/directive/ngInclude.js | 6 +- src/ng/directive/ngRepeat.js | 6 +- src/ng/directive/ngSwitch.js | 16 +-- src/ngRoute/directive/ngView.js | 6 +- test/ng/compileSpec.js | 203 ++++++++++++++++++++++++++- test/ng/directive/ngIfSpec.js | 28 ++++ test/ng/directive/ngIncludeSpec.js | 30 ++++ test/ng/directive/ngRepeatSpec.js | 27 ++++ test/ngRoute/directive/ngViewSpec.js | 38 +++++ 11 files changed, 434 insertions(+), 73 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 039c211c17b9..4d83f379d22f 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -178,8 +178,9 @@ * * `$scope` - Current scope associated with the element * * `$element` - Current element * * `$attrs` - Current attributes object for the element - * * `$transclude` - A transclude linking function pre-bound to the correct transclusion scope: - * `function(cloneLinkingFn)`. + * * `$transclude` - A transclude linking function pre-bound to the correct transclusion scope. + * The scope can be overridden by an optional first argument. + * `function([scope], cloneLinkingFn)`. * * * #### `require` @@ -272,7 +273,7 @@ * * `tAttrs` - template attributes - Normalized list of attributes declared on this element shared * between all directive compile functions. * - * * `transclude` - A transclude linking function: `function(scope, cloneLinkingFn)`. + * * `transclude` - [*DEPRECATED*!] A transclude linking function: `function(scope, cloneLinkingFn)` * *
* **Note:** The template instance and the link instance may be different objects if the template has @@ -281,6 +282,12 @@ * should be done in a linking function rather than in a compile function. *
* + *
+ * **Note:** The `transclude` function that is passed to the compile function is deperecated, as it + * e.g. does not know about the right outer scope. Please use the transclude function that is passed + * to the link function instead. + *
+ * A compile function can have a return value which can be either a function or an object. * * * returning a (post-link) function - is equivalent to registering the linking function via the @@ -295,7 +302,7 @@ * This property is used only if the `compile` property is not defined. * * 
- *   function link(scope, iElement, iAttrs, controller) { ... }
+ *   function link(scope, iElement, iAttrs, controller, transcludeFn) { ... }
  *
* * The link function is responsible for registering DOM listeners as well as updating the DOM. It is @@ -316,6 +323,10 @@ * element defines a controller. The controller is shared among all the directives, which allows * the directives to use the controllers as a communication channel. * + * * `transcludeFn` - A transclude linking function pre-bound to the correct transclusion scope. + * The scope can be overridden by an optional first argument. This is the same as the `$transclude` + * parameter of directive controllers. + * `function([scope], cloneLinkingFn)`. * * * #### Pre-linking function @@ -821,7 +832,7 @@ function $CompileProvider($provide) { var compositeLinkFn = compileNodes($compileNodes, transcludeFn, $compileNodes, maxPriority, ignoreDirective, previousCompileContext); - return function publicLinkFn(scope, cloneConnectFn){ + return function publicLinkFn(scope, cloneConnectFn, transcludeControllers){ assertArg(scope, 'scope'); // important!!: we must call our jqLite.clone() since the jQuery one is trying to be smart // and sometimes changes the structure of the DOM. @@ -829,6 +840,10 @@ function $CompileProvider($provide) { ? JQLitePrototype.clone.call($compileNodes) // IMPORTANT!!! : $compileNodes; + forEach(transcludeControllers, function(instance, name) { + $linkNode.data('$' + name + 'Controller', instance); + }); + // Attach scope only to non-text nodes. for(var i = 0, ii = $linkNode.length; i')($rootScope); + var children = element.children(), i; + expect(transcludeCtrl).toBeDefined(); + + expect(element.data('$transcludeController')).toBe(transcludeCtrl); + for (i=0; i')($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('
{{$id}}
')($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('
')($rootScope); + expect(capturedChildCtrl).toBeTruthy(); + }); + + }); }); @@ -3471,7 +3578,6 @@ describe('$compile', function() { }); }); - it('should only allow one element transclusion per element', function() { module(function() { directive('first', valueFn({ @@ -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('
')($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
')($rootScope); + var children = element.children(), i; + for (i=0; i
')($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('
'); + element = $compile('
')($rootScope); + $httpBackend.flush(); + expect(transclude).toBeDefined(); + }); + }); + + }); it('should safely create transclude comment node and not break with "-->"', inject(function($rootScope) { diff --git a/test/ng/directive/ngIfSpec.js b/test/ng/directive/ngIfSpec.js index 3173f476c39b..427bfd597917 100755 --- a/test/ng/directive/ngIfSpec.js +++ b/test/ng/directive/ngIfSpec.js @@ -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: '
', + 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('
')($rootScope); + $rootScope.$apply(); + expect(controller.flag).toBe(true); + dealoc(element); + }); + }); +}); + describe('ngIf animations', function () { var body, element, $rootElement; diff --git a/test/ng/directive/ngIncludeSpec.js b/test/ng/directive/ngIncludeSpec.js index beb29da759a8..aba71e44ce9c 100644 --- a/test/ng/directive/ngIncludeSpec.js +++ b/test/ng/directive/ngIncludeSpec.js @@ -439,6 +439,36 @@ describe('ngInclude', function() { }); }); +describe('ngInclude 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: '
', + replace: true, + controller: function() { + this.flag = true; + } + })); + directive('test', valueFn({ + require: '^template', + link: function(scope, el, attr, ctrl) { + controller = ctrl; + } + })); + }); + inject(function($compile, $rootScope, $httpBackend) { + $httpBackend.expectGET('include.html').respond('
'); + var element = $compile('
')($rootScope); + $rootScope.$apply(); + $httpBackend.flush(); + expect(controller.flag).toBe(true); + dealoc(element); + }); + }); +}); + describe('ngInclude animations', function() { var body, element, $rootElement; diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 9dde36e7cc24..6584f31addd8 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -1058,6 +1058,33 @@ describe('ngRepeat', function() { }); }); +describe('ngRepeat 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: '
', + 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('
')($rootScope); + $rootScope.$apply(); + expect(controller.flag).toBe(true); + dealoc(element); + }); + }); +}); describe('ngRepeat animations', function() { var body, element, $rootElement; diff --git a/test/ngRoute/directive/ngViewSpec.js b/test/ngRoute/directive/ngViewSpec.js index 1df19d6a1cb2..e96da022aa80 100644 --- a/test/ngRoute/directive/ngViewSpec.js +++ b/test/ngRoute/directive/ngViewSpec.js @@ -514,6 +514,44 @@ describe('ngView', function() { }); }); +describe('ngView and transcludes', function() { + it('should allow access to directive controller from children when used in a replace template', function() { + var controller; + module('ngRoute'); + module(function($compileProvider, $routeProvider) { + $routeProvider.when('/view', {templateUrl: 'view.html'}); + var directive = $compileProvider.directive; + directive('template', function() { + return { + template: '
', + replace: true, + controller: function() { + this.flag = true; + } + }; + }); + + directive('test', function() { + return { + require: '^template', + link: function(scope, el, attr, ctrl) { + controller = ctrl; + } + }; + }); + }); + inject(function($compile, $rootScope, $httpBackend, $location) { + $httpBackend.expectGET('view.html').respond('
'); + var element = $compile('
')($rootScope); + $location.url('/view'); + $rootScope.$apply(); + $httpBackend.flush(); + expect(controller.flag).toBe(true); + dealoc(element); + }); + }); +}); + describe('ngView animations', function() { var body, element, $rootElement;