Accordion: add a class at the panel level to indicate open state

[davious]

Would you add a class to indicate the panel is open at the panel level?

I'm using a caret indicator in the panel-title like so:

.panel-title > a:before { 
  content: $fa-var-angle-right;
  font: normal normal normal 14px/1 FontAwesome; }

.panel-open .panel-title > a:before {
  content: $fa-var-angle-down; }

Adding a class like panel-open on the panel div would allow me to just go like

<accordion-group is-open="thisDrawer.open">

instead of

<accordion-group is-open="thisDrawer.open" ng-class="{'panel-open': thisDrawer.open}">

[RobJacobs]

This is a trivial change, simple as changing the template for the accordion-group to something like:

<div class="panel-heading" ng-class="{'panel-open': isOpen}">

My only concerns are:

1. Adding another watch to the directive.
2. Is the 'panel-open' class correct or should it just be 'open'.

[davious]

@RobJacobs thanks for considering this.

For 1. would a broadcast and on be any better?
For 2. let's mimic bootstrap's use of [aria-expanded]

over on bootstrap's example
they have an aria-expanded = true/false attribute on a. We could mimic this in ui bootstrap.

the styling could then be:

.panel-title > a[aria-expanded]:before { 
  content: $fa-var-angle-right;
  font: normal normal normal 14px/1 FontAwesome; }

.panel-title > a[aria-expanded=true]:before {
  content: $fa-var-angle-down; }

[icfantv]

I would argue that using events for something like this is wrong. I would consider adding a class such that no actual CSS is tied to the name. The CSS class open is used by Bootstrap in a number of different ways and doesn't contain any actual styling by itself. Additionally, no pre-existing CSS exists for .open.panel-heading, .open > .panel-heading, or .open panel-heading.

I'm ok with this change. @RobJacobs do you want to create the PR or would you like me to?

[icfantv]

UPDATED the aria-expanded attribute is already set correctly by the collapse directive.

Just an update on this. Adding aria-expanded is not sufficient since aria is specifically for accessibility. The solution here should add aria-expanded="true" and open when open~~, and aria-expanded="false" when closed~~. Note that there is no closed equivalent in Bootstrap.

[wesleycho]

I think this is ok - adding another two $watches is not the greatest, but this beats putting an ng-class on the accordion-group and it transcluding that so there are 4 $watches as a result.

[blowsie]

This commit broke my page, I can no longer use ng-class on an accordion group