Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($compile): support transcluding multi-element directives #15555

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -2575,7 +2575,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

// We have transclusion slots,
// collect them up, compile them and store their transclusion functions
$template = [];
$template = window.document.createDocumentFragment();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this use $window or $document?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other places inside this file use window directly (e.g. here, here and here). Normally, I would prefer $window (or $document). But it doesn't bother me too much here because:

  1. This file is dealing with pretty "low-level" stuff.
  2. Switching to $window would be a breaking change (since every $window or $document mock should then provide the necessary APIs).


var slotMap = createMap();
var filledSlots = createMap();
Expand Down Expand Up @@ -2603,10 +2603,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
var slotName = slotMap[directiveNormalize(nodeName_(node))];
if (slotName) {
filledSlots[slotName] = true;
slots[slotName] = slots[slotName] || [];
slots[slotName].push(node);
slots[slotName] = slots[slotName] || window.document.createDocumentFragment();
slots[slotName].appendChild(node);
} else {
$template.push(node);
$template.appendChild(node);
}
});

Expand All @@ -2620,9 +2620,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
for (var slotName in slots) {
if (slots[slotName]) {
// Only define a transclusion function if the slot was filled
slots[slotName] = compilationGenerator(mightHaveMultipleTransclusionError, slots[slotName], transcludeFn);
slots[slotName] = compilationGenerator(mightHaveMultipleTransclusionError, slots[slotName].childNodes, transcludeFn);
}
}

$template = $template.childNodes;
}

$compileNode.empty(); // clear contents
Expand Down
44 changes: 44 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8843,6 +8843,50 @@ describe('$compile', function() {
});
});


it('should correctly handle multi-element directives', function() {
module(function() {
directive('foo', valueFn({
template: '[<div ng-transclude></div>]',
transclude: true
}));
directive('bar', valueFn({
template: '[<div ng-transclude="header"></div>|<div ng-transclude="footer"></div>]',
transclude: {
header: 'header',
footer: 'footer'
}
}));
});

inject(function($compile, $rootScope) {
var tmplWithFoo =
'<foo>' +
'<div ng-if-start="true">Hello, </div>' +
'<div ng-if-end>world!</div>' +
'</foo>';
var tmplWithBar =
'<bar>' +
'<header ng-if-start="true">This is a </header>' +
'<header ng-if-end>header!</header>' +
'<footer ng-if-start="true">This is a </footer>' +
'<footer ng-if-end>footer!</footer>' +
'</bar>';

var elem1 = $compile(tmplWithFoo)($rootScope);
var elem2 = $compile(tmplWithBar)($rootScope);

$rootScope.$digest();

expect(elem1.text()).toBe('[Hello, world!]');
expect(elem2.text()).toBe('[This is a header!|This is a footer!]');

dealoc(elem1);
dealoc(elem2);
});
});


//see issue https://github.com/angular/angular.js/issues/12936
it('should use the proper scope when it is on the root element of a replaced directive template', function() {
module(function() {
Expand Down