From 089c0f8b0e64f00e2629de07f35af03f01c34686 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Tue, 13 Mar 2012 14:09:31 -0700 Subject: fix(forms): fix nesting issues and add tests --- src/directive/form.js | 60 ++++++++++--------- test/directive/formSpec.js | 142 +++++++++++++++++++++++++++------------------ 2 files changed, 119 insertions(+), 83 deletions(-) diff --git a/src/directive/form.js b/src/directive/form.js index 8c172889..a1bfcd12 100644 --- a/src/directive/form.js +++ b/src/directive/form.js @@ -1,6 +1,12 @@ 'use strict'; +var nullFormCtrl = { + $addControl: noop, + $removeControl: noop, + $setValidity: noop +} + /** * @ngdoc object * @name angular.module.ng.$compileProvider.directive.form.FormController @@ -24,18 +30,23 @@ * of `FormController`. * */ -FormController.$inject = ['name', '$element']; -function FormController(name, element) { +FormController.$inject = ['name', '$element', '$attrs']; +function FormController(name, element, attrs) { var form = this, - parentForm = element.parent().inheritedData('$formController'), + parentForm = element.parent().inheritedData('$formController') || nullFormCtrl, errors = form.$error = {}; + // init state + form.$name = attrs.name; + form.$dirty = false; + form.$pristine = true; + form.$valid = true; + form.$invalid = false; + // publish the form into scope name(this); - if (parentForm) { - parentForm.$addControl(form); - } + parentForm.$addControl(form); form.$addControl = function(control) { if (control.$name && !form.hasOwnProperty(control.$name)) { @@ -71,26 +82,13 @@ function FormController(name, element) { form.$pristine = false; } - // init state - form.$dirty = false; - form.$pristine = true; - form.$valid = true; - form.$invalid = false; - function cleanupControlErrors(queue, validationToken, control) { if (queue) { control = control || this; // so that we can be used in forEach; - for (var i = 0, length = queue.length; i < length; i++) { - if (queue[i] === control) { - queue.splice(i, 1); - if (!queue.length) { - delete errors[validationToken]; - - if (parentForm) { - parentForm.$setValidity(validationToken, true, form); - } - } - } + arrayRemove(queue, control); + if (!queue.length) { + delete errors[validationToken]; + parentForm.$setValidity(validationToken, true, form); } } } @@ -98,13 +96,10 @@ function FormController(name, element) { function addControlError(validationToken, control) { var queue = errors[validationToken]; if (queue) { - if (indexOf(queue, control)) return; + if (includes(queue, control)) return; } else { errors[validationToken] = queue = []; - - if (parentForm) { - parentForm.$setValidity(validationToken, false, form); - } + parentForm.$setValidity(validationToken, false, form); } queue.push(control); } @@ -222,6 +217,15 @@ var formDirective = [function() { formElement[value ? 'addClass' : 'removeClass']('ng-' + name); }); }); + + var parentFormCtrl = formElement.parent().inheritedData('$formController'); + if (parentFormCtrl) { + formElement.bind('$destroy', function() { + parentFormCtrl.$removeControl(controller); + if (attr.name) delete scope[attr.name]; + extend(controller, nullFormCtrl); //stop propagating child destruction handlers upwards + }); + } } }; } diff --git a/test/directive/formSpec.js b/test/directive/formSpec.js index 41c90a89..fbcf4aec 100644 --- a/test/directive/formSpec.js +++ b/test/directive/formSpec.js @@ -94,29 +94,6 @@ describe('form', function() { }); - it('should chain nested forms', function() { - doc = jqLite( - '' + - '' + - '' + - '' + - ''); - $compile(doc)(scope); - - var parent = scope.parent; - var child = scope.child; - var input = child.inputA; - - input.$setValidity('MyError', false); - expect(parent.$error.MyError).toEqual([child]); - expect(child.$error.MyError).toEqual([input]); - - input.$setValidity('MyError', true); - expect(parent.$error.MyError).toBeUndefined(); - expect(child.$error.MyError).toBeUndefined(); - }); - - it('should support two forms on a single scope', function() { doc = $compile( '
' + @@ -152,38 +129,6 @@ describe('form', function() { }); - it('should chain nested forms in repeater', function() { - doc = jqLite( - '' + - '' + - '' + - '' + - ''); - $compile(doc)(scope); - - scope.$apply(function() { - scope.forms = [1]; - }); - - var parent = scope.parent; - var child = doc.find('input').scope().child; - var input = child.text; - - expect(parent).toBeDefined(); - expect(child).toBeDefined(); - expect(input).toBeDefined(); - - input.$setValidity('myRule', false); - expect(input.$error.myRule).toEqual(true); - expect(child.$error.myRule).toEqual([input]); - expect(parent.$error.myRule).toEqual([child]); - - input.$setValidity('myRule', true); - expect(parent.$error.myRule).toBeUndefined(); - expect(child.$error.myRule).toBeUndefined(); - }); - - it('should publish widgets', function() { doc = jqLite('
'); $compile(doc)(scope); @@ -197,6 +142,93 @@ describe('form', function() { }); + describe('nested forms', function() { + + it('should chain nested forms', function() { + doc = jqLite( + '' + + '' + + '' + + '' + + '' + + ''); + $compile(doc)(scope); + + var parent = scope.parent, + child = scope.child, + inputA = child.inputA, + inputB = child.inputB; + + inputA.$setValidity('MyError', false); + inputB.$setValidity('MyError', false); + expect(parent.$error.MyError).toEqual([child]); + expect(child.$error.MyError).toEqual([inputA, inputB]); + + inputA.$setValidity('MyError', true); + expect(parent.$error.MyError).toEqual([child]); + expect(child.$error.MyError).toEqual([inputB]); + + inputB.$setValidity('MyError', true); + expect(parent.$error.MyError).toBeUndefined(); + expect(child.$error.MyError).toBeUndefined(); + }); + + + it('should deregister a child form when its DOM is removed', function() { + doc = jqLite( + '' + + '' + + '' + + '' + + ''); + $compile(doc)(scope); + scope.$apply(); + + var parent = scope.parent, + child = scope.child; + + expect(parent.$error.required).toEqual([child]); + doc.children().remove(); //remove child + + expect(parent.child).toBeUndefined(); + expect(scope.child).toBeUndefined(); + expect(parent.$error.required).toBeUndefined(); + }); + + + it('should chain nested forms in repeater', function() { + doc = jqLite( + '' + + '' + + '' + + '' + + ''); + $compile(doc)(scope); + + scope.$apply(function() { + scope.forms = [1]; + }); + + var parent = scope.parent; + var child = doc.find('input').scope().child; + var input = child.text; + + expect(parent).toBeDefined(); + expect(child).toBeDefined(); + expect(input).toBeDefined(); + + input.$setValidity('myRule', false); + expect(input.$error.myRule).toEqual(true); + expect(child.$error.myRule).toEqual([input]); + expect(parent.$error.myRule).toEqual([child]); + + input.$setValidity('myRule', true); + expect(parent.$error.myRule).toBeUndefined(); + expect(child.$error.myRule).toBeUndefined(); + }); + }) + + describe('validation', function() { beforeEach(function() { -- cgit v1.2.3