fix(form): preperly clean up when invalid widget is removed

Removing invalid widget sometimes resulted in improper cleanup of the form state.
angular · Apr 4, 2012 · 21b77ad · 21b77ad
1 parent 2f5dba4
commit 21b77ad
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 38 deletions.
diff --git a/src/ng/directive/form.js b/src/ng/directive/form.js
@@ -70,23 +70,43 @@ function FormController(element, attrs) {
     if (control.$name && form[control.$name] === control) {
       delete form[control.$name];
     }
-    forEach(errors, cleanupControlErrors, control);
+    forEach(errors, function(queue, validationToken) {
+      form.$setValidity(validationToken, true, control);
+    });
   };
 
   form.$setValidity = function(validationToken, isValid, control) {
-    if (isValid) {
-      cleanupControlErrors(errors[validationToken], validationToken, control);
+    var queue = errors[validationToken];
 
-      if (!invalidCount) {
-        toggleValidCss(isValid);
-        form.$valid = true;
-        form.$invalid = false;
+    if (isValid) {
+      if (queue) {
+        arrayRemove(queue, control);
+        if (!queue.length) {
+          invalidCount--;
+          if (!invalidCount) {
+            toggleValidCss(isValid);
+            form.$valid = true;
+            form.$invalid = false;
+          }
+          errors[validationToken] = false;
+          toggleValidCss(true, validationToken);
+          parentForm.$setValidity(validationToken, true, form);
+        }
       }
+
     } else {
       if (!invalidCount) {
         toggleValidCss(isValid);
       }
-      addControlError(validationToken, control);
+      if (queue) {
+        if (includes(queue, control)) return;
+      } else {
+        errors[validationToken] = queue = [];
+        invalidCount++;
+        toggleValidCss(false, validationToken);
+        parentForm.$setValidity(validationToken, false, form);
+      }
+      queue.push(control);
 
       form.$valid = false;
       form.$invalid = true;
@@ -99,31 +119,6 @@ function FormController(element, attrs) {
     form.$pristine = false;
   };
 
-  function cleanupControlErrors(queue, validationToken, control) {
-    if (queue) {
-      control = control || this; // so that we can be used in forEach;
-      arrayRemove(queue, control);
-      if (!queue.length) {
-        invalidCount--;
-        errors[validationToken] = false;
-        toggleValidCss(true, validationToken);
-        parentForm.$setValidity(validationToken, true, form);
-      }
-    }
-  }
-
-  function addControlError(validationToken, control) {
-    var queue = errors[validationToken];
-    if (queue) {
-      if (includes(queue, control)) return;
-    } else {
-      errors[validationToken] = queue = [];
-      invalidCount++;
-      toggleValidCss(false, validationToken);
-      parentForm.$setValidity(validationToken, false, form);
-    }
-    queue.push(control);
-  }
 }
 
 

diff --git a/test/ng/directive/formSpec.js b/test/ng/directive/formSpec.js
@@ -189,16 +189,16 @@ describe('form', function() {
 
     it('should deregister a child form when its DOM is removed', function() {
       doc = jqLite(
-          '<form name="parent">' +
-            '<div class="ng-form" name="child">' +
-              '<input ng:model="modelA" name="inputA" required>' +
-            '</div>' +
+        '<form name="parent">' +
+          '<div class="ng-form" name="child">' +
+          '<input ng:model="modelA" name="inputA" required>' +
+          '</div>' +
           '</form>');
       $compile(doc)(scope);
       scope.$apply();
 
       var parent = scope.parent,
-          child = scope.child;
+        child = scope.child;
 
       expect(parent).toBeDefined();
       expect(child).toBeDefined();
@@ -211,6 +211,39 @@ describe('form', function() {
     });
 
 
+    it('should deregister a input when its removed from DOM', function() {
+      doc = jqLite(
+        '<form name="parent">' +
+          '<div class="ng-form" name="child">' +
+            '<input ng:model="modelA" name="inputA" required>' +
+          '</div>' +
+        '</form>');
+      $compile(doc)(scope);
+      scope.$apply();
+
+      var parent = scope.parent,
+          child = scope.child,
+          input = child.inputA;
+
+      expect(parent).toBeDefined();
+      expect(child).toBeDefined();
+      expect(parent.$error.required).toEqual([child]);
+      expect(child.$error.required).toEqual([input]);
+      expect(doc.hasClass('ng-invalid')).toBe(true);
+      expect(doc.hasClass('ng-invalid-required')).toBe(true);
+      expect(doc.find('div').hasClass('ng-invalid')).toBe(true);
+      expect(doc.find('div').hasClass('ng-invalid-required')).toBe(true);
+      doc.find('input').remove(); //remove child
+
+      expect(parent.$error.required).toBe(false);
+      expect(child.$error.required).toBe(false);
+      expect(doc.hasClass('ng-valid')).toBe(true);
+      expect(doc.hasClass('ng-valid-required')).toBe(true);
+      expect(doc.find('div').hasClass('ng-valid')).toBe(true);
+      expect(doc.find('div').hasClass('ng-valid-required')).toBe(true);
+    });
+
+
     it('should chain nested forms in repeater', function() {
       doc = jqLite(
          '<ng:form name=parent>' +