fix(ng:class): make ng:class friendly towards other code adding/removing

src/directives.js

@@ -200,7 +200,7 @@ angularDirective("ng:controller", function(expression){
    <doc:example>
      <doc:source>
        Enter name: <input type="text" name="name" value="Whirled"> <br>
-       Hello <span ng:bind="name" />!
+       Hello <span ng:bind="name"></span>!
      </doc:source>
      <doc:scenario>
        it('should check ng:bind', function(){
@@ -549,16 +549,11 @@ angularDirective("ng:submit", function(expression, element) {
 
 function ngClass(selector) {
   return function(expression, element) {
-    var existing = element[0].className + ' ';
     return function(element) {
-      this.$watch(function(scope) {
+      this.$watch(expression, function(scope, newVal, oldVal) {
         if (selector(scope.$index)) {
-          var ngClassVal = scope.$eval(element.attr('ng:class'));
-          if (isArray(ngClassVal)) ngClassVal = ngClassVal.join(' ');
-          var value = scope.$eval(expression);
-          if (isArray(value)) value = value.join(' ');
-          if (ngClassVal && ngClassVal !== value) value = value + ' ' + ngClassVal;
-          element[0].className = trim(existing + value);
+          element.removeClass(isArray(oldVal) ? oldVal.join(' ') : oldVal)
+          element.addClass(isArray(newVal) ? newVal.join(' ') : newVal);
         }
       });
     };
@@ -571,11 +566,17 @@ function ngClass(selector) {
  * @name angular.directive.ng:class
  *
  * @description
- * The `ng:class` allows you to set CSS class on HTML element
- * conditionally.
+ * The `ng:class` allows you to set CSS class on HTML element dynamically by databinding an
+ * expression that represents all classes to be added.
+ *
+ * The directive won't add duplicate classes if a particular class was already set.
+ *
+ * When the expression changes, the previously added classes are removed and only then the classes
+ * new classes are added.
  *
  * @element ANY
- * @param {expression} expression {@link guide/dev_guide.expressions Expression} to eval.
+ * @param {expression} expression {@link guide/dev_guide.expressions Expression} to eval. The result
+ *   of the evaluation can be a string representing space delimited class names or an array.
  *
  * @example
    <doc:example>
@@ -612,12 +613,15 @@ angularDirective("ng:class", ngClass(function(){return true;}));
  *
  * @description
  * The `ng:class-odd` and `ng:class-even` works exactly as
- * `ng:class`, except it works in conjunction with `ng:repeat`
- * and takes affect only on odd (even) rows.
+ * {@link angular.directive.ng:class ng:class}, except it works in conjunction with `ng:repeat` and
+ * takes affect only on odd (even) rows.
+ *
+ * This directive can be applied only within a scope of an
+ * {@link angular.widget.@ng:repeat ng:repeat}.
  *
  * @element ANY
- * @param {expression} expression {@link guide/dev_guide.expressions Expression} to eval. Must be
- *  inside `ng:repeat`.
+ * @param {expression} expression {@link guide/dev_guide.expressions Expression} to eval. The result
+ *   of the evaluation can be a string representing space delimited class names or an array.
  *
  * @example
    <doc:example>
@@ -650,12 +654,15 @@ angularDirective("ng:class-odd", ngClass(function(i){return i % 2 === 0;}));
  *
  * @description
  * The `ng:class-odd` and `ng:class-even` works exactly as
- * `ng:class`, except it works in conjunction with `ng:repeat`
- * and takes affect only on odd (even) rows.
+ * {@link angular.directive.ng:class ng:class}, except it works in conjunction with `ng:repeat` and
+ * takes affect only on odd (even) rows.
+ *
+ * This directive can be applied only within a scope of an
+ * {@link angular.widget.@ng:repeat ng:repeat}.
  *
  * @element ANY
- * @param {expression} expression {@link guide/dev_guide.expressions Expression} to eval. Must be
- *  inside `ng:repeat`.
+ * @param {expression} expression {@link guide/dev_guide.expressions Expression} to eval. The result
+ *   of the evaluation can be a string representing space delimited class names or an array.
  *
  * @example
    <doc:example>

src/jqLite.js

@@ -155,16 +155,24 @@ function JQLiteHasClass(element, selector, _) {
 }
 
 function JQLiteRemoveClass(element, selector) {
-  element.className = trim(
-      (" " + element.className + " ")
-      .replace(/[\n\t]/g, " ")
-      .replace(" " + selector + " ", " ")
-  );
+  if (selector) {
+    forEach(selector.split(' '), function(cssClass) {
+      element.className = trim(
+          (" " + element.className + " ")
+          .replace(/[\n\t]/g, " ")
+          .replace(" " + trim(cssClass) + " ", " ")
+      );
+    });
+  }
 }
 
-function JQLiteAddClass(element, selector ) {
-  if (!JQLiteHasClass(element, selector)) {
-    element.className = trim(element.className + ' ' + selector);
+function JQLiteAddClass(element, selector) {
+  if (selector) {
+    forEach(selector.split(' '), function(cssClass) {
+      if (!JQLiteHasClass(element, cssClass)) {
+        element.className = trim(element.className + ' ' + trim(cssClass));
+      }
+    });
   }
 }
 
@@ -245,7 +253,13 @@ forEach({
   },
 
   attr: function(element, name, value){
-    if (SPECIAL_ATTR[name]) {
+    if (name === 'class') {
+      if(isDefined(value)) {
+        element.className = value;
+      } else {
+        return element.className;
+      }
+    } else if (SPECIAL_ATTR[name]) {
       if (isDefined(value)) {
         element[name] = !!value;
       } else {

test/BinderSpec.js

@@ -393,14 +393,6 @@ describe('Binder', function(){
     assertHidden(scope.$element);
   });
 
-  it('BindClassUndefined', function(){
-    var scope = this.compile('<div ng:class="undefined"/>');
-    scope.$apply();
-
-    assertEquals(
-        '<div class="undefined" ng:class="undefined"></div>',
-        sortedHtml(scope.$element));
-  });
 
   it('BindClass', function(){
     var scope = this.compile('<div ng:class="clazz"/>');

test/directivesSpec.js

@@ -196,13 +196,103 @@ describe("directive", function(){
       expect(element.hasClass('B')).toBe(false);
     });
 
-    it('should support adding multiple classes', function(){
+
+    it('should support adding multiple classes via an array', function(){
       var scope = compile('<div class="existing" ng:class="[\'A\', \'B\']"></div>');
       scope.$digest();
       expect(element.hasClass('existing')).toBeTruthy();
       expect(element.hasClass('A')).toBeTruthy();
       expect(element.hasClass('B')).toBeTruthy();
     });
+
+
+    it('should support adding multiple classes via a space delimited string', function(){
+      var scope = compile('<div class="existing" ng:class="\'A B\'"></div>');
+      scope.$digest();
+      expect(element.hasClass('existing')).toBeTruthy();
+      expect(element.hasClass('A')).toBeTruthy();
+      expect(element.hasClass('B')).toBeTruthy();
+    });
+
+
+    it('should preserve class added post compilation with pre-existing classes', function() {
+      var scope = compile('<div class="existing" ng:class="dynClass"></div>');
+      scope.dynClass = 'A';
+      scope.$digest();
+      expect(element.hasClass('existing')).toBe(true);
+
+      // add extra class, change model and eval
+      element.addClass('newClass');
+      scope.dynClass = 'B';
+      scope.$digest();
+
+      expect(element.hasClass('existing')).toBe(true);
+      expect(element.hasClass('B')).toBe(true);
+      expect(element.hasClass('newClass')).toBe(true);
+    });
+
+
+    it('should preserve class added post compilation without pre-existing classes"', function() {
+      var scope = compile('<div ng:class="dynClass"></div>');
+      scope.dynClass = 'A';
+      scope.$digest();
+      expect(element.hasClass('A')).toBe(true);
+
+      // add extra class, change model and eval
+      element.addClass('newClass');
+      scope.dynClass = 'B';
+      scope.$digest();
+
+      expect(element.hasClass('B')).toBe(true);
+      expect(element.hasClass('newClass')).toBe(true);
+    });
+
+
+    it('should preserve other classes with similar name"', function() {
+      var scope = compile('<div class="ui-panel ui-selected" ng:class="dynCls"></div>');
+      scope.dynCls = 'panel';
+      scope.$digest();
+      scope.dynCls = 'foo';
+      scope.$digest();
+      expect(element.attr('class')).toBe('ui-panel ui-selected ng-directive foo');
+    });
+
+
+    it('should not add duplicate classes', function() {
+      var scope = compile('<div class="panel bar" ng:class="dynCls"></div>');
+      scope.dynCls = 'panel';
+      scope.$digest();
+      expect(element.attr('class')).toBe('panel bar ng-directive');
+    });
+
+
+    it('should remove classes even if it was specified via class attribute', function() {
+      var scope = compile('<div class="panel bar" ng:class="dynCls"></div>');
+      scope.dynCls = 'panel';
+      scope.$digest();
+      scope.dynCls = 'window';
+      scope.$digest();
+      expect(element.attr('class')).toBe('bar ng-directive window');
+    });
+
+
+    it('should remove classes even if they were added by another code', function() {
+      var scope = compile('<div ng:class="dynCls"></div>');
+      scope.dynCls = 'foo';
+      scope.$digest();
+      element.addClass('foo');
+      scope.dynCls = '';
+      scope.$digest();
+      expect(element.attr('class')).toBe('ng-directive');
+    });
+
+
+    it('should convert undefined and null values to an empty string', function() {
+      var scope = compile('<div ng:class="dynCls"></div>');
+      scope.dynCls = [undefined, null];
+      scope.$digest();
+      expect(element.attr('class')).toBe('ng-directive');
+    });
   });
 
 

test/jqLiteSpec.js

@@ -168,6 +168,15 @@ describe('jqLite', function(){
       var elm = jqLite('<div class="any">a</div>');
       expect(elm.attr('non-existing')).toBeUndefined();
     });
+
+    it('should special-case "class" attribute', function() {
+      // stupid IE9 returns null for element.getAttribute('class') when element has ng:class attr
+      var elm = jqLite('<div class=" any " ng:class="dynCls">a</div>');
+      expect(elm.attr('class')).toBe(' any ');
+
+      elm.attr('class', 'foo  bar');
+      expect(elm.attr('class')).toBe('foo  bar');
+    });
   });
 
 
@@ -181,13 +190,50 @@ describe('jqLite', function(){
     });
 
 
-    describe('addClass', function(){
-      it('should allow adding of class', function(){
+    describe('addClass', function() {
+      it('should allow adding of class', function() {
         var selector = jqLite([a, b]);
         expect(selector.addClass('abc')).toEqual(selector);
         expect(jqLite(a).hasClass('abc')).toEqual(true);
         expect(jqLite(b).hasClass('abc')).toEqual(true);
       });
+
+
+      it('should ignore falsy values', function() {
+        var jqA = jqLite(a);
+        expect(jqA[0].className).toBe('');
+
+        jqA.addClass(undefined);
+        expect(jqA[0].className).toBe('');
+
+        jqA.addClass(null);
+        expect(jqA[0].className).toBe('');
+
+        jqA.addClass(false);
+        expect(jqA[0].className).toBe('');
+      });
+
+
+      it('should allow multiple classes to be added in a single string', function() {
+        var jqA = jqLite(a);
+        expect(a.className).toBe('');
+
+        jqA.addClass('foo bar baz');
+        expect(a.className).toBe('foo bar baz');
+      });
+
+
+      it('should not add duplicate classes', function() {
+        var jqA = jqLite(a);
+        expect(a.className).toBe('');
+
+        a.className = 'foo';
+        jqA.addClass('foo');
+        expect(a.className).toBe('foo');
+
+        jqA.addClass('bar foo baz');
+        expect(a.className).toBe('foo bar baz');
+      });
     });
 
 
@@ -223,6 +269,7 @@ describe('jqLite', function(){
         expect(jqLite(b).hasClass('abc')).toEqual(false);
       });
 
+
       it('should correctly remove middle class', function() {
         var element = jqLite('<div class="foo bar baz"></div>');
         expect(element.hasClass('bar')).toBe(true);
@@ -233,6 +280,15 @@ describe('jqLite', function(){
         expect(element.hasClass('bar')).toBe(false);
         expect(element.hasClass('baz')).toBe(true);
       });
+
+
+      it('should remove multiple classes specified as one string', function() {
+        var jqA = jqLite(a);
+
+        a.className = 'foo bar baz';
+        jqA.removeClass('foo baz noexistent');
+        expect(a.className).toBe('bar');
+      });
     });
   });
 
@@ -257,6 +313,19 @@ describe('jqLite', function(){
       expect(jqLite(a).css('prop')).toBeFalsy();
       expect(jqLite(b).css('prop')).toBeFalsy();
     });
+
+
+    it('should set a bunch of css properties specified via an object', function() {
+      expect(jqLite(a).css('foo')).toBeFalsy();
+      expect(jqLite(a).css('bar')).toBeFalsy();
+      expect(jqLite(a).css('baz')).toBeFalsy();
+
+      jqLite(a).css({'foo': 'a', 'bar': 'b', 'baz': ''});
+
+      expect(jqLite(a).css('foo')).toBe('a');
+      expect(jqLite(a).css('bar')).toBe('b');
+      expect(jqLite(a).css('baz')).toBeFalsy();
+    });
   });