fix(accordion): panel-open class not shown with custom class

[chenyuzhcy]

Closes #4849

Working plunker: http://plnkr.co/edit/9ZBDb8jvo0ds5lhTW5MH?p=preview

[icfantv]

@chenyuzhcy, so unfortunately, we can't use ng-class here. In fact, this is what we used to have elsewhere in our code base (and perhaps even here...I don't remember off hand).

The issue is that the uib-accordion-group directive uses replace: true and Angular has decided to not support reconciling the ng-class issues when using this property.

[wesleycho]

I don't think we used to use ng-class at all - we did manage to break ng-class support somehow in part due to replace: true being applied.

For reference, here was the prior commit that added the dynamic class addition: 5ee23a4#diff-2c7728e12322ff30329ef487569c9435

[icfantv]

@chenyuzhcy, here's the link to the angular issue addressing the ng-class issues.

[icfantv]

@wesleycho, I couldn't remember exactly, only that we've decided to not support it. #4847 from a few days ago says as much.

[wesleycho]

I thought our decision applied to people using ng-class on the accordion-group element?

[icfantv]

...and not the template used internally....yes. That's totally fair.

[wesleycho]

I think we do want the ng-class to replace the interpolated class, so

<div class="panel" ng-class="panelClass || 'panel-default'">

This is to keep behavior consistent with dynamic class changing.

[wesleycho]

I made the change - thanks for spending the time on this quick fix!

[trickpattyFH20]

I had ng-class being used on an accordion-group element which now breaks after upgrading to the latest release. My workaround is to rewrite my CSS to support ng-class being used on a wrapping parent element.

I believe the ng-class merging should have either been done internally here till Angular releases a fix, or the implementation of ng-class in this directive should have just been avoided in favor of the interpolated class.

will use class={{classes}} workaround instead of ng-class...

[wesleycho]

@trickpattyFH20 you are relying on unsupported behavior in Angular itself with regards to ng-class and directives that use replace: true. See #4172 for details. This abuse of ng-class actually has negative performance ramifications as well

[trickpattyFH20]

@wesleycho wouldn't it be plausible to rework the uibAccordionGroup directive to not use replace:true?

[wesleycho]

That would result in breaking how Bootstrap's CSS works, and all theming libraries.

[trickpattyFH20]

@wesleycho do you have any sources about what is considered "abuse" of ng-class?
I don't think that an aditional interpolated class should be considered abuse.

I could see it as abuse if you parsed attrs.ngClass and recompiled it in the directive...
but why not just avoid all of that and use the interpolated class like before? This seems like a perfectly reasonable workaround to #5695

[wesleycho]

A simpler workaround would be to just do class="{{customClasses()}}" than bloating library code for a situation explicitly not supported by Angular.