Skip to content

Commit 089c0f8

Browse files
IgorMinarIgorMinar
committed
fix(forms): fix nesting issues and add tests
1 parent b6ae6e5 commit 089c0f8

File tree

2 files changed

+119
-83
lines changed

2 files changed

+119
-83
lines changed

‎src/directive/form.js

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
'use strict';
22

33

4+
var nullFormCtrl = {
5+
$addControl: noop,
6+
$removeControl: noop,
7+
$setValidity: noop
8+
}
9+
410
/**
511
* @ngdoc object
612
* @name angular.module.ng.$compileProvider.directive.form.FormController
@@ -24,18 +30,23 @@
2430
* of `FormController`.
2531
*
2632
*/
27-
FormController.$inject = ['name', '$element'];
28-
function FormController(name, element) {
33+
FormController.$inject = ['name', '$element', '$attrs'];
34+
function FormController(name, element, attrs) {
2935
var form = this,
30-
parentForm = element.parent().inheritedData('$formController'),
36+
parentForm = element.parent().inheritedData('$formController') || nullFormCtrl,
3137
errors = form.$error = {};
3238

39+
// init state
40+
form.$name = attrs.name;
41+
form.$dirty = false;
42+
form.$pristine = true;
43+
form.$valid = true;
44+
form.$invalid = false;
45+
3346
// publish the form into scope
3447
name(this);
3548

36-
if (parentForm) {
37-
parentForm.$addControl(form);
38-
}
49+
parentForm.$addControl(form);
3950

4051
form.$addControl = function(control) {
4152
if (control.$name && !form.hasOwnProperty(control.$name)) {
@@ -71,40 +82,24 @@ function FormController(name, element) {
7182
form.$pristine = false;
7283
}
7384

74-
// init state
75-
form.$dirty = false;
76-
form.$pristine = true;
77-
form.$valid = true;
78-
form.$invalid = false;
79-
8085
function cleanupControlErrors(queue, validationToken, control) {
8186
if (queue) {
8287
control = control || this; // so that we can be used in forEach;
83-
for (var i = 0, length = queue.length; i < length; i++) {
84-
if (queue[i] === control) {
85-
queue.splice(i, 1);
86-
if (!queue.length) {
87-
delete errors[validationToken];
88-
89-
if (parentForm) {
90-
parentForm.$setValidity(validationToken, true, form);
91-
}
92-
}
93-
}
88+
arrayRemove(queue, control);
89+
if (!queue.length) {
90+
delete errors[validationToken];
91+
parentForm.$setValidity(validationToken, true, form);
9492
}
9593
}
9694
}
9795

9896
function addControlError(validationToken, control) {
9997
var queue = errors[validationToken];
10098
if (queue) {
101-
if (indexOf(queue, control)) return;
99+
if (includes(queue, control)) return;
102100
} else {
103101
errors[validationToken] = queue = [];
104-
105-
if (parentForm) {
106-
parentForm.$setValidity(validationToken, false, form);
107-
}
102+
parentForm.$setValidity(validationToken, false, form);
108103
}
109104
queue.push(control);
110105
}
@@ -222,6 +217,15 @@ var formDirective = [function() {
222217
formElement[value ? 'addClass' : 'removeClass']('ng-' + name);
223218
});
224219
});
220+
221+
var parentFormCtrl = formElement.parent().inheritedData('$formController');
222+
if (parentFormCtrl) {
223+
formElement.bind('$destroy', function() {
224+
parentFormCtrl.$removeControl(controller);
225+
if (attr.name) delete scope[attr.name];
226+
extend(controller, nullFormCtrl); //stop propagating child destruction handlers upwards
227+
});
228+
}
225229
}
226230
};
227231
}

‎test/directive/formSpec.js

Lines changed: 87 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -94,29 +94,6 @@ describe('form', function() {
9494
});
9595

9696

