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

Commit

Permalink
docs(ngShowHide): make a note of values considered to be falsy
Browse files Browse the repository at this point in the history 
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 #3436
Closes #5762
  • Loading branch information
@Foxandxss @caitp
Foxandxss authored and caitp committed Jan 15, 2014
1 parent cd0af8a commit e591ddc
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/ng/directive/ngShowHide.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@
*
* Just remember to include the important flag so the CSS override will function.
*
* <div class="alert alert-warning">
* **Note:** Here is a list of values that ngShow will consider as a falsy value (case insensitive):<br />
* "f" / "0" / "false" / "no" / "n" / "[]"
* </div>
*
* ## A note about animations with ngShow
*
* Animations in ngShow/ngHide work with the show and hide events that are triggered when the directive expression
Expand Down Expand Up @@ -200,6 +205,11 @@ var ngShowDirective = ['$animate', function($animate) {
* </pre>
*
* Just remember to include the important flag so the CSS override will function.
*
* <div class="alert alert-warning">
* **Note:** Here is a list of values that ngHide will consider as a falsy value (case insensitive):<br />
* "f" / "0" / "false" / "no" / "n" / "[]"
* </div>
*
* ## A note about animations with ngHide
*
Expand Down

4 comments on commit e591ddc

@mgol
Copy link
Member

@mgol mgol commented on e591ddc Jan 16, 2014

Choose a reason for hiding this comment

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

I still think making those values falsey was a mistake (a very early one), it's not intuitive and it required for our team to introduce functions checking truthiness in a couple of places. It's quite common if you rely on the fact that a certain field is present & not empty in data that comes from a remote server and we have spent some time figuring out why our data was displaying in a weird name in a couple of places. Besides, what if "n" in our backend doesn't mean no but, for example, one of options marked by letters a-z?

Documenting it is good but truthiness of JS values is so well-known that it didn't even occur to us at first to check in the docs for that.

It's impossible to change it in a point release since it's a breaking change but I still think it's sth that should be fixed in 1.3.0.

cc @IgorMinar

@mgol
Copy link
Member

@mgol mgol commented on e591ddc Jan 16, 2014

Choose a reason for hiding this comment

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

BTW, this list seems incomplete as these are only JS-truthy values that are considered falsey by Angular, all JS-falsey values are Angular-falsey as well.

@caitp
Copy link
Contributor

@caitp caitp commented on e591ddc Jan 16, 2014

Choose a reason for hiding this comment

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

That's true, if you want to change the wording to explain that these are the "extra" falsy values which people don't really expect to be falsy, that would be great!

I think we are probably going to ditch toBoolean() in the future, which will address your first point, but only time will tell I guess

@mgol
Copy link
Member

@mgol mgol commented on e591ddc Jun 23, 2014

Choose a reason for hiding this comment

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

toBoolean is being dropped in #7960 is someone's interested.

Please sign in to comment.