Skip to content

Commit

Permalink
fix(ingIf/ngShow/ngHide): follow javscript truthy/falsy logic
Browse files Browse the repository at this point in the history
Make ngIf, ngShow and ngHide follow javascript `truthy`/`falsy`
logic and not the custom toBoolean logic

Fixes angular#5414 angular#4277 angular#3969

BREAKING CHANGE: The expressions

* `<div ng-hide="[]">X</div>`
* `<div ng-hide="'f'">X</div>`
* `<div ng-hide="'[]'">X</div>`

used to be evaluated to `false` and the elements were hidden.

The same effect is present for `ng-show` and the elements are now
  visible; and with `ng-if` and the elements are now removed

If you were previously doing `ng-show="exp"` where
  `$scope.exp = 'no' // (or 'n' or 'f')`, then instead write
  `ng-show="exp && exp !== 'no'` (or 'n' or 'f').
  • Loading branch information
lgalfaso committed Jun 5, 2014
1 parent ff03698 commit 615873d
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/ng/directive/ngIf.js
Expand Up @@ -85,9 +85,9 @@ var ngIfDirective = ['$animate', function($animate) {
$$tlb: true,
link: function ($scope, $element, $attr, ctrl, $transclude) {
var block, childScope, previousElements;
$scope.$watch($attr.ngIf, function ngIfWatchAction(value) {
$scope.$watch('!!(' + $attr.ngIf + ')', function ngIfWatchAction(value) {

if (toBoolean(value)) {
if (value) {
if (!childScope) {
$transclude(function (clone, newScope) {
childScope = newScope;
Expand Down
8 changes: 4 additions & 4 deletions src/ng/directive/ngShowHide.js
Expand Up @@ -162,8 +162,8 @@
*/
var ngShowDirective = ['$animate', function($animate) {
return function(scope, element, attr) {
scope.$watch(attr.ngShow, function ngShowWatchAction(value){
$animate[toBoolean(value) ? 'removeClass' : 'addClass'](element, 'ng-hide');
scope.$watch('!!(' + attr.ngShow + ')', function ngShowWatchAction(value){
$animate[value ? 'removeClass' : 'addClass'](element, 'ng-hide');
});
};
}];
Expand Down Expand Up @@ -318,8 +318,8 @@ var ngShowDirective = ['$animate', function($animate) {
*/
var ngHideDirective = ['$animate', function($animate) {
return function(scope, element, attr) {
scope.$watch(attr.ngHide, function ngHideWatchAction(value){
$animate[toBoolean(value) ? 'addClass' : 'removeClass'](element, 'ng-hide');
scope.$watch('!!(' + attr.ngHide + ')', function ngHideWatchAction(value){
$animate[value ? 'addClass' : 'removeClass'](element, 'ng-hide');
});
};
}];
4 changes: 2 additions & 2 deletions test/BinderSpec.js
Expand Up @@ -245,7 +245,7 @@ describe('Binder', function() {

assertHidden(element);

$rootScope.hidden = 'false';
$rootScope.hidden = false;
$rootScope.$apply();

assertVisible(element);
Expand All @@ -264,7 +264,7 @@ describe('Binder', function() {

assertVisible(element);

$rootScope.show = 'false';
$rootScope.show = false;
$rootScope.$apply();

assertHidden(element);
Expand Down
12 changes: 12 additions & 0 deletions test/ng/directive/ngIfSpec.js
Expand Up @@ -136,6 +136,18 @@ describe('ngIf', function () {
expect(element.text()).toBe('before;after;');
});

it('should evaluate using javascript `truthy`/`falsy` logic', function () {
var cases = ['[]', 'f', [], [''], 'false', {}, function() {}, function(f) {}, 0, false, null, undefined, '', NaN];
element.append($compile(
'<div ng-if="value">Lucas</div>'
)($scope));
angular.forEach(cases, function(value) {
$scope.value = value;
$scope.$apply();
expect(element.text()).toBe(value ? 'Lucas' : '');
});
});

it('should restore the element to its compiled state', function() {
$scope.value = true;
makeIf('value');
Expand Down
24 changes: 24 additions & 0 deletions test/ng/directive/ngShowHideSpec.js
Expand Up @@ -38,6 +38,18 @@ describe('ngShow / ngHide', function() {
$rootScope.$digest();
expect(element).toBeShown();
}));


it('should follow javascript `truthy`/`falsy` logic', inject(function($rootScope, $compile) {
var cases = ['[]', 'f', [], [''], 'false', {}, function() {}, function(f) {}, 0, false, null, undefined, '', NaN];
element = jqLite('<div ng-show="exp"></div>');
element = $compile(element)($rootScope);
angular.forEach(cases, function(value) {
$rootScope.exp = value;
$rootScope.$digest();
expect(element)[value ? 'toBeShown' : 'toBeHidden']();
});
}));
});

describe('ngHide', function() {
Expand All @@ -49,6 +61,18 @@ describe('ngShow / ngHide', function() {
$rootScope.$digest();
expect(element).toBeHidden();
}));


it('should follow javascript `truthy`/`falsy` logic', inject(function($rootScope, $compile) {
var cases = ['[]', 'f', [], [''], 'false', {}, function() {}, function(f) {}, 0, false, null, undefined, '', NaN];
element = jqLite('<div ng-hide="exp"></div>');
element = $compile(element)($rootScope);
angular.forEach(cases, function(value) {
$rootScope.exp = value;
$rootScope.$digest();
expect(element)[value ? 'toBeHidden' : 'toBeShown']();
});
}));
});
});

Expand Down

0 comments on commit 615873d

Please sign in to comment.