97-
it('should chain nested forms', function() {
98-
doc = jqLite(
99-
'<ng:form name="parent">' +
100-
'<ng:form name="child">' +
101-
'<input ng:model="modelA" name="inputA">' +
102-
'</ng:form>' +
103-
'</ng:form>');
104-
$compile(doc)(scope);
105-
106-
var parent = scope.parent;
107-
var child = scope.child;
108-
var input = child.inputA;
109-
110-
input.$setValidity('MyError', false);
111-
expect(parent.$error.MyError).toEqual([child]);
112-
expect(child.$error.MyError).toEqual([input]);
113-
114-
input.$setValidity('MyError', true);
115-
expect(parent.$error.MyError).toBeUndefined();
116-
expect(child.$error.MyError).toBeUndefined();
117-
});
118-
119-
12097
it('should support two forms on a single scope', function() {
12198
doc = $compile(
12299
'<div>' +
@@ -152,38 +129,6 @@ describe('form', function() {
152129
});
153130

154131

155-
it('should chain nested forms in repeater', function() {
156-
doc = jqLite(
157-
'<ng:form name=parent>' +
158-
'<ng:form ng:repeat="f in forms" name=child>' +
159-
'<input type=text ng:model=text name=text>' +
160-
'</ng:form>' +
161-
'</ng:form>');
162-
$compile(doc)(scope);
163-
164-
scope.$apply(function() {
165-
scope.forms = [1];
166-
});
167-
168-
var parent = scope.parent;
169-
var child = doc.find('input').scope().child;
170-
var input = child.text;
171-
172-
expect(parent).toBeDefined();
173-
expect(child).toBeDefined();
174-
expect(input).toBeDefined();
175-
176-
input.$setValidity('myRule', false);
177-
expect(input.$error.myRule).toEqual(true);
178-
expect(child.$error.myRule).toEqual([input]);
179-
expect(parent.$error.myRule).toEqual([child]);
180-
181-
input.$setValidity('myRule', true);
182-
expect(parent.$error.myRule).toBeUndefined();
183-
expect(child.$error.myRule).toBeUndefined();
184-
});
185-
186-
187132
it('should publish widgets', function() {
188133
doc = jqLite('<form name="form"><input type="text" name="w1" ng-model="some" /></form>');
189134
$compile(doc)(scope);
@@ -197,6 +142,93 @@ describe('form', function() {
197142
});
198143

199144

145+
describe('nested forms', function() {
146+
147+
it('should chain nested forms', function() {
148+
doc = jqLite(
149+
'<ng:form name="parent">' +
150+
'<ng:form name="child">' +
151+
'<input ng:model="modelA" name="inputA">' +
152+
'<input ng:model="modelB" name="inputB">' +
153+
'</ng:form>' +
154+
'</ng:form>');
155+
$compile(doc)(scope);
156+
157+
var parent = scope.parent,
158+
child = scope.child,
159+
inputA = child.inputA,
160+
inputB = child.inputB;
161+
162+
inputA.$setValidity('MyError', false);
163+
inputB.$setValidity('MyError', false);
164+
expect(parent.$error.MyError).toEqual([child]);
165+
expect(child.$error.MyError).toEqual([inputA, inputB]);
166+
167+
inputA.$setValidity('MyError', true);
168+
expect(parent.$error.MyError).toEqual([child]);
169+
expect(child.$error.MyError).toEqual([inputB]);
170+
171+
inputB.$setValidity('MyError', true);
172+
expect(parent.$error.MyError).toBeUndefined();
173+
expect(child.$error.MyError).toBeUndefined();
174+
});
175+
176+
177+
it('should deregister a child form when its DOM is removed', function() {
178+
doc = jqLite(
179+
'<ng:form name="parent">' +
180+
'<ng:form name="child">' +
181+
'<input ng:model="modelA" name="inputA" required>' +
182+
'</ng:form>' +
183+
'</ng:form>');
184+
$compile(doc)(scope);
185+
scope.$apply();
186+
187+
var parent = scope.parent,
188+
child = scope.child;
189+
190+
expect(parent.$error.required).toEqual([child]);
191+
doc.children().remove(); //remove child
192+
193+
expect(parent.child).toBeUndefined();
194+
expect(scope.child).toBeUndefined();
195+
expect(parent.$error.required).toBeUndefined();
196+
});
197+
198+
199+
it('should chain nested forms in repeater', function() {
200+
doc = jqLite(
201+
'<ng:form name=parent>' +
202+
'<ng:form ng:repeat="f in forms" name=child>' +
203+
'<input type=text ng:model=text name=text>' +
204+
'</ng:form>' +
205+
'</ng:form>');
206+
$compile(doc)(scope);
207+
208+
scope.$apply(function() {
209+
scope.forms = [1];
210+
});
211+
212+
var parent = scope.parent;
213+
var child = doc.find('input').scope().child;
214+
var input = child.text;
215+
216+
expect(parent).toBeDefined();
217+
expect(child).toBeDefined();
218+
expect(input).toBeDefined();
219+
220+
input.$setValidity('myRule', false);
221+
expect(input.$error.myRule).toEqual(true);
222+
expect(child.$error.myRule).toEqual([input]);
223+
expect(parent.$error.myRule).toEqual([child]);
224+
225+
input.$setValidity('myRule', true);
226+
expect(parent.$error.myRule).toBeUndefined();
227+
expect(child.$error.myRule).toBeUndefined();
228+
});
229+
})
230+
231+
200232
describe('validation', function() {
201233

202234
beforeEach(function() {

0 commit comments

Comments
 (0)