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 (angular@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: ``` <div data-fancy-directive-start>{{start}}</div> <p>Grouped content</p> <div data-fancy-directive-end>{{end}}</div> .directive('fancyDirective', function() { return { link: angular.noop }; }) ``` After: ``` <div data-fancy-directive-start>{{start}}</div> <p>Grouped content</p> <div data-fancy-directive-end>{{end}}</div> .directive('fancyDirective', function() { return { multiElement: true, // Explicitly mark as a multi-element directive. link: angular.noop }; }) ``` Closes angular#5372 Closes angular#6574 Closes angular#5370 Closes angular#8044 Closes angular#7336
ckknight · Jul 16, 2014 · ec71e5e · ec71e5e
1 parent 3085d01
commit ec71e5e
Show file tree

Hide file tree

Showing 7 changed files with 169 additions and 27 deletions.
diff --git 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<ii; i++) {
+          directive = directives[i];
+          if (directive.multiElement) {
+            return true;
+          }
+        }
+      }
+      return false;
+    }
+
     /**
      * When the element is replaced with HTML template then the new attributes
      * on the template need to be merged with the existing attributes in the DOM.

diff --git a/src/ng/directive/ngIf.js b/src/ng/directive/ngIf.js
@@ -78,6 +78,7 @@
  */
 var ngIfDirective = ['$animate', function($animate) {
   return {
+    multiElement: true,
     transclude: 'element',
     priority: 600,
     terminal: true,

diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js
@@ -202,6 +202,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
   var NG_REMOVED = '$$NG_REMOVED';
   var ngRepeatMinErr = minErr('ngRepeat');
   return {
+    multiElement: true,
     transclude: 'element',
     priority: 1000,
     terminal: true,

diff --git a/src/ng/directive/ngShowHide.js b/src/ng/directive/ngShowHide.js
@@ -156,10 +156,13 @@
   </example>
  */
 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) {
   </example>
  */
 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
@@ -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
@@ -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('<span data-dash-starter data-on-dash-start="starter"></span>')($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('<span data-dash-ender data-on-dash-end="ender"></span>')($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(
-          '<div></div>' +
-              '<div ng-repeat-start="i in [1,2]">{{i}}A</div>' +
-              '<span ng-bind-start="\'.\'"></span>' +
-              '<span ng-bind-end></span>' +
-              '<div ng-repeat-end>{{i}}B;</div>' +
-          '<div></div>')($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(
+            '<div></div>' +
+                '<div ng-repeat-start="i in [1,2]">{{i}}A</div>' +
+                '<span ng-multi-bind-start="\'.\'"></span>' +
+                '<span ng-multi-bind-end></span>' +
+                '<div ng-repeat-end>{{i}}B;</div>' +
+            '<div></div>')($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
@@ -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(
+      '<ul ng-switch="select">' +
+        '<li ng-switch-when-start="1">A</li>' +
+        '<li>B</li>' +
+        '<li ng-switch-when-end>C</li>' +
+        '<li ng-switch-when-start="2">D</li>' +
+        '<li>E</li>' +
+        '<li ng-switch-when-end>F</li>' +
+      '</ul>')($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(
       '<ng:switch on="select">' +
@@ -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(
+      '<ul ng-switch="select">' +
+        '<li ng-switch-when-start="1">A</li>' +
+        '<li>B</li>' +
+        '<li ng-switch-when-end>C</li>' +
+        '<li ng-switch-default-start>D</li>' +
+        '<li>E</li>' +
+        '<li ng-switch-default-end>F</li>' +
+      '</ul>')($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(
       '<ul ng-switch="select">' +