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

When interpolated class attribute changes, classes from ng-class are not merged in #1016

Closed
maxmart opened this issue Jun 4, 2012 · 4 comments

Comments

@maxmart
Copy link
Contributor

maxmart commented Jun 4, 2012

Open http://jsfiddle.net/ADukg/224/
Set ngclass to true by clicking the first button. Note how the ngclass text turns green.
Then set class to true by clicking the second button. Note how the ngclass text turned red again.

So:
Result: div element has class="static class", thus coloring "class" green and "ngclass" red
Expected result: div element has class="static class ngclass", thus coloring both "class" and "ngclass" green

I have a (maybe very ugly) fix for this, which involves:

  1. Not setting attr[name] to undefined when name === 'class' in addAttrInterpolateDirective()
  2. $watch:ing $interpolate(attr['class']) in classDirective() and reapplying the "ngclasses" when it changes.

The reason I'm not totally sure about this solution is that I don't know why you're setting attributes to undefined in addAttrInterpolateDirective(), and I'm not sure what consequences it will have if we don't.

What do you think about this?

I can post some more code later, but I want to see if all tests pass first.

@IgorMinar
Copy link
Contributor

Why do you want to use both the interpolation and ngClass directive to control the value of the class attribute? Can you use just one or another?

The provided patch works, but introduces performance issues and unwanted dependency between $compile service and ngClass directive. So we can't merge it as is.

Unless we can find a valid reason why both interpolation and ngClass should be able to work together, I'm inclined to say that we are not going to fix this because the fix would complicate the code unnecessarily.

If we do find a valid reason to support this, then we should just change the watch expression of the ngClass directive from attr[name] to watching both attr[name] and attr.class and reapply ngClass if either of these changes.

@maxmart
Copy link
Contributor Author

maxmart commented Jun 7, 2012

The reason is that I want to apply one class conditionally (either applied or not) and others based on the value of a string variable. Like this: <div class="status_{{status}}" ng-class="{active: active}"></div>
Of course it's possible to solve this using other methods, but I feel that the expected result is that it should just work. It's very confusing that it works fine until the status happens to change.

I also see now that my solution wasn't very good :)

This seems to work and is more like what you suggest (I think)

      attr.$observe('class', function(value) {
        var ngClass = scope.$eval(attr[name]);
        reapply(ngClass, ngClass);
      });

which does not require changes to compile.js, and fewer changes to ngClass.js (don't need to inject $interpolate)

Would that also be inefficient if say both class and ng-class change in the same digest cycle or something?

maxmart added a commit to maxmart/angular.js that referenced this issue Jun 7, 2012
@maxmart
Copy link
Contributor Author

maxmart commented Jun 7, 2012

I pushed the new solution, so if you find my motivation reasonable and the code easy enough, it's there if you want it :)

@mhevery mhevery closed this as completed in b41fe9f Sep 4, 2012
mhevery pushed a commit that referenced this issue Sep 5, 2012
mhevery pushed a commit that referenced this issue Sep 6, 2012
mhevery pushed a commit that referenced this issue Sep 6, 2012
mhevery pushed a commit to mhevery/angular.js that referenced this issue Sep 7, 2012
@dmitryevseev
Copy link

Experienced this issue with AngularJS v1.1.0

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.

4 participants