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

fix(ngShow/ngHide): follow javscript truthy/falsy logic #5573

Closed
wants to merge 1 commit into from

Conversation

lgalfaso
Copy link
Contributor

Make ngShow and ngHide follow javascript truthy/falsy logic and
not the custom toBoolean logic

Closes #5414
Closes #4277

@caitp
Copy link
Contributor

caitp commented Dec 30, 2013

If this does get merged, it is in fact a breaking change, and the commit message needs a BREAKING CHANGE block.

@lgalfaso
Copy link
Contributor Author

@caitp you are right, amended the patch with a commit message stating that this is a breaking change

@caitp
Copy link
Contributor

caitp commented Dec 30, 2013

Yeah, it's a bit more than that, toBoolean() treats some pretty weird values as falsy ("f", "n", "no") etc. I can't imagine anyone is actually using these, but just in case they are it would be good to show how to migrate so that they can get the same functionality after this patch.

@lgalfaso
Copy link
Contributor Author

I think that there is no easy way to migrate without exposing the toBoolean function or asking the user to write a function/expression that has this special logic present.

Do you think it would be good to add something like

To migrate, add the special cases to the expression. E.g. from

<div ng-hide="foo">X</div>

to

<div ng-hide="foo || foo == '[]'">X</div>

@caitp
Copy link
Contributor

caitp commented Dec 30, 2013

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'). It's just showing a basic short-term hack to get it working while they refactor the logic

@lgalfaso
Copy link
Contributor Author

@caitp Thanks!

Make ngShow and ngHide follow javascript `truthy`/`falsy` logic and
not the custom toBoolean logic

Fixes angular#5414 angular#4277

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

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').
@ghost ghost assigned tbosch Dec 31, 2013
@tbosch
Copy link
Contributor

tbosch commented Dec 31, 2013

Hi,
could you double check which other directives also use the toBoolean logic and change them also with your PR? E.g. ngIf, ...

Thanks!

@lgalfaso
Copy link
Contributor Author

@tbosch please look at #4005

@lgalfaso
Copy link
Contributor Author

lgalfaso commented Feb 5, 2014

This issue is going to be handled as part of #4005

@lgalfaso lgalfaso closed this Feb 5, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.