Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Allow different accordion-group classes #3968

Closed
hejmsdz opened this issue Jul 19, 2015 · 15 comments
Closed

Allow different accordion-group classes #3968

hejmsdz opened this issue Jul 19, 2015 · 15 comments

Comments

@hejmsdz
Copy link

hejmsdz commented Jul 19, 2015

The class of panels (aka accordion groups) at https://github.com/angular-ui/bootstrap/blob/master/template/accordion/accordion-group.html are hard-coded to panel-default. I would like to be able to use a different one. Currently, even if I use ng-class to assign another class to the panel, the panel-default class is still there.

@wesleycho
Copy link
Contributor

This sounds reasonable - feel free to open a PR.

@RicardoRFaria
Copy link
Contributor

One question. The behavior of this feature is:

  • if I place attribute ng-class he will override panel-default and panel too, maybe.
    or
  • if I place attribute panelClass (or something like this) he will override all classes of panel.

@wesleycho
Copy link
Contributor

Another solution is class="foo {{bar}}"

@RicardoRFaria
Copy link
Contributor

And when I place class="foo {{bar}}" will override all default classes? (panel panel-default)

@wesleycho
Copy link
Contributor

it could be class="foo {{bar || 'panel panel-default'}}" or something like that

@RicardoRFaria
Copy link
Contributor

ok, thx

@RobJacobs
Copy link
Contributor

Just a suggestion, rather than go through all the work involved with removing/adding classes, would it be better to offer the ability to override the templates with an attribute? I.E., changing this:

templateUrl: 'template/accordion/accordion.html'

To this:

templateUrl: function(element, attrs) {
   return attrs.template || 'template/accordion/accordion.html';
}

@wesleycho
Copy link
Contributor

That is also a valid option/feature - may be even better for performance reasons & simplified api.

@RobJacobs
Copy link
Contributor

It got me thinking, it would be a nice (and easy) feature to add to all the simple (mostly markup) directives and would relieve the burden of having to do customization on the default templates.

@RicardoRFaria
Copy link
Contributor

I think that give option to change the default template is good, in other way, when you change the template "only to customize the css" you lose features like accessibility (that isn't implemented in accordion yet).

@wesleycho
Copy link
Contributor

Hm? I merged your PR with accessibility enhancements to collapse in 9255134 .

@RicardoRFaria
Copy link
Contributor

@wesleycho sorry, I talked collapse, but is accordion.

@icfantv
Copy link
Contributor

icfantv commented Aug 20, 2015

Just an update here. We now have support for @RobJacobs solution in master so the only question remaining is do we allow a custom panel class option ala the suggestion by @wesleycho above? While this seems like a pretty easy change to make and test, we're kind of pulling the thread on the proverbial sweater, ala:

If we allow customization here, don't we also need to also add it for the panel-heading, panel-title, and panel-collapse classes? And what about panel-group in the accordion directive template? And if we allow this, are we really still Bootstrap?

It sounds like using the custom template solution is the right way to go here and I think we can close this.

@wesleycho
Copy link
Contributor

In this case, there are other supported panel classes, such as panel-success - see https://github.com/twbs/bootstrap/blob/master/less/panels.less

@icfantv
Copy link
Contributor

icfantv commented Aug 20, 2015

Ah crud, I'd forgotten about those. So we probably only want to support replacing just the panel-default as you describe above.

@icfantv icfantv self-assigned this Aug 20, 2015
icfantv added a commit to icfantv/bootstrap that referenced this issue Aug 20, 2015
* add support for custom panel class

fixes angular-ui#3968
@wesleycho wesleycho modified the milestones: 0.13.4 (Performance), Backlog Aug 20, 2015
icfantv added a commit to icfantv/bootstrap that referenced this issue Aug 21, 2015
* add support for custom panel class

fixes angular-ui#3968
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants