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

Commit

Permalink
fix(ngIf): don't create multiple elements when changing from a truthy…
Browse files Browse the repository at this point in the history
… to another thruthy value.

Fixes #4852.
  • Loading branch information
tbosch committed Nov 12, 2013
1 parent 9577702 commit 4612705
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
22 changes: 11 additions & 11 deletions src/ng/directive/ngIf.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
}
/*
The transition styles can also be placed on the CSS base class above
The transition styles can also be placed on the CSS base class above
*/
.animate-if.ng-enter, .animate-if.ng-leave {
-webkit-transition:all cubic-bezier(0.250, 0.460, 0.450, 0.940) 0.5s;
Expand Down Expand Up @@ -92,16 +92,16 @@ var ngIfDirective = ['$animate', function($animate) {
$scope.$watch($attr.ngIf, function ngIfWatchAction(value) {

if (toBoolean(value)) {

childScope = $scope.$new();
transclude(childScope, function (clone) {
block = {
startNode: clone[0],
endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ')
};
$animate.enter(clone, $element.parent(), $element);
});

if (!childScope) {
childScope = $scope.$new();
transclude(childScope, function (clone) {
block = {
startNode: clone[0],
endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ')
};
$animate.enter(clone, $element.parent(), $element);
});
}
} else {

if (childScope) {
Expand Down
16 changes: 16 additions & 0 deletions test/ng/directive/ngIfSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ describe('ngIf', function () {
expect(element.children().length).toBe(1);
});

it('should not add the element twice if the condition goes from true to true', function () {
$scope.hello = 'true1';
makeIf('hello');
expect(element.children().length).toBe(1);
$scope.$apply('hello = "true2"');
expect(element.children().length).toBe(1);
});

it('should not recreate the element if the condition goes from true to true', function () {
$scope.hello = 'true1';
makeIf('hello');
element.children().data('flag', true);
$scope.$apply('hello = "true2"');
expect(element.children().data('flag')).toBe(true);
});

it('should create then remove the element if condition changes', function () {
$scope.hello = true;
makeIf('hello');
Expand Down

5 comments on commit 4612705

@ulfryk
Copy link

@ulfryk ulfryk commented on 4612705 Nov 12, 2013

Choose a reason for hiding this comment

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

Hmm.. still it does not work for me...

@tbosch
Copy link
Contributor Author

@tbosch tbosch commented on 4612705 Nov 12, 2013

Choose a reason for hiding this comment

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

Hi,
I updated the original plnkr.co to use the current master (http://code.angularjs.org/snapshot/angular.js) and it does what it should do: http://plnkr.co/edit/2Adgfo09S65Zw3cpcLbG?p=preview.

Could you explain in more detail what is not working?
Thanks,
T

@ulfryk
Copy link

@ulfryk ulfryk commented on 4612705 Nov 12, 2013

Choose a reason for hiding this comment

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

Hi,

this example on plnkr works well, and also your change is quite logic...
Maybe I made some mistake, I'll check tomorrow as soon as I get to office :)

@ulfryk
Copy link

@ulfryk ulfryk commented on 4612705 Nov 13, 2013

Choose a reason for hiding this comment

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

You got right - everything is well and working.
I tried to override ngIf directive in 'ng' module, but instead it was
added so 2 similar directives worked in the same time (so one
was working good but other one was appending more copies of
element).

Sorry for disturbing..

@tbosch
Copy link
Contributor Author

@tbosch tbosch commented on 4612705 Nov 13, 2013 via email

Choose a reason for hiding this comment

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

Please sign in to comment.