fix(tooltip): do not toggle when isOpen is not boolean

[wesleycho]

• Coerce isOpen to boolean to do a proper comparison for firing
  toggleTooltipBind

Here is the fix in action.

This fixes #4335.

[Foxandxss]

Not sure if that is readable or not. One important thing. If you deactivate a JSHINT rule (with -WXXX) you have to put it after the code that is breaking the rule (+WXXX).

I would also add a test.

[wesleycho]

Can you explain about the readable and jshint parts?

There is no other good way to write this, due to val possibly not being a boolean, and a strict comparison is being done to a boolean. I could change it to a weak equality check, but that is pretty onerous and it becomes unobvious why we would want a weak check.

For the jshint change, can you explain with an example block of what you mean?

[Foxandxss]

Of course.

Readable for me is because you need more than a couple of seconds to realize what is going on on that line. If you say there is no better way, I am fine with it. The biggest issue for me is having to disable jshint rules for me, that is like a big sign of "Are you sure?".

I am not up to date with the issue, but can't you do something like:

val = !!val;
if (val !== ttScope.isOpen) { .. }

At least to avoid jshint.

Now, about the jshint thing. You know that jshint has lot of rules and they will make you fail the checking.

To avoid that, you can disable a jshint rule with that you did, but that will disable it for the rest of the file. You can put the rule up again. So the idea is:

 /* jshint -W123 */
// Code that breaks rule 123
 /* jshint +W123 */
// Rule 123 is activated again

You can see that in action here: https://github.com/angular/angular.js/blob/master/src/ng/parse.js#L805-L821

Where angular disables the "new Function" protection because they needed it.

[wesleycho]

I could make the change, but IMO it doesn't make it any easier to read - my opinion is that JSHint is wrong here, but it doesn't sound like they're going to change their stance on this from what I could tell.

I'll make the changes necessary and write a test around this sometime this weekend.

[Foxandxss]

Don't bother wes. Leave it as is (fixing jshint rule) and add that test please.

[jikkujj]

How do i help get this change as part of a build? Am new to all this