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

Commit e19067c

Browse files
committed
fix(ngIf): ngIf removes elements dynamically added to it
When using ngIf with ngInclude on the same element, ngIf previously did not remove elements added by ngInclude. Similarly, when using ngIfStart/End, ngIf will miss elements added between the start/end markers added after ngIf is linked. This commit changes the behavior of ngIf to add a comment node at the end of its elements such that elements between the starting comment and this ending comment are removed when ngIf's predicate does not hold.
1 parent 9d0a697 commit e19067c

File tree

3 files changed

+65
-9
lines changed

3 files changed

+65
-9
lines changed

src/ng/compile.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1175,7 +1175,7 @@ function $CompileProvider($provide) {
11751175
if (directiveValue = directive.transclude) {
11761176
// Special case ngRepeat so that we don't complain about duplicate transclusion, ngRepeat
11771177
// knows how to handle this on its own.
1178-
if (directiveName !== 'ngRepeat') {
1178+
if (directiveName !== 'ngRepeat' && directiveName !== 'ngIf') {
11791179
assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
11801180
transcludeDirective = directive;
11811181
}

src/ng/directive/ngIf.js

+27-8
Original file line numberDiff line numberDiff line change
@@ -86,25 +86,44 @@ var ngIfDirective = ['$animate', function($animate) {
8686
restrict: 'A',
8787
compile: function (element, attr, transclude) {
8888
return function ($scope, $element, $attr) {
89-
var childElement, childScope;
89+
var block = {}, childScope;
9090
$scope.$watch($attr.ngIf, function ngIfWatchAction(value) {
91-
if (childElement) {
92-
$animate.leave(childElement);
93-
childElement = undefined;
91+
if (block.startNode) {
92+
$animate.leave(getBlockElements(block));
93+
block = {};
9494
}
95-
if (childScope) {
96-
childScope.$destroy();
97-
childScope = undefined;
95+
if (block.startNode) {
96+
getBlockElements(block).$destroy();
97+
block = {};
9898
}
9999
if (toBoolean(value)) {
100100
childScope = $scope.$new();
101101
transclude(childScope, function (clone) {
102-
childElement = clone;
102+
block.startNode = clone[0];
103+
block.endNode = clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ');
103104
$animate.enter(clone, $element.parent(), $element);
104105
});
105106
}
106107
});
107108
};
108109
}
109110
};
111+
112+
// TODO(bford): this helper was copypasta'd from ngRepeat
113+
function getBlockElements(block) {
114+
if (block.startNode === block.endNode) {
115+
return jqLite(block.startNode);
116+
}
117+
118+
var element = block.startNode;
119+
var elements = [element];
120+
121+
do {
122+
element = element.nextSibling;
123+
if (!element) break;
124+
elements.push(element);
125+
} while (element !== block.endNode);
126+
127+
return jqLite(elements);
128+
}
110129
}];

test/ng/directive/ngIfSpec.js

+37
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,43 @@ describe('ngIf', function () {
6060
expect(element.children().length).toBe(9);
6161
});
6262

63+
it('should play nice with ngInclude on the same element', inject(function($templateCache) {
64+
$templateCache.put('test.html', [200, '{{value}}', {}]);
65+
66+
$scope.value = 'first';
67+
element.append($compile(
68+
'<div ng-if="value==\'first\'" ng-include="\'test.html\'"></div>'
69+
)($scope));
70+
$scope.$apply();
71+
expect(element.text()).toBe('first');
72+
73+
$scope.value = 'later';
74+
$scope.$apply();
75+
expect(element.text()).toBe('');
76+
}));
77+
78+
it('should work with multiple elements', function() {
79+
$scope.show = true;
80+
$scope.things = [1, 2, 3];
81+
element.append($compile(
82+
'<div>before;</div>' +
83+
'<div ng-if-start="show">start;</div>' +
84+
'<div ng-repeat="thing in things">{{thing}};</div>' +
85+
'<div ng-if-end>end;</div>' +
86+
'<div>after;</div>'
87+
)($scope));
88+
$scope.$apply();
89+
expect(element.text()).toBe('before;start;1;2;3;end;after;');
90+
91+
$scope.things.push(4);
92+
$scope.$apply();
93+
expect(element.text()).toBe('before;start;1;2;3;4;end;after;');
94+
95+
$scope.show = false;
96+
$scope.$apply();
97+
expect(element.text()).toBe('before;after;');
98+
});
99+
63100
it('should restore the element to its compiled state', function() {
64101
$scope.value = true;
65102
makeIf('value');

0 commit comments

Comments
 (0)