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

fix(tabset): preserve tabset class attribute #584

Closed
wants to merge 1 commit into from
Closed

fix(tabset): preserve tabset class attribute #584

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 27, 2013

this should fix #581

@pkozlowski-opensource
Copy link
Member

@ajoslin what do you think about this approach?

Personally I was more in favor of doing something like in aaa8d81
I kind of don't like how we privilege one attribute.

@ghost
Copy link
Author

ghost commented Jun 28, 2013

I think this is more convenient for user to specify same class attribute as twitter bootstrap uses. Thus user of angular-bootstrap shouldn`t learn specific attributes of tabset directive. But this is only my opinion.

@pkozlowski-opensource
Copy link
Member

@bk1te @ajoslin is a master here so I will let him decide.

Just few notes here:

  • we are re-defining HTML vocabulary with those tags anyway so we need to assume that people knowing only Bootstrap CSS / HTML will have to learn new syntax anyway
  • we want to be CSS framework agnostic in a longer run

@ajoslin
Copy link
Contributor

ajoslin commented Jun 28, 2013

looks good to me :-)

Allowing css classes on the tabset is definitely a good thing to have. I'll merge it in a few minutes

@bekos
Copy link
Contributor

bekos commented Jun 28, 2013

@ajoslin @pkozlowski-opensource I think you can add replace: true for the tabset directive, and this might work.
Didn;t have time to test, but I can do if you wait, before merging this one.

@bekos
Copy link
Contributor

bekos commented Jun 28, 2013

OK. Tests don't fail and demo works fine if we do <tabset class="tabs-left">.
@ajoslin Do you agree on this approach?

@ajoslin
Copy link
Contributor

ajoslin commented Jun 28, 2013

Yeah. You're right.

ajoslin added a commit that referenced this pull request Jun 28, 2013
* replace is now set to true in the tabset directive options
@ajoslin
Copy link
Contributor

ajoslin commented Jun 28, 2013

Fixed in 8868f23, did the replace method instead. Thanks @bk1te for bringing this issue up :-)

@ajoslin ajoslin closed this Jun 28, 2013
@ghost
Copy link
Author

ghost commented Jun 28, 2013

thnks for resolving this issue

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Are tabs-left or tabs-right supported in 0.4.0?
3 participants