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

Tabs don't play nicely with ng-class #1741

Closed
xaralis opened this issue Feb 5, 2014 · 18 comments
Closed

Tabs don't play nicely with ng-class #1741

xaralis opened this issue Feb 5, 2014 · 18 comments

Comments

@xaralis
Copy link

xaralis commented Feb 5, 2014

Tabs component have issues when you add ng-class on the tab element.

Following chunk:

<tab ng-controller="Ctrl" ng-class="{true: 'valid', false: 'invalid'}[isValid()]"></tab>

results in:

<li ng-class="{true: 'valid', false: 'invalid'}[isValid()] {active: active, disabled: disabled}" ng-controller="Ctrl" class="ng-isolate-scope">
</li>

Have a close look to the ng-class which is improperly combined with tab's own ones.

@pkozlowski-opensource
Copy link
Member

http://plnkr.co/ please....

@xaralis
Copy link
Author

xaralis commented Feb 6, 2014

Have a look here: http://plnkr.co/edit/nUgm0Fmc7DNt3UJ2Rhfs?p=preview

The ng-class directive gets screwed up when attaching it on the <tab> element. It even raises an angular parsing error. The reason is that tabs add their own expression but somehow don't expect anyone else attaching ng-class too.

@pkozlowski-opensource
Copy link
Member

@xaralis are you sure you've included the right plunk? The one attached doesn't even have directives from this repo....

@xaralis
Copy link
Author

xaralis commented Feb 7, 2014

@pkozlowski-opensource Oh sorry, I forgot to freeze the plunker. There is a tabset directive. I hope it belongs here - it's angular-ui-bootstrap thing as far as I know.

@chrisirhc
Copy link
Contributor

Yes, this is an issue. The tab directive also uses the ng-class so I think AngularJS is concatenating the attribute.

See: http://plnkr.co/edit/VhklMKb4s0Ndg1MVUMMi?p=preview

To workaround this, one can use a custom template for the tab to add a class to the tab or use another div within the tab to style its contents.

@xaralis
Copy link
Author

xaralis commented Feb 7, 2014

I know it's possible to work around this (we did in our project) but for sake of consistency across your angular-related code, this should work -- it works everywhere else.

@chrisirhc
Copy link
Contributor

It is actually quite complex to make this work with ngClass because it would mean that we cannot use the ngClass directive in the default template of the tab. This reduces customizability because the class toggling logic will then have to be moved into the directive definition.

@xaralis
Copy link
Author

xaralis commented Feb 7, 2014

I see. If that's the case, it would be useful to stress this out in the docs so that the users know how to handle this correctly.

Or .. is it impossible to merge the ng-class expressions somehow?

@chrisirhc
Copy link
Contributor

It'll be interesting if AngularJS supports merging of ngClass on replaced elements/templates. Perhaps you could make a feature request on angular.js for that.

Feel free to send a pull request for additional documentation. 😄

@KamBha
Copy link

KamBha commented Mar 15, 2014

I have a similar problem where I want to attach a class to the tab heading if a form within the tab is marked as invalid (ie make the tab aware of the error).

Could a solution be to create a uiClass attribute against the heading and the content (or even uiHeadingClass and uiContentClass)? These directives would simply evaluate a class and add it the appropriate section of the tab.

@NaomiN
Copy link

NaomiN commented Mar 11, 2015

KamBha, I support this idea. We also want to highlight the tab with invalid class if the form within it is invalid. We're using ui-views to display the form for each tab so I am not sure how to implement the suggested workaround of separate div.

@chrisirhc chrisirhc added this to the Backlog milestone Mar 27, 2015
@Kepro
Copy link

Kepro commented Apr 10, 2015

+1

@uguryilmaz
Copy link

+1

@ravikiran438
Copy link

Also mentioned here angular/angular.js#5695

@brujo-rojas
Copy link

+1

@brujo-rojas
Copy link

I had the same problem

 <tab ng-class="{'hide-tab':($first && $last)}" >
   //...
</tab>

but I solved that with this

 <tab class="{{ ($first && $last) ? 'hide-tab':'' }} " >
   //...
</tab>

@icfantv
Copy link
Contributor

icfantv commented Aug 11, 2015

Hey all, please see the discussion in #4172. Using ng-class on a directive that uses replace: true is a very bad idea. There are numerous issues around replace: true and even talk of deprecating/removing it. But, unfortunately, it's not going anywhere soon and the caveat released by the core Angular team was that use it, but do so at your peril. I.e., use it knowing that doing so incorrectly can break your DOM.

I suspect this issue will not be addressed.

@wesleycho
Copy link
Contributor

Closing as won't support given the resolution in Angular.

@wesleycho wesleycho modified the milestones: Purgatory, Backlog Aug 11, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests