From ec71e5eb1500c4f9cd0908c65b58c90554272213 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Wed, 11 Dec 2013 15:54:37 -0500 Subject: [PATCH] feat($compile): explicitly request multi-element directive behaviour Directives which expect to make use of the multi-element grouping feature introduced in 1.1.6 (https://github.com/angular/angular.js/commit/e46100f7) must now add the property multiElement to their definition object, with a truthy value. This enables the use of directive attributes ending with the words '-start' and '-end' for single-element directives. BREAKING CHANGE: Directives which previously depended on the implicit grouping between directive-start and directive-end attributes must be refactored in order to see this same behaviour. Before: ```
{{start}}

Grouped content

{{end}}
.directive('fancyDirective', function() { return { link: angular.noop }; }) ``` After: ```
{{start}}

Grouped content

{{end}}
.directive('fancyDirective', function() { return { multiElement: true, // Explicitly mark as a multi-element directive. link: angular.noop }; }) ``` Closes #5372 Closes #6574 Closes #5370 Closes #8044 Closes #7336 --- src/ng/compile.js | 38 +++++++++++++-- src/ng/directive/ngIf.js | 1 + src/ng/directive/ngRepeat.js | 1 + src/ng/directive/ngShowHide.js | 22 +++++---- src/ng/directive/ngSwitch.js | 2 + test/ng/compileSpec.js | 80 +++++++++++++++++++++++++------ test/ng/directive/ngSwitchSpec.js | 52 ++++++++++++++++++++ 7 files changed, 169 insertions(+), 27 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index eb7b84018217..93803a8dd1d1 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -111,6 +111,13 @@ * The directive definition object provides instructions to the {@link ng.$compile * compiler}. The attributes are: * + * ### `multiElement` + * When this property is set to true, the HTML compiler will collect DOM nodes between + * nodes with the attributes `directive-name-start` and `directive-name-end`, and group them + * together as the directive elements. It is recomended that this feature be used on directives + * which are not strictly behavioural (such as {@link api/ng.directive:ngClick ngClick}), and which + * do not manipulate or replace child nodes (such as {@link api/ng.directive:ngInclude ngInclude}). + * * #### `priority` * When there are multiple directives defined on a single DOM element, sometimes it * is necessary to specify the order in which the directives are applied. The `priority` is used @@ -1047,10 +1054,12 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } var directiveNName = ngAttrName.replace(/(Start|End)$/, ''); - if (ngAttrName === directiveNName + 'Start') { - attrStartName = name; - attrEndName = name.substr(0, name.length - 5) + 'end'; - name = name.substr(0, name.length - 6); + if (directiveIsMultiElement(directiveNName)) { + if (ngAttrName === directiveNName + 'Start') { + attrStartName = name; + attrEndName = name.substr(0, name.length - 5) + 'end'; + name = name.substr(0, name.length - 6); + } } nName = directiveNormalize(name.toLowerCase()); @@ -1663,6 +1672,27 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } + /** + * looks up the directive and returns true if it is a multi-element directive, + * and therefore requires DOM nodes between -start and -end markers to be grouped + * together. + * + * @param {string} name name of the directive to look up. + * @returns true if directive was registered as multi-element. + */ + function directiveIsMultiElement(name) { + if (hasDirectives.hasOwnProperty(name)) { + for(var directive, directives = $injector.get(name + Suffix), + i = 0, ii = directives.length; i */ var ngShowDirective = ['$animate', function($animate) { - return function(scope, element, attr) { - scope.$watch(attr.ngShow, function ngShowWatchAction(value){ - $animate[value ? 'removeClass' : 'addClass'](element, 'ng-hide'); - }); + return { + multiElement: true, + link: function(scope, element, attr) { + scope.$watch(attr.ngShow, function ngShowWatchAction(value){ + $animate[value ? 'removeClass' : 'addClass'](element, 'ng-hide'); + }); + } }; }]; @@ -307,9 +310,12 @@ var ngShowDirective = ['$animate', function($animate) { */ var ngHideDirective = ['$animate', function($animate) { - return function(scope, element, attr) { - scope.$watch(attr.ngHide, function ngHideWatchAction(value){ - $animate[value ? 'addClass' : 'removeClass'](element, 'ng-hide'); - }); + return { + multiElement: true, + link: function(scope, element, attr) { + scope.$watch(attr.ngHide, function ngHideWatchAction(value){ + $animate[value ? 'addClass' : 'removeClass'](element, 'ng-hide'); + }); + } }; }]; diff --git a/src/ng/directive/ngSwitch.js b/src/ng/directive/ngSwitch.js index b4dc2c9395be..9abe61ca860f 100644 --- a/src/ng/directive/ngSwitch.js +++ b/src/ng/directive/ngSwitch.js @@ -185,6 +185,7 @@ var ngSwitchWhenDirective = ngDirective({ transclude: 'element', priority: 800, require: '^ngSwitch', + multiElement: true, link: function(scope, element, attrs, ctrl, $transclude) { ctrl.cases['!' + attrs.ngSwitchWhen] = (ctrl.cases['!' + attrs.ngSwitchWhen] || []); ctrl.cases['!' + attrs.ngSwitchWhen].push({ transclude: $transclude, element: element }); @@ -195,6 +196,7 @@ var ngSwitchDefaultDirective = ngDirective({ transclude: 'element', priority: 800, require: '^ngSwitch', + multiElement: true, link: function(scope, element, attr, ctrl, $transclude) { ctrl.cases['?'] = (ctrl.cases['?'] || []); ctrl.cases['?'].push({ transclude: $transclude, element: element }); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index d22b52f075be..aedff6e4d795 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -5487,6 +5487,40 @@ describe('$compile', function() { expect(element.attr('dash-test4')).toBe('JamieMason'); })); + it('should keep attributes ending with -start single-element directives', function() { + module(function($compileProvider) { + $compileProvider.directive('dashStarter', function(log) { + return { + link: function(scope, element, attrs) { + log(attrs.onDashStart); + } + }; + }); + }); + inject(function($compile, $rootScope, log) { + $compile('')($rootScope); + $rootScope.$digest(); + expect(log).toEqual('starter'); + }); + }); + + + it('should keep attributes ending with -end single-element directives', function() { + module(function($compileProvider) { + $compileProvider.directive('dashEnder', function(log) { + return { + link: function(scope, element, attrs) { + log(attrs.onDashEnd); + } + }; + }); + }); + inject(function($compile, $rootScope, log) { + $compile('')($rootScope); + $rootScope.$digest(); + expect(log).toEqual('ender'); + }); + }); }); }); @@ -5545,19 +5579,29 @@ describe('$compile', function() { })); - it('should group on nested groups', inject(function($compile, $rootScope) { - $rootScope.show = false; - element = $compile( - '
' + - '
{{i}}A
' + - '' + - '' + - '
{{i}}B;
' + - '
')($rootScope); - $rootScope.$digest(); - element = jqLite(element[0].parentNode.childNodes); // reset because repeater is top level. - expect(element.text()).toEqual('1A..1B;2A..2B;'); - })); + it('should group on nested groups', function() { + module(function($compileProvider) { + $compileProvider.directive("ngMultiBind", valueFn({ + multiElement: true, + link: function(scope, element, attr) { + element.text(scope.$eval(attr.ngMultiBind)); + } + })); + }); + inject(function($compile, $rootScope) { + $rootScope.show = false; + element = $compile( + '
' + + '
{{i}}A
' + + '' + + '' + + '
{{i}}B;
' + + '
')($rootScope); + $rootScope.$digest(); + element = jqLite(element[0].parentNode.childNodes); // reset because repeater is top level. + expect(element.text()).toEqual('1A..1B;2A..2B;'); + }); + }); it('should group on nested groups of same directive', inject(function($compile, $rootScope) { @@ -5749,6 +5793,7 @@ describe('$compile', function() { module(function($compileProvider) { $compileProvider.directive('foo', function() { return { + multiElement: true }; }); }); @@ -5766,12 +5811,16 @@ describe('$compile', function() { it('should correctly collect ranges on multiple directives on a single element', function () { module(function($compileProvider) { $compileProvider.directive('emptyDirective', function() { - return function (scope, element) { - element.data('x', 'abc'); + return { + multiElement: true, + link: function (scope, element) { + element.data('x', 'abc'); + } }; }); $compileProvider.directive('rangeDirective', function() { return { + multiElement: true, link: function (scope) { scope.x = 'X'; scope.y = 'Y'; @@ -5799,6 +5848,7 @@ describe('$compile', function() { module(function($compileProvider) { $compileProvider.directive('foo', function() { return { + multiElement: true }; }); }); diff --git a/test/ng/directive/ngSwitchSpec.js b/test/ng/directive/ngSwitchSpec.js index 9044147e139c..48e27ae9f50a 100644 --- a/test/ng/directive/ngSwitchSpec.js +++ b/test/ng/directive/ngSwitchSpec.js @@ -66,6 +66,32 @@ describe('ngSwitch', function() { })); + it('should show all elements between start and end markers that match the current value', + inject(function($rootScope, $compile) { + element = $compile( + '')($rootScope); + + $rootScope.$apply('select = "1"'); + expect(element.find('li').length).toBe(3); + expect(element.find('li').eq(0).text()).toBe('A'); + expect(element.find('li').eq(1).text()).toBe('B'); + expect(element.find('li').eq(2).text()).toBe('C'); + + $rootScope.$apply('select = "2"'); + expect(element.find('li').length).toBe(3); + expect(element.find('li').eq(0).text()).toBe('D'); + expect(element.find('li').eq(1).text()).toBe('E'); + expect(element.find('li').eq(2).text()).toBe('F'); + })); + + it('should switch on switch-when-default', inject(function($rootScope, $compile) { element = $compile( '' + @@ -80,6 +106,32 @@ describe('ngSwitch', function() { })); + it('should show all default elements between start and end markers when no match', + inject(function($rootScope, $compile) { + element = $compile( + '
    ' + + '
  • A
    • ' + + '
  • B
    • ' + + '
  • C
    • ' + + '
  • D
    • ' + + '
  • E
    • ' + + '
  • F
    • ' + + '
')($rootScope); + + $rootScope.$apply('select = "1"'); + expect(element.find('li').length).toBe(3); + expect(element.find('li').eq(0).text()).toBe('A'); + expect(element.find('li').eq(1).text()).toBe('B'); + expect(element.find('li').eq(2).text()).toBe('C'); + + $rootScope.$apply('select = "2"'); + expect(element.find('li').length).toBe(3); + expect(element.find('li').eq(0).text()).toBe('D'); + expect(element.find('li').eq(1).text()).toBe('E'); + expect(element.find('li').eq(2).text()).toBe('F'); + })); + + it('should show all switch-when-default', inject(function($rootScope, $compile) { element = $compile( '
    ' +