refactor(accordion): use optional binding for isOpen

[bekos]

No description provided.

[mvhecke]

I viewed your changes and I don't see anything strange, a good step in simplifying things:thumbsup:

[bekos]

@Gamemaniak There is a similar pattern in carousel if you care ;-)

[chrisirhc]

Looks good to me. Again, awesome change. Good to clean up after AngularJS 1.2.

Just one thing, the coercion bugs me a little.
But running the watcher twice probably won't cause any problems.

[chrisirhc]

This coercion bugs me a little as it causes the watcher to get triggered twice if isOpen is set to a truthy value that's not true.

Say isOpen = "abc", first digest is set to !!"abc" which is true, watcher runs again because the isOpen changed from "abc" to true.

I'll argue that if (value) is adequate. Though that means isOpen may not be a boolean if the user decides to set it to something else.

[bekos]

I think you are right, scope.isOpen = !!value has to be removed. What about this:

scope.$watch('isOpen', function(value) {
    if ( !!value ) {
         accordionCtrl.closeOthers(scope);
    }
});

[chrisirhc]

If I recall, if (value) is good enough, as JavaScript automatically does the !! coercion so the result is equivalent.

[bekos]

Yeah, you are right, those are equivalent :-)

[chrisirhc]

😄 I'm happy about this refactor and more to come! 👍

[mvhecke]

@bekos Wil do that tomorrow, I presume you you're referring to the slide directive?

[bekos]

@Gamemaniak Yes.

[pkozlowski-opensource]

Awesome @bekos 👍