Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
feat(input): support dynamic element validation
Browse files Browse the repository at this point in the history
Interpolates the form and form control attribute name, so that dynamic form controls (such as those
rendered in an ngRepeat) will always have their expected interpolated name.

The control will be present in its parent form controller with the interpolated property name, and
this name can change when the interpolated value changes.

Closes #4791
Closes #1404
  • Loading branch information
caitp committed Sep 23, 2014
1 parent dc3de7f commit 729c238
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 10 deletions.
38 changes: 31 additions & 7 deletions src/ng/directive/form.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
var nullFormCtrl = {
$addControl: noop,
$$renameControl: nullFormRenameControl,
$removeControl: noop,
$setValidity: noop,
$$setPending: noop,
Expand All @@ -14,6 +15,10 @@ var nullFormCtrl = {
},
SUBMITTED_CLASS = 'ng-submitted';

function nullFormRenameControl(control, name) {
control.$name = name;
}

/**
* @ngdoc type
* @name form.FormController
Expand Down Expand Up @@ -51,17 +56,18 @@ SUBMITTED_CLASS = 'ng-submitted';
*
*/
//asks for $scope to fool the BC controller module
FormController.$inject = ['$element', '$attrs', '$scope', '$animate'];
function FormController(element, attrs, $scope, $animate) {
FormController.$inject = ['$element', '$attrs', '$scope', '$animate', '$interpolate'];
function FormController(element, attrs, $scope, $animate, $interpolate) {
var form = this,
parentForm = element.parent().controller('form') || nullFormCtrl,
controls = [];

var parentForm = form.$$parentForm = element.parent().controller('form') || nullFormCtrl;

// init state
form.$error = {};
form.$$success = {};
form.$pending = undefined;
form.$name = attrs.name || attrs.ngForm;
form.$name = $interpolate(attrs.name || attrs.ngForm || '')($scope);

This comment has been minimized.

Copy link
@thorn0

thorn0 Sep 24, 2014

Contributor

Aren't the values in the attributes object already interpolated? Why call $interpolate here?
Oh, now I see that during controller initialization they're not interpolated yet. Is it documented?

This comment has been minimized.

Copy link
@caitp

caitp Sep 24, 2014

Author Contributor

attribute interpolation happens at priority 100 (which is documented). We don't necessarily need to put it here, because not long after, the $observe will fire. However this gets us a bit of an early start.

form.$dirty = false;
form.$pristine = true;
form.$valid = true;
Expand Down Expand Up @@ -127,6 +133,17 @@ function FormController(element, attrs, $scope, $animate) {
}
};

// Private API: rename a form control
form.$$renameControl = function(control, newName) {
var oldName = control.$name;

if (form[oldName] === control) {
delete form[oldName];
}
form[newName] = control;
control.$name = newName;
};

/**
* @ngdoc method
* @name form.FormController#$removeControl
Expand Down Expand Up @@ -466,13 +483,20 @@ var formDirectiveFactory = function(isNgForm) {
});
}

var parentFormCtrl = formElement.parent().controller('form'),
alias = attr.name || attr.ngForm;
var parentFormCtrl = controller.$$parentForm,
alias = controller.$name;

if (alias) {
setter(scope, alias, controller, alias);
attr.$observe(attr.name ? 'name' : 'ngForm', function(newValue) {
if (alias === newValue) return;
setter(scope, alias, undefined, alias);
alias = newValue;
setter(scope, alias, controller, alias);
parentFormCtrl.$$renameControl(controller, alias);
});

This comment has been minimized.

Copy link
@dragosrususv

dragosrususv Sep 26, 2014

Hey @caitp ,

Nice PR.
Still, PERF wise, adding an observe on EACH form or input for a below 0.1% case scenario doesn't seem like the best option. Is there any way we could add a parameter to disable this $observe with an AngularJS configuration parameter when we load our app at config phase?

Have a wonderful day!

Kind regards,
Dragos

This comment has been minimized.

Copy link
@caitp

caitp Sep 26, 2014

Author Contributor

observe doesn't really have an effect on performance... a watcher is only set up if the attribute is interpolated (compiled with a value containing interpolated expression) (this happens regardless of whether or not $observe is used). If you don't use interpolated names, you won't see extra watchers added causing digest times to increase

This comment has been minimized.

Copy link
@dragosrususv

dragosrususv Sep 26, 2014

Ouh, only for interpolated expression :) Sorry, my head needs an upgrade, didn't $compile() that :)

}
if (parentFormCtrl) {
if (parentFormCtrl !== nullFormCtrl) {
formElement.on('$destroy', function() {
parentFormCtrl.$removeControl(controller);
if (alias) {
Expand Down
12 changes: 9 additions & 3 deletions src/ng/directive/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -1657,8 +1657,8 @@ var VALID_CLASS = 'ng-valid',
*
*
*/
var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$parse', '$animate', '$timeout', '$rootScope', '$q',
function($scope, $exceptionHandler, $attr, $element, $parse, $animate, $timeout, $rootScope, $q) {
var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$parse', '$animate', '$timeout', '$rootScope', '$q', '$interpolate',
function($scope, $exceptionHandler, $attr, $element, $parse, $animate, $timeout, $rootScope, $q, $interpolate) {
this.$viewValue = Number.NaN;
this.$modelValue = Number.NaN;
this.$validators = {};
Expand All @@ -1675,7 +1675,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
this.$error = {}; // keep invalid keys here
this.$$success = {}; // keep valid keys here
this.$pending = undefined; // keep pending keys here
this.$name = $attr.name;
this.$name = $interpolate($attr.name || '', false)($scope);


var parsedNgModel = $parse($attr.ngModel),
Expand Down Expand Up @@ -2387,6 +2387,12 @@ var ngModelDirective = function() {
// notify others, especially parent forms
formCtrl.$addControl(modelCtrl);

attr.$observe('name', function(newValue) {
if (modelCtrl.$name !== newValue) {
formCtrl.$$renameControl(modelCtrl, newValue);
}
});

scope.$on('$destroy', function() {
formCtrl.$removeControl(modelCtrl);
});
Expand Down
51 changes: 51 additions & 0 deletions test/ng/directive/formSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,57 @@ describe('form', function() {
});
});


it('should rename nested form controls when interpolated name changes', function() {
scope.idA = 'A';
scope.idB = 'X';

doc = $compile(
'<form name="form">' +
'<div ng-form="nested{{idA}}">' +
'<div ng-form name="nested{{idB}}"' +
'</div>' +
'</div>' +
'</form'
)(scope);

scope.$digest();
var formA = scope.form.nestedA;
expect(formA).toBeDefined();
expect(formA.$name).toBe('nestedA');

var formX = formA.nestedX;
expect(formX).toBeDefined();
expect(formX.$name).toBe('nestedX');

scope.idA = 'B';
scope.idB = 'Y';
scope.$digest();

expect(scope.form.nestedA).toBeUndefined();
expect(scope.form.nestedB).toBe(formA);
expect(formA.nestedX).toBeUndefined();
expect(formA.nestedY).toBe(formX);
});


it('should rename forms with no parent when interpolated name changes', function() {
var element = $compile('<form name="name{{nameID}}"></form>')(scope);
var element2 = $compile('<div ng-form="name{{nameID}}"></div>')(scope);
scope.nameID = "A";
scope.$digest();
var form = element.controller('form');
var form2 = element2.controller('form');
expect(form.$name).toBe('nameA');
expect(form2.$name).toBe('nameA');

scope.nameID = "B";
scope.$digest();
expect(form.$name).toBe('nameB');
expect(form2.$name).toBe('nameB');
});


describe('$setSubmitted', function() {
beforeEach(function() {
doc = $compile(
Expand Down
36 changes: 36 additions & 0 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,42 @@ describe('input', function() {
}
}));


it('should interpolate input names', function() {
scope.nameID = '47';
compileInput('<input type="text" ng-model="name" name="name{{nameID}}" />');
expect(scope.form.name47.$pristine).toBeTruthy();
changeInputValueTo('caitp');
expect(scope.form.name47.$dirty).toBeTruthy();
});


it('should rename form controls in form when interpolated name changes', function() {
scope.nameID = "A";
compileInput('<input type="text" ng-model="name" name="name{{nameID}}" />');
expect(scope.form.nameA.$name).toBe('nameA');
var oldModel = scope.form.nameA;
scope.nameID = "B";
scope.$digest();
expect(scope.form.nameA).toBeUndefined();
expect(scope.form.nameB).toBe(oldModel);
expect(scope.form.nameB.$name).toBe('nameB');
});


it('should rename form controls in null form when interpolated name changes', function() {
var element = $compile('<input type="text" ng-model="name" name="name{{nameID}}" />')(scope);
scope.nameID = "A";
scope.$digest();
var model = element.controller('ngModel');
expect(model.$name).toBe('nameA');

scope.nameID = "B";
scope.$digest();
expect(model.$name).toBe('nameB');
});


describe('"change" event', function() {
function assertBrowserSupportsChangeEvent(inputEventSupported) {
// Force browser to report a lack of an 'input' event
Expand Down
27 changes: 27 additions & 0 deletions test/ng/directive/selectSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,33 @@ describe('select', function() {
});


it('should interpolate select names', function() {
scope.robots = ['c3p0', 'r2d2'];
scope.name = 'r2d2';
scope.nameID = 47;
compile('<select ng-model="name" name="name{{nameID}}">' +
'<option ng-repeat="r in robots">{{r}}</option>' +
'</select>');
expect(scope.form.name47.$pristine).toBeTruthy();
browserTrigger(element.find('option').eq(0));
expect(scope.form.name47.$dirty).toBeTruthy();
expect(scope.name).toBe('c3p0');
});


it('should rename select controls in form when interpolated name changes', function() {
scope.nameID = "A";
compile('<select ng-model="name" name="name{{nameID}}"></select>');
expect(scope.form.nameA.$name).toBe('nameA');
var oldModel = scope.form.nameA;
scope.nameID = "B";
scope.$digest();
expect(scope.form.nameA).toBeUndefined();
expect(scope.form.nameB).toBe(oldModel);
expect(scope.form.nameB.$name).toBe('nameB');
});


describe('empty option', function() {

it('should select the empty option when model is undefined', function() {
Expand Down

0 comments on commit 729c238

Please sign in to comment.