Skip to content

Commit c6110e8

Browse files
NarretzNarretz
committed
fix(form, ngModel): correctly notify parent form when children are added
Test that re-added controls propagate validity changes to the parent form. Ensure that when a form / control that was removed and then attached to a different parent, is renamed / deleted, the new parent will be notified of the change. Document that dynamic adding / removing of controls may require manually propagating the current state of the control to the parent form.
1 parent 290b504 commit c6110e8

3 files changed

Lines changed: 218 additions & 6 deletions

File tree

‎src/ng/directive/form.js‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,23 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
114114
/**
115115
* @ngdoc method
116116
* @name form.FormController#$addControl
117+
* @param {object} control control object, either a {@link form.FormController} or an
118+
* {@link ngModel.NgModelController}
117119
*
118120
* @description
119-
* Register a control with the form.
121+
* Register a control with the form. Input elements using ngModelController do this automatically
122+
* when they are linked.
120123
*
121-
* Input elements using ngModelController do this automatically when they are linked.
124+
* Note that the current state of the control will not be reflected on the new parent form. This
125+
* is not an issue with normal use, as freshly compiled and linked controls are in a `$pristine`
126+
* state.
127+
*
128+
* However, if the method is used programmatically, for example by adding dynamically created controls,
129+
* or controls that have been previously removed without destroying their corresponding DOM element,
130+
* it's the developers responsiblity to make sure the current state propagates to the parent form.
131+
*
132+
* For example, if an input control is added that is already `$dirty` and has `$error` properties,
133+
* calling `$setDirty()` and `$validate()` afterwards will propagate the state to the parent form.
122134
*/
123135
form.$addControl = function(control) {
124136
// Breaking change - before, inputs whose name was "hasOwnProperty" were quietly ignored
@@ -147,11 +159,18 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
147159
/**
148160
* @ngdoc method
149161
* @name form.FormController#$removeControl
162+
* @param {object} control control object, either a {@link form.FormController} or an
163+
* {@link ngModel.NgModelController}
150164
*
151165
* @description
152166
* Deregister a control from the form.
153167
*
154168
* Input elements using ngModelController do this automatically when they are destroyed.
169+
*
170+
* Note that only the removed control's validation state (`$errors`etc.) will be removed from the
171+
* form. `$dirty`, `$submitted` states will not be changed, because the expected behavior can be
172+
* different from case to case. For example, removing the only `$dirty` control from a form may or
173+
* may not mean that the form is still `$dirty`.
155174
*/
156175
form.$removeControl = function(control) {
157176
if (control.$name && form[control.$name] === control) {
@@ -503,13 +522,13 @@ var formDirectiveFactory = function(isNgForm) {
503522
attr.$observe(nameAttr, function(newValue) {
504523
if (controller.$name === newValue) return;
505524
setter(scope, undefined);
506-
parentFormCtrl.$$renameControl(controller, newValue);
525+
controller.$$parentForm.$$renameControl(controller, newValue);
507526
setter = getSetter(controller.$name);
508527
setter(scope, controller);
509528
});
510529
}
511530
formElement.on('$destroy', function() {
512-
parentFormCtrl.$removeControl(controller);
531+
controller.$$parentForm.$removeControl(controller);
513532
setter(scope, undefined);
514533
extend(controller, nullFormCtrl); //stop propagating child destruction handlers upwards
515534
});

‎src/ng/directive/ngModel.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,12 +1038,12 @@ var ngModelDirective = ['$rootScope', function($rootScope) {
10381038

10391039
attr.$observe('name', function(newValue) {
10401040
if (modelCtrl.$name !== newValue) {
1041-
formCtrl.$$renameControl(modelCtrl, newValue);
1041+
modelCtrl.$$parentForm.$$renameControl(modelCtrl, newValue);
10421042
}
10431043
});
10441044

10451045
scope.$on('$destroy', function() {
1046-
formCtrl.$removeControl(modelCtrl);
1046+
modelCtrl.$$parentForm.$removeControl(modelCtrl);
10471047
});
10481048
},
10491049
post: function ngModelPostLink(scope, element, attr, ctrls) {

‎test/ng/directive/formSpec.js‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,82 @@ describe('form', function() {
9595
});
9696

9797

98+
it('should react to validation changes in manually added controls', function() {
99+
doc = $compile(
100+
'<form name="myForm">' +
101+
'<input name="control" ng-maxlength="1" ng-model="value" store-model-ctrl/>' +
102+
'</form>')(scope);
103+
104+
scope.$digest();
105+
106+
var form = scope.myForm;
107+
108+
var input = doc.find('input').eq(0);
109+
110+
// remove control and invalidate it
111+
form.$removeControl(control);
112+
expect(form.control).toBeUndefined();
113+
114+
changeInputValue(input, 'abc');
115+
expect(control.$error.maxlength).toBe(true);
116+
expect(control.$dirty).toBe(true);
117+
expect(form.$error.maxlength).toBeFalsy();
118+
expect(form.$dirty).toBe(false);
119+
120+
// re-add the control; its current validation state is not propagated
121+
form.$addControl(control);
122+
expect(form.control).toBe(control);
123+
expect(form.$error.maxlength).toBeFalsy();
124+
expect(form.$dirty).toBe(false);
125+
126+
// Only when the input changes again its validation state is propagated
127+
changeInputValue(input, 'abcd');
128+
expect(form.$error.maxlength[0]).toBe(control);
129+
expect(form.$dirty).toBe(false);
130+
});
131+
132+
133+
it('should use the correct parent when renaming and removing dynamically added controls', function() {
134+
scope.controlName = 'childControl';
135+
scope.hasChildControl = true;
136+
137+
doc = $compile(
138+
'<form name="myForm">' +
139+
'<div ng-if="hasChildControl">' +
140+
'<input name="{{controlName}}" ng-maxlength="1" ng-model="value"/>' +
141+
'</div>' +
142+
'</form>' +
143+
'<form name="otherForm"></form>')(scope);
144+
145+
scope.$digest();
146+
147+
var form = scope.myForm;
148+
var otherForm = scope.otherForm;
149+
var childControl = form.childControl;
150+
151+
// remove child form and add it to another form
152+
form.$removeControl(childControl);
153+
otherForm.$addControl(childControl);
154+
155+
expect(form.childControl).toBeUndefined();
156+
expect(otherForm.childControl).toBe(childControl);
157+
158+
// rename the childControl
159+
scope.controlName = 'childControlMoved';
160+
scope.$digest();
161+
162+
expect(form.childControlMoved).toBeUndefined();
163+
expect(otherForm.childControl).toBeUndefined();
164+
expect(otherForm.childControlMoved).toBe(childControl);
165+
166+
scope.hasChildControl = false;
167+
scope.$digest();
168+
169+
expect(form.childControlMoved).toBeUndefined();
170+
expect(otherForm.childControlMoved).toBeUndefined();
171+
});
172+
173+
98174
it('should remove scope reference when form with no parent form is removed from the DOM', function() {
99175
var formController;
100176
scope.ctrl = {};
@@ -614,6 +690,123 @@ describe('form', function() {
614690
expect(child.$error.required).toEqual([inputB]);
615691
});
616692

693+
694+
it('should ignore changes in manually removed child forms', function() {
695+
doc = $compile(
696+
'<form name="myForm">' +
697+
'<ng-form name="childform">' +
698+
'<input name="childformcontrol" ng-maxlength="1" ng-model="value"/>' +
699+
'</ng-form>' +
700+
'</form>')(scope);
701+
702+
var form = scope.myForm;
703+
var childformController = doc.find('ng-form').eq(0).controller('form');
704+
705+
var input = doc.find('input').eq(0);
706+
var inputController = input.controller('ngModel');
707+
708+
changeInputValue(input, 'ab');
709+
scope.$apply();
710+
711+
expect(form.$dirty).toBe(true);
712+
expect(form.$error.maxlength).toBeTruthy();
713+
expect(form.$error.maxlength[0].$name).toBe('childform');
714+
715+
inputController.$setPristine();
716+
expect(form.$dirty).toBe(true);
717+
718+
form.$setPristine();
719+
720+
// remove child form
721+
form.$removeControl(childformController);
722+
expect(form.childform).toBeUndefined();
723+
expect(form.$error.maxlength).toBeFalsy();
724+
725+
changeInputValue(input, 'abc');
726+
scope.$apply();
727+
728+
expect(form.$error.maxlength).toBeFalsy();
729+
expect(form.$dirty).toBe(false);
730+
});
731+
732+
733+
it('should react to changes in manually added child forms', function() {
734+
doc = $compile(
735+
'<form name="myForm">' +
736+
'<ng-form name="childForm">' +
737+
'<input name="childformcontrol" ng-maxlength="1" ng-model="value" />' +
738+
'</ng-form>' +
739+
'</form>')(scope);
740+
741+
var form = scope.myForm;
742+
var childFormController = doc.find('ng-form').eq(0).controller('form');
743+
744+
var input = doc.find('input').eq(0);
745+
746+
// remove child form so we can add it manually
747+
form.$removeControl(childFormController);
748+
changeInputValue(input, 'ab');
749+
750+
expect(form.childForm).toBeUndefined();
751+
expect(form.$dirty).toBe(false);
752+
expect(form.$error.maxlength).toBeFalsy();
753+
754+
// re-add the child form; its current validation state is not propagated
755+
form.$addControl(childFormController);
756+
expect(form.childForm).toBe(childFormController);
757+
expect(form.$error.maxlength).toBeFalsy();
758+
expect(form.$dirty).toBe(false);
759+
760+
// Only when the input inside the child form changes, the validation state is propagated
761+
changeInputValue(input, 'abc');
762+
expect(form.$error.maxlength[0]).toBe(childFormController);
763+
expect(form.$dirty).toBe(false);
764+
});
765+
766+
767+
it('should use the correct parent when renaming and removing dynamically added forms', function() {
768+
scope.formName = 'childForm';
769+
scope.hasChildForm = true;
770+
771+
doc = $compile(
772+
'<form name="myForm">' +
773+
'<div ng-if="hasChildForm">' +
774+
'<ng-form name="{{formName}}">' +
775+
'<input name="childformcontrol" ng-maxlength="1" ng-model="value"/>' +
776+
'</ng-form>' +
777+
'</div>' +
778+
'</form>' +
779+
'<form name="otherForm"></form>')(scope);
780+
781+
scope.$digest();
782+
783+
var form = scope.myForm;
784+
var otherForm = scope.otherForm;
785+
var childForm = form.childForm;
786+
787+
// remove child form and add it to another form
788+
form.$removeControl(childForm);
789+
otherForm.$addControl(childForm);
790+
791+
expect(form.childForm).toBeUndefined();
792+
expect(otherForm.childForm).toBe(childForm);
793+
794+
// rename the childForm
795+
scope.formName = 'childFormMoved';
796+
scope.$digest();
797+
798+
expect(form.childFormMoved).toBeUndefined();
799+
expect(otherForm.childForm).toBeUndefined();
800+
expect(otherForm.childFormMoved).toBe(childForm);
801+
802+
scope.hasChildForm = false;
803+
scope.$digest();
804+
805+
expect(form.childFormMoved).toBeUndefined();
806+
expect(otherForm.childFormMoved).toBeUndefined();
807+
});
808+
809+
617810
it('should chain nested forms in repeater', function() {
618811
doc = jqLite(
619812
'<ng:form name=parent>' +

0 commit comments

Comments
 (0)