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

fix(ngIf): ngIf removes elements dynamically added to it #4726

Closed
wants to merge 3 commits into from
Closed
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
1 change: 1 addition & 0 deletions src/.jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
"assertArgFn": false,
"assertNotHasOwnProperty": false,
"getter": false,
"getBlockElements": false,

/* AngularPublic.js */
"version": false,
Expand Down
25 changes: 24 additions & 1 deletion src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@
-assertArg,
-assertArgFn,
-assertNotHasOwnProperty,
-getter
-getter,
-getBlockElements

*/

Expand Down Expand Up @@ -1318,3 +1319,25 @@ function getter(obj, path, bindFnToScope) {
}
return obj;
}

/**
* Return the siblings between `startNode` and `endNode`, inclusive
* @param {Object} object with `startNode` and `endNode` properties
* @returns jQlite object containing the elements
*/
function getBlockElements(block) {
if (block.startNode === block.endNode) {
return jqLite(block.startNode);
}

var element = block.startNode;
var elements = [element];

do {
element = element.nextSibling;
if (!element) break;
elements.push(element);
} while (element !== block.endNode);

return jqLite(elements);
}
7 changes: 4 additions & 3 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1173,9 +1173,10 @@ function $CompileProvider($provide) {
}

if (directiveValue = directive.transclude) {
// Special case ngRepeat so that we don't complain about duplicate transclusion, ngRepeat
// knows how to handle this on its own.
if (directiveName !== 'ngRepeat') {
// Special case ngIf and ngRepeat so that we don't complain about duplicate transclusion.
// This option should only be used by directives that know how to how to safely handle element transclusion,
// where the transcluded nodes are added or replaced after linking.
if (!directive.$$tlb) {
assertNoDuplicate('transclusion', transcludeDirective, directive, $compileNode);
transcludeDirective = directive;
}
Expand Down
18 changes: 10 additions & 8 deletions src/ng/directive/ngIf.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,24 @@ var ngIfDirective = ['$animate', function($animate) {
priority: 600,
terminal: true,
restrict: 'A',
$$tlb: true,
compile: function (element, attr, transclude) {
return function ($scope, $element, $attr) {
var childElement, childScope;
var block = {}, childScope;
$scope.$watch($attr.ngIf, function ngIfWatchAction(value) {
if (childElement) {
$animate.leave(childElement);
childElement = undefined;
if (block.startNode) {
$animate.leave(getBlockElements(block));
block = {};
}
if (childScope) {
childScope.$destroy();
childScope = undefined;
if (block.startNode) {
getBlockElements(block).$destroy();
block = {};
}
if (toBoolean(value)) {
childScope = $scope.$new();
transclude(childScope, function (clone) {
childElement = clone;
block.startNode = clone[0];
block.endNode = clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ');
$animate.enter(clone, $element.parent(), $element);
});
}
Expand Down
18 changes: 1 addition & 17 deletions src/ng/directive/ngRepeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
transclude: 'element',
priority: 1000,
terminal: true,
$$tlb: true,
compile: function(element, attr, linker) {
return function($scope, $element, $attr){
var expression = $attr.ngRepeat;
Expand Down Expand Up @@ -393,22 +394,5 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
};
}
};

function getBlockElements(block) {
if (block.startNode === block.endNode) {
return jqLite(block.startNode);
}

var element = block.startNode;
var elements = [element];

do {
element = element.nextSibling;
if (!element) break;
elements.push(element);
} while (element !== block.endNode);

return jqLite(elements);
}
}];

37 changes: 37 additions & 0 deletions test/ng/directive/ngIfSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,43 @@ describe('ngIf', function () {
expect(element.children().length).toBe(9);
});

it('should play nice with ngInclude on the same element', inject(function($templateCache) {
$templateCache.put('test.html', [200, '{{value}}', {}]);

$scope.value = 'first';
element.append($compile(
'<div ng-if="value==\'first\'" ng-include="\'test.html\'"></div>'
)($scope));
$scope.$apply();
expect(element.text()).toBe('first');

$scope.value = 'later';
$scope.$apply();
expect(element.text()).toBe('');
}));

it('should work with multiple elements', function() {
$scope.show = true;
$scope.things = [1, 2, 3];
element.append($compile(
'<div>before;</div>' +
'<div ng-if-start="show">start;</div>' +
'<div ng-repeat="thing in things">{{thing}};</div>' +
'<div ng-if-end>end;</div>' +
'<div>after;</div>'
)($scope));
$scope.$apply();
expect(element.text()).toBe('before;start;1;2;3;end;after;');

$scope.things.push(4);
$scope.$apply();
expect(element.text()).toBe('before;start;1;2;3;4;end;after;');

$scope.show = false;
$scope.$apply();
expect(element.text()).toBe('before;after;');
});

it('should restore the element to its compiled state', function() {
$scope.value = true;
makeIf('value');
Expand Down