Repository navigation
NgClass bugs in alpha 48 and multiple css classes #5763
Description
This is a plunk of the official one in the api preview with multiple class added instead, and change to alpha48
http://plnkr.co/edit/TxllaqdcEblrBW065XmX?p=preview
if you click the button and disabled back and forth, you will see that both class don't get added.
Activity
pkozlowski-opensource commentedon Dec 10, 2015
Could you please be more precise? What is the expected behavior? What do you mean by "both class" here? I don't see any buggy behavior... What is a browser that you are testing with?
user414 commentedon Dec 10, 2015
Sorry was referrring to this piece of code
pkozlowski-opensource commentedon Dec 10, 2015
Oh, I see, thnx. It looks like a legit bug, but a corner case as well, so not sure if we will be able to resolve it before beta (unless someone from the community wants to step in!).
pkozlowski-opensource commentedon Dec 10, 2015
Actually, thinking about it some more, I don't think we can fix this one. If I understand your plunk correctly we are talking about the following use-case:
[ng-class]="{'foo bar': condition, 'foo baz': !condition}"So we are kind of saying "I want to have and not have
fooclass at the same time" which obviously doesn't make sense. It is an order of class addition / removal that will lead to different results - this is not something deterministic.I guess you need to change your code to be sth like:
[ng-class]="{'active has-error': isOn, 'disabled has-success': isDisabled, 'has-feedback': isOn || isDisabled}".Closing, as I don't think we can fix it in a way that would make sense to everyone... But I would be happy if someone can prove me otherwise!
user414 commentedon Dec 10, 2015
I am not sure I understand what you are saying. First in this plunk it's not the case of condition and !condition it's
[ng-class]="{'foo bar': condition, 'foo baz': !condition}"
it certainly not up to Angular to bother about what's on the left side, the specs state
Object - each key corresponds to a CSS class name while values are interpreted as expressions evaluating to Boolean. If a given expression evaluates to true a corresponding CSS class is added - otherwise it is removed.
and so all the string on the left should be outputed. For reference I thought maybe that unlike angular1 perhaps multiple class name on the left might not be supported but
if (condition)
output foo bar
else if (!condition)
output foo baz
now in the background there can be optimization and all that but for Angular to simply decide that it would not output foo is incorrect. It doesn't respect the current documentation/specification and I doesn't make sense to me. It's not up to a language to interpret what the program is trying to do it should just follow what the language specification are, and in this case if I understand correctly it is to output all string if the expression evaluates to true. It should be irrelevant what's in that string. Or I'm missing something:-)
pkozlowski-opensource commentedon Dec 10, 2015
But there is no order in JS objects
user414 commentedon Dec 10, 2015
Not sure what order has to do in here? The problem is not that the order is incorrect as in foo bar or bar foo the problem is that there is no foo at all. Can you explain further the order reference?
pkozlowski-opensource commentedon Dec 10, 2015
Sorry, can't check your comments right now. Re-opened, will look into this in the coming day(s)
gkalpak commentedon Dec 20, 2015
@user414, you seem to be missing a crucial part of
ng-class' functionality: removing the added classes when the expression evaluates tofalse.[ng-class]="{'active has-error has-feedback': isOn, 'disabled has-success has-feedback': isDisabled}"can be broken down to:1.1: If
isOnevaluates totrue, add classesactive,has-errorandhas-feedback.1.1: If
isOnevaluates tofalse, remove classesactive,has-errorandhas-feedback.2.1: If
isDisabledevaluates totrue, add classesdisabled,has-successandhas-feedback.2.2: If
isDisabledevaluates tofalse, remove classesdisabled,has-successandhas-feedback.There is so way to keep track of how the classes were added to the element's classList or who added them and it's not
ng-class' purpose to do so. It just applies the rules it knows about.Thus, whenever you change the values so that
isOn === trueandisDisabled === false, rules (1.1) and (2.2) will both trigger, resulting in the following sequence of events (in a simplified way):activehas-errorandhas-feedbackget added (due to 1.1).disabled,has-successandhas-feedbackget removed (due to 2.2).As a result,
has-feedbackgets added and then removed and therefore won't be present on the element. If the rules were applied in the reverse order (i.e. first 2.2 then 1.1), thenhas-feedbackwould be present whenisOn === trueandisDisabled === false, but wouldn't be present whenisOn === falseandisDisabled === true. This is where order matters.As @pkozlowski-opensource mentioned, there is no guarantee wrt the order in which object properties are iterated over (it is implementation specific and may defer between browsers/version etc).
So, in order to avoid unexpected behavior, it is a good practice to always manipulated each class in one rule only (e.g. as suggested in #5763 (comment)).
user414 commentedon Dec 20, 2015
@gkalpak Thanks for the explanation, now it's all clear. Sorry for being hard of understanding on this one. To drive the point home about the remove perhaps we could change the documentation to have this , or a clearer modified version, showing exactly the implication of the remove part. In reality, this is actually more powerful then a standard add implementation but never realized it until now. We can close this, thanks again.
pkozlowski-opensource commentedon Dec 28, 2015
@gkalpak thnx for putting in writing all those details! I was about to come back to this issue and saw your perfect explanation!
angular-automatic-lock-bot commentedon Sep 7, 2019
This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.
Read more about our automatic conversation locking policy.
This action has been performed automatically by a bot.