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

feat(accordion): allow custom panel class #4242

Closed
wants to merge 2 commits into from

Conversation

icfantv
Copy link
Contributor

@icfantv icfantv commented Aug 20, 2015

  • add support for custom panel class

fixes #3968

* add support for custom panel class

fixes angular-ui#3968
@wesleycho wesleycho added this to the 0.13.4 (Performance) milestone Aug 20, 2015
@@ -1,4 +1,4 @@
<div class="panel panel-default">
<div class="panel panel-{{panelClass || 'default'}}">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would prefer this be {{panelClass || 'panel-default'}} - gives the users of the library a little more flexibility without any cost.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really went back and forth here on this (as well as putting the default inside the directive ala scope.panelClass = attrs.panelClass || 'panel-default'; but opted for what I did so as to keep the code as close to the element as possible) but ultimately decided to mimic what we do in tabs.

Recall that we did talk about only supporting the built-in contextual panel classes from Bootstrap - which is also why I went this route.

But I don't have a strong feeling either way and if we want to be totally flexible that solution would do it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some users of the library who use it with custom CSS or other CSS libraries - I think we should try to be less opinionated here for minimal pain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

* add support for custom panel class

fixes angular-ui#3968
@wesleycho
Copy link
Contributor

LGTM

@icfantv icfantv closed this in 5ee23a4 Aug 21, 2015
@icfantv icfantv deleted the accordion_classes_3698 branch August 21, 2015 16:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow different accordion-group classes
2 participants