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

False boolean values found when using ng-show/ng-hide #3436

Closed
johnwun opened this issue Aug 2, 2013 · 7 comments
Closed

False boolean values found when using ng-show/ng-hide #3436

johnwun opened this issue Aug 2, 2013 · 7 comments

Comments

@johnwun
Copy link

johnwun commented Aug 2, 2013

Re-opening this ticket from a year ago. This is clearly a bug in Angular, and should not have been closed.

If you type 'n', 'no', 'f', 'false' or '0' into the search field, ng-hide / ng-show is triggered incorrectly.

Here is an updated plunker illustrating the issue:
http://plnkr.co/U99tFB

Original ticket:
#1229

@tuuling
Copy link

tuuling commented Dec 2, 2013

I agree. The docs clearly say :"When the ngShow expression evaluates to false then the ng-hide CSS class is added to the class attribute on the element causing it to become hidden." There is no way that one would expect a string in some cases evaluates to true and in other cases to false.

@jgeorge-gc
Copy link

+1

I'd expect a string value to evaluate to true. The current behavior is very confusing and should be fixed or at least mentioned in the docs.

@jhartikainen
Copy link

It's rather non-standard behavior which could very easily cause bugs, so unless there's some actual reason for this, +1 for fix.

@caitp
Copy link
Contributor

caitp commented Jan 15, 2014

The documentation fix should inform users of this behaviour, however I think the toBoolean() stuff is probably going to be removed in the future, so a proper solution for your bug may be waiting... Feel free to re-open this issue if you don't find a documentation update satisfactory

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
This issue has been a focus of problems for some users and we discussed it on the IRC that it should
be at least documented.

~Amended the style to use bootstrap notes, I think overall it looks better and catches the eyes more
easily. However there are no anchor links to these, if these are necessary they can be added later.

Closes angular#3436
Closes angular#5762
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
This issue has been a focus of problems for some users and we discussed it on the IRC that it should
be at least documented.

~Amended the style to use bootstrap notes, I think overall it looks better and catches the eyes more
easily. However there are no anchor links to these, if these are necessary they can be added later.

Closes angular#3436
Closes angular#5762
@bytasv
Copy link

bytasv commented Feb 18, 2014

Oh.. this kind of magic is just pure evil.. Why would anyone do such a weird thing and introduce this kind of typecasting?
Simply changing function toBoolean(value) { ... } to:

function toBoolean(value) {
  return Boolean(value);
}

Would bring an expected behaviour.

@constfun
Copy link

constfun commented Jun 9, 2014

This issue should be reopened.

As @bytasv has mentioned this is pure evil. This is clearly a bug, moreover it's a a bug that is very difficult to catch! Mentioning it in the docs is not a solution.

@caitp
Copy link
Contributor

caitp commented Jun 9, 2014

Dudes, calm down! I am in perfect agreement that it's not the right thing to do. Maybe some day we can remove toBoolean() from the tree entirely. It would be a breaking change, so if it's going to happen, it should happen before 1.3 goes stable.

PRs welcome, IMO.

dag0310 added a commit to dag0310/shopper that referenced this issue Feb 8, 2015
…her one is entered alone. After the second letter has been entered, the results will be returned.

Reason: 'f' will be interpreted as false and 'n' as null, which are both falsy.

Further reference:
http://stackoverflow.com/questions/18177522/why-ng-show-doesnt-work-when-f-or-n-is-typed

angular/angular.js#1229

angular/angular.js#3436
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants