Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
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 (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 #5372
Closes #6574
Closes #5370
Closes #8044
Closes #7336
  • Loading branch information
caitp committed Jul 16, 2014
1 parent 048a5f1 commit e8066c4
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 27 deletions.
38 changes: 34 additions & 4 deletions src/ng/compile.js
Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/ng/directive/ngIf.js
Expand Up @@ -78,6 +78,7 @@
*/
var ngIfDirective = ['$animate', function($animate) {
return {
multiElement: true,
transclude: 'element',
priority: 600,
terminal: true,
Expand Down
1 change: 1 addition & 0 deletions src/ng/directive/ngRepeat.js
Expand Up @@ -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,
Expand Down
22 changes: 14 additions & 8 deletions src/ng/directive/ngShowHide.js
Expand Up @@ -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');
});
}
};
}];

Expand Down Expand Up @@ -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');
});
}
};
}];
2 changes: 2 additions & 0 deletions src/ng/directive/ngSwitch.js
Expand Up @@ -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 });
Expand All @@ -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 });
Expand Down
80 changes: 65 additions & 15 deletions test/ng/compileSpec.js
Expand Up @@ -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');
});
});
});

});
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -5749,6 +5793,7 @@ describe('$compile', function() {
module(function($compileProvider) {
$compileProvider.directive('foo', function() {
return {
multiElement: true
};
});
});
Expand All @@ -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';
Expand Down Expand Up @@ -5799,6 +5848,7 @@ describe('$compile', function() {
module(function($compileProvider) {
$compileProvider.directive('foo', function() {
return {
multiElement: true
};
});
});
Expand Down
52 changes: 52 additions & 0 deletions test/ng/directive/ngSwitchSpec.js
Expand Up @@ -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">' +
Expand All @@ -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">' +
Expand Down

0 comments on commit e8066c4

Please sign in to comment.