Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Class setting broken when combining ng-class and multiple interpolations in class attribute #10811

Closed
chezih opened this issue Jan 20, 2015 · 21 comments

Comments

@chezih
Copy link

chezih commented Jan 20, 2015

I'm in the middle of the transition from version 1.2.* to 1.3.* , and I came across a very strange and critical bug.

In my application I have a very simple directive contain a template with ng-class (with condition to scope property) for some reason it's not working with 1.3.* version, and it's work fine with 1.2.* version.

Have a look on this Plunker to illustrates the problem.

This Plunker code is with angular 1.2.* version, and as you can see it's work fine.

Try to change the angular version (index.html)

<script src="https://code.angularjs.org/1.3.9/angular.js"></script>
    <script src="https://code.angularjs.org/1.3.9/angular-animate.js"></script>
   <!--<script src="https://code.angularjs.org/1.2.28/angular.js"></script>
   <script src="https://code.angularjs.org/1.2.28/angular-animate.js"></script>-->

Refresh the page, and then you can see the bug:
Angular doesn't refresh the ng-class according to the 'active' property changing.

I tried to understand what can causes to this bug, and after a lot of tries I found that 'ngAnimate' module causes to this problem. try to delete the 'ngAnimate' dependency (script.js):

  //var app = angular.module('app', ['ngAnimate']);
    var app = angular.module('app', []);

And then you can see that everything is fine, so 'ngAnimate' version 1.3.* causes to this problem.

So it's AngularJS bug, am I right?

If not, what I'm doing wrong?

@Narretz
Copy link
Contributor

Narretz commented Jan 21, 2015

Hi, to clarify, the problem is that when you mouseover an item, a specific class does not get applied? Or is the class applied, but the styling is not? It would really help if you could reduce the example even more, i.e. remove all parts of the code and styling that have nothing to do with the problem.

@Narretz Narretz added this to the Backlog milestone Jan 21, 2015
@Narretz Narretz self-assigned this Jan 21, 2015
@chezih
Copy link
Author

chezih commented Jan 22, 2015

Hi,
Here is an updated Plunker, I tried to focus on the problem as much as I could.
The bug not about the mouseover (in the new example I deleted it).
The problem is the class inside the ng-class (in the directive) doesn't apply according to 'active' changed (true or false). Only when I click on the directive (and execute 'nav.activeIndex = $index') only then the 'active' is updated and effect on ng-class.
The bug: 'active' should effect on ng-class when the control is evaluate at the first time (when the control is initialized) like in version 1.2.*, and not only when I click on it.

Thanks!!

@Narretz
Copy link
Contributor

Narretz commented Jan 22, 2015

Did you save the plunker? It looks like it's the same one.

@Narretz
Copy link
Contributor

Narretz commented Jan 22, 2015

Okay, I see the mouseover is gone. However, there's still too much going on. Sometimes the button "containers" change color erratically, turning to white and black, then back to purple. Happens in both 1.2.x and 1.3.x.
Then, once with 1.3.x the button containers were all white.

Is there actually a way to reproduce this in a plunkr? What exactly should I do and see (which colors when if you want)?

@chezih
Copy link
Author

chezih commented Jan 22, 2015

I'm using chrome browser, and it's work fine in 1.2.x version!.
I don't understand what you don't understand?
It's very simple:
In version 1.2.x the 'active' changes effect on the ng-class (inside the template) in the beggining!!! (when the control is initialized - when the ng-repeat apply on it).
because this line: ng-init="nav={};nav.activeIndex = 0" only the first directive get the active=true and become a purple and the rest active=false and the got the black color. When you click on the directive it's become active=true because this line click="nav.activeIndex = $index", and everithing works fine!!.
In 1.3.x: when the control is initialized and ng-repeate apply on the directive the active for all directive is not true and not false (this is my problem, I don't know why) and therefore you see only white (because the template is without any background color), only when I click on it the active is updated and make the color changes.
Ostensibly this bug don't have any relation to ng-animate, but if I delete the ngAnimate dependency everything work fine!

@gkalpak
Copy link
Member

gkalpak commented Jan 22, 2015

@chezih: Your plunkr is sooo far away from an SSCCE. There is so much unrelated stuff in there, that makes it difficult and time-consuming to isolate the problem.
Providing a minimal, runnable example that reproduces the issue, means that people (such as @Narretz, me and probably a couple of others) won't waste time on trying to figure out what the problem is, which again means they will have more time to fix problems.

That said, the problem seems to be caused by the use of interpolated keys in the ngClass object.
Replacing the interpolated strings with hard-coded values solves the issue (Demo plunkr).

I am not sure if interpolated keys were intentionally supported in older versions (I had no idea they even worked), or supported "by chance", in which case we wouldn't fix this.
(If that's the case, then you could rely on other ways to achieve this functionality, e.g. using a template function instead of a hard-coded template, using a function on scope to generate the classes, etc.)

@gkalpak
Copy link
Member

gkalpak commented Jan 22, 2015

Although I don't believe it is the same issue, point (D) on #9109 (comment) might be relevant.

@Narretz
Copy link
Contributor

Narretz commented Jan 22, 2015

@gkalpak That looks interesting! I have reduced the problem to this form:
http://plnkr.co/edit/BBJ4QLSsbxZSPEPMEXJ4?p=preview

Once you have ng-Class and two interpolations in the class attr, the ng-class expression is not applied. In this form, there's no effect in removing ngAnimate, though.

Looks like a nice debugging experience for a cold and dark winter night.

@Narretz
Copy link
Contributor

Narretz commented Jan 22, 2015

I also tracked the breakage down to 1.3.0-beta.19; unfortunately there are a ton of changes that could be the cause.

@chezih
Copy link
Author

chezih commented Jan 22, 2015

@gkalpak - Thank for your comments!! - I'll try to do it better for next time...
Do you have some example for using template function (or link you can refer me to it)? It's can help me a lot (I tried to find some example in the web without success).
@Narretz - Nice bug! but I don't think it's my bug (as you said "there's no effect in removing ngAnimate", in my case you can see that ngAnimate caused the problem), maybe you should open a new bug

@Narretz
Copy link
Contributor

Narretz commented Jan 22, 2015

@chezih Your problem is fixed by removing one of the interpolated classes in the class attribute OR by removing ngAnimate

@chezih
Copy link
Author

chezih commented Jan 22, 2015

@Narretz - thanks!!

@gkalpak
Copy link
Member

gkalpak commented Jan 22, 2015

@Narretz: I don't think your plunkr reproduces the problem. It reproduces the problem in #9109, but I believe it is a different issue. E.g. removing the one interpolation from class, makes it work, but interpolating the key in ngClass, makes it break again (which I believe is the problem). (Example)

@chezih: Tried with the template function and it doesn't work either. It seems to be replace=true-related, since removing it works (with and without a template function): Demo

One more reason for deprecating replace :)

@gkalpak
Copy link
Member

gkalpak commented Jan 22, 2015

@chezih: So, ngClass seems to not like replace: true, but class does OK:
Demo using a function to determine the classes string.

@Narretz
Copy link
Contributor

Narretz commented Jan 23, 2015

@gkalpak I tested it with the original plnkr (which has interpolated ngClass key), and it works once I remove one interpolated class from the class attribute. Strange!

@gkalpak
Copy link
Member

gkalpak commented Jan 23, 2015

@Narretz: Hm...indeed it does. I am pretty sure I tried that and it didn't work (but obviously I did something else). Then it might be more related to #9109 than I thought. Or there might be more than one issues going on here. I don't know.

replace: true is a big mess ! I am glad it was deprecated :)

@Narretz Narretz removed their assignment Aug 9, 2015
@Narretz Narretz changed the title Angular ng-animate 1.3.* Causes to unexpected behavior to ng-class inside directive Class setting broken when combining ng-class and multiple interpolations in class attribute Nov 30, 2015
@chenasraf
Copy link

+1. I've found out that directive transclude is used along with directive replace, you can not use interpolations in the class -- if I try to interpolate anything, the class gets put twice inside, and the interpolations are always empty.

@sneakyfildy
Copy link

+1 this is a problem
I have a template, which has the following (inside an ng-repeat)
ng-class = {selected: "x == y"}
class = "{{x}} my-class-name"

when application starts with x == y, no "selected" class is applied until either ngAnimate is removed or {{x}} expression is removed from class attribute.

@tspendel
Copy link

+1 Can we expect to solve the problem in the near future?

@tomaszbob1
Copy link

tomaszbob1 commented Dec 29, 2016

+1 The same problem on Angular 1.5.x

@gkalpak
Copy link
Member

gkalpak commented Dec 29, 2016

This is a (documented) known issue afaict (see here and there) and there are no plans to fix this.
In a nutshell, you should not be dynamically modifying an attribute's value from several directives (e.g. ngClass and class with interpolation) and you should not use interpolation in attributes that expect an Angular expression. Furthermore, if replace: true is deprecated, should be avoided and there are several (known and unknown) corner cases that will not work correctly with it.

Based on that, I am going to close this as won't fix. If you are facing a different problem (that doesn't fall into the "known issue" category), please open a new issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.