fix(forms): remove the need for extra form scope

the forms/controls code refactored not to depend on events which forced
us to create new scope for each form element.
This commit is contained in:
Igor Minar 2012-03-12 15:54:48 -07:00
parent 9134f5ce5a
commit 5e6ba25201
4 changed files with 192 additions and 119 deletions

View file

@ -22,49 +22,52 @@
* of `FormController`.
*
*/
FormController.$inject = ['$scope', 'name'];
function FormController($scope, name) {
FormController.$inject = ['$scope', 'name', '$element'];
function FormController($scope, name, element) {
var form = this,
parentForm = element.parent().inheritedData('$formController'),
errors = form.error = {};
// publish the form into scope
name(this);
$scope.$on('$destroy', function(event, widget) {
if (!widget) return;
if (parentForm) {
parentForm.$addControl(form);
}
if (widget.widgetId && form[widget.widgetId] === widget) {
delete form[widget.widgetId];
form.$addControl = function(control) {
if (control.name && !form.hasOwnProperty(control.name)) {
form[control.name] = control;
}
forEach(errors, removeWidget, widget);
});
}
$scope.$on('$valid', function(event, error, widget) {
removeWidget(errors[error], error, widget);
if (equals(errors, {})) {
form.valid = true;
form.invalid = false;
form.$removeControl = function(control) {
if (control.name && form[control.name] === control) {
delete form[control.name];
}
});
forEach(errors, cleanupControlErrors, control);
};
$scope.$on('$invalid', function(event, error, widget) {
addWidget(error, widget);
form.$setValidity = function(validationToken, isValid, control) {
if (isValid) {
cleanupControlErrors(errors[validationToken], validationToken, control);
form.valid = false;
form.invalid = true;
});
if (equals(errors, {})) {
form.valid = true;
form.invalid = false;
}
} else {
addControlError(validationToken, control);
$scope.$on('$viewTouch', function() {
form.valid = false;
form.invalid = true;
}
};
form.$setDirty = function() {
form.dirty = true;
form.pristine = false;
});
$scope.$on('$newFormControl', function(event, widget) {
if (widget.widgetId && !form.hasOwnProperty(widget.widgetId)) {
form[widget.widgetId] = widget;
}
});
}
// init state
form.dirty = false;
@ -72,32 +75,40 @@ function FormController($scope, name) {
form.valid = true;
form.invalid = false;
function removeWidget(queue, errorKey, widget) {
function cleanupControlErrors(queue, validationToken, control) {
if (queue) {
widget = widget || this; // so that we can be used in forEach;
control = control || this; // so that we can be used in forEach;
for (var i = 0, length = queue.length; i < length; i++) {
if (queue[i] === widget) {
if (queue[i] === control) {
queue.splice(i, 1);
if (!queue.length) {
delete errors[errorKey];
delete errors[validationToken];
if (parentForm) {
parentForm.$setValidity(validationToken, true, form);
}
}
}
}
}
}
function addWidget(errorKey, widget) {
var queue = errors[errorKey];
function addControlError(validationToken, control) {
var queue = errors[validationToken];
if (queue) {
for (var i = 0, length = queue.length; i < length; i++) {
if (queue[i] === widget) {
if (queue[i] === control) {
return;
}
}
} else {
errors[errorKey] = queue = [];
errors[validationToken] = queue = [];
if (parentForm) {
parentForm.$setValidity(validationToken, false, form);
}
}
queue.push(widget);
queue.push(control);
}
}
@ -107,12 +118,12 @@ function FormController($scope, name) {
* @name angular.module.ng.$compileProvider.directive.form
* @restrict EA
*
* @scope
* @description
* Directive that instantiates
* {@link angular.module.ng.$compileProvider.directive.form.FormController FormController}.
*
* If `name` attribute is specified, the controller is published to the scope as well.
* If `name` attribute is specified, the form controller is published onto the current scope under
* this name.
*
* # Alias: `ng-form`
*
@ -164,28 +175,28 @@ function FormController($scope, name) {
<doc:source>
<script>
function Ctrl($scope) {
$scope.text = 'guest';
$scope.userType = 'guest';
}
</script>
<form name="myForm" ng-controller="Ctrl">
text: <input type="text" name="input" ng-model="text" required>
<span class="error" ng-show="myForm.input.error.REQUIRED">Required!</span>
<tt>text = {{text}}</tt><br/>
<tt>myForm.input.valid = {{myForm.input.valid}}</tt><br/>
<tt>myForm.input.error = {{myForm.input.error}}</tt><br/>
<tt>myForm.valid = {{myForm.valid}}</tt><br/>
<tt>myForm.error.REQUIRED = {{!!myForm.error.REQUIRED}}</tt><br/>
userType: <input name="input" ng-model="userType" required>
<span class="error" ng-show="myForm.input.error.REQUIRED">Required!</span><br>
<tt>userType = {{userType}}</tt><br>
<tt>myForm.input.valid = {{myForm.input.valid}}</tt><br>
<tt>myForm.input.error = {{myForm.input.error}}</tt><br>
<tt>myForm.valid = {{myForm.valid}}</tt><br>
<tt>myForm.error.REQUIRED = {{!!myForm.error.REQUIRED}}</tt><br>
</form>
</doc:source>
<doc:scenario>
it('should initialize to model', function() {
expect(binding('text')).toEqual('guest');
expect(binding('userType')).toEqual('guest');
expect(binding('myForm.input.valid')).toEqual('true');
});
it('should be invalid if empty', function() {
input('text').enter('');
expect(binding('text')).toEqual('');
input('userType').enter('');
expect(binding('userType')).toEqual('');
expect(binding('myForm.input.valid')).toEqual('false');
});
</doc:scenario>
@ -195,7 +206,6 @@ var formDirective = [function() {
return {
name: 'form',
restrict: 'E',
scope: true,
inject: {
name: 'accessor'
},
@ -203,7 +213,6 @@ var formDirective = [function() {
compile: function() {
return {
pre: function(scope, formElement, attr, controller) {
formElement.data('$form', controller);
formElement.bind('submit', function(event) {
if (!attr.action) event.preventDefault();
});

View file

@ -131,7 +131,7 @@ var inputType = {
it('should be invalid if over max', function() {
input('value').enter('123');
expect(binding('value')).toEqual('12');
expect(binding('value')).toEqual('');
expect(binding('myForm.input.valid')).toEqual('false');
});
</doc:scenario>
@ -382,7 +382,7 @@ function textInputType(scope, element, attr, ctrl) {
var pattern = attr.ngPattern,
patternValidator;
var emit = function(regexp, value) {
var validate = function(regexp, value) {
if (isEmpty(value) || regexp.test(value)) {
ctrl.setValidity('PATTERN', true);
return value;
@ -396,7 +396,7 @@ function textInputType(scope, element, attr, ctrl) {
if (pattern.match(/^\/(.*)\/$/)) {
pattern = new RegExp(pattern.substr(1, pattern.length - 2));
patternValidator = function(value) {
return emit(pattern, value)
return validate(pattern, value)
};
} else {
patternValidator = function(value) {
@ -405,7 +405,7 @@ function textInputType(scope, element, attr, ctrl) {
if (!patternObj || !patternObj.test) {
throw new Error('Expected ' + pattern + ' to be a RegExp but was ' + patternObj);
}
return emit(patternObj, value);
return validate(patternObj, value);
};
}
@ -676,7 +676,7 @@ function checkboxInputType(scope, element, attr, ctrl) {
it('should be invalid if empty when required', function() {
input('user.name').enter('');
expect(binding('user')).toEqual('{"last":"visitor","name":null}');
expect(binding('user')).toEqual('{"last":"visitor"}');
expect(binding('myForm.userName.valid')).toEqual('false');
expect(binding('myForm.valid')).toEqual('false');
});
@ -690,16 +690,16 @@ function checkboxInputType(scope, element, attr, ctrl) {
it('should be invalid if less than required min length', function() {
input('user.last').enter('xx');
expect(binding('user')).toEqual('{"last":"visitor","name":"guest"}');
expect(binding('user')).toEqual('{"name":"guest"}');
expect(binding('myForm.lastName.valid')).toEqual('false');
expect(binding('myForm.lastName.error')).toMatch(/MINLENGTH/);
expect(binding('myForm.valid')).toEqual('false');
});
it('should be valid if longer than max length', function() {
it('should be invalid if longer than max length', function() {
input('user.last').enter('some ridiculously long name');
expect(binding('user'))
.toEqual('{"last":"visitor","name":"guest"}');
.toEqual('{"name":"guest"}');
expect(binding('myForm.lastName.valid')).toEqual('false');
expect(binding('myForm.lastName.error')).toMatch(/MAXLENGTH/);
expect(binding('myForm.valid')).toEqual('false');
@ -748,13 +748,14 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', 'ngModel',
this.modelValue = Number.NaN;
this.parsers = [];
this.formatters = [];
this.viewChangeListeners = [];
this.error = {};
this.pristine = true;
this.dirty = false;
this.valid = true;
this.invalid = false;
this.render = noop;
this.widgetId = $attr.name;
this.name = $attr.name;
/**
@ -763,32 +764,34 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', 'ngModel',
* @methodOf angular.module.ng.$compileProvider.directive.ng-model.NgModelController
*
* @description
* Change the validity state, and notifies the form when the widget changes validity. (i.e. does
* not emit `$invalid` if given validator is already marked as invalid).
* Change the validity state, and notifies the form when the widget changes validity. (i.e. it
* does not notify form if given validator is already marked as invalid).
*
* This method should be called by validators - ie the parser or formatter method.
* This method should be called by validators - i.e. the parser or formatter functions.
*
* @param {string} name Name of the validator.
* @param {boolean} isValid Whether it should $emit `$valid` (true) or `$invalid` (false) event.
* @param {string} validationToken Name of the validator.
* @param {boolean} isValid Whether the current state is valid (true) or invalid (false).
*/
this.setValidity = function(name, isValid) {
this.setValidity = function(validationToken, isValid) {
if (!isValid && this.error[name]) return;
if (isValid && !this.error[name]) return;
if (!isValid && this.error[validationToken]) return;
if (isValid && !this.error[validationToken]) return;
if (isValid) {
delete this.error[name];
delete this.error[validationToken];
if (equals(this.error, {})) {
this.valid = true;
this.invalid = false;
}
} else {
this.error[name] = true;
this.error[validationToken] = true;
this.invalid = true;
this.valid = false;
}
return $scope.$emit(isValid ? '$valid' : '$invalid', name, this);
if (this.$form) {
this.$form.$setValidity(validationToken, isValid, this);
}
};
@ -804,10 +807,10 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', 'ngModel',
* For example {@link angular.module.ng.$compileProvider.directive.input input} or
* {@link angular.module.ng.$compileProvider.directive.select select} directives call it.
*
* It internally calls all `formatters` and if resulted value is valid, update the model and emits
* `$viewChange` event afterwards.
* It internally calls all `formatters` and if resulted value is valid, updates the model and
* calls all registered change listeners.
*
* @param {string} value Value from the view
* @param {string} value Value from the view.
*/
this.setViewValue = function(value) {
this.viewValue = value;
@ -816,17 +819,23 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', 'ngModel',
if (this.pristine) {
this.dirty = true;
this.pristine = false;
$scope.$emit('$viewTouch', this);
if (this.$form) this.$form.$setDirty();
}
forEach(this.parsers, function(fn) {
value = fn(value);
});
if (isDefined(value) && this.model !== value) {
if (this.modelValue !== value) {
this.modelValue = value;
ngModel(value);
$scope.$emit('$viewChange', value, this);
forEach(this.viewChangeListeners, function(listener) {
try {
listener();
} catch(e) {
$exceptionHandler(e);
}
})
}
};
@ -892,22 +901,28 @@ var ngModelDirective = [function() {
inject: {
ngModel: 'accessor'
},
require: 'ngModel',
require: ['ngModel', '^?form'],
controller: NgModelController,
link: function(scope, element, attr, ctrl) {
link: function(scope, element, attr, ctrls) {
// notify others, especially parent forms
scope.$emit('$newFormControl', ctrl);
var modelCtrl = ctrls[0],
formCtrl = ctrls[1];
modelCtrl.$form = formCtrl;
if (formCtrl) formCtrl.$addControl(modelCtrl);
forEach(['valid', 'invalid', 'pristine', 'dirty'], function(name) {
scope.$watch(function() {
return ctrl[name];
return modelCtrl[name];
}, function(value) {
element[value ? 'addClass' : 'removeClass']('ng-' + name);
});
});
element.bind('$destroy', function() {
scope.$emit('$destroy', ctrl);
if (formCtrl) formCtrl.$removeControl(modelCtrl);
});
}
};
@ -965,8 +980,8 @@ var ngModelDirective = [function() {
var ngChangeDirective = valueFn({
require: 'ngModel',
link: function(scope, element, attr, ctrl) {
scope.$on('$viewChange', function(event, value, widget) {
if (ctrl === widget) scope.$eval(attr.ngChange);
ctrl.viewChangeListeners.push(function() {
scope.$eval(attr.ngChange);
});
}
});
@ -1044,7 +1059,7 @@ var requiredDirective = [function() {
var validator = function(value) {
if (attr.required && (isEmpty(value) || value === false)) {
ctrl.setValidity('REQUIRED', false);
return null;
return;
} else {
ctrl.setValidity('REQUIRED', true);
return value;

View file

@ -26,8 +26,8 @@ describe('form', function() {
it('should instantiate form and attach it to DOM', function() {
doc = $compile('<form>')(scope);
expect(doc.data('$form')).toBeTruthy();
expect(doc.data('$form') instanceof FormController).toBe(true);
expect(doc.data('$formController')).toBeTruthy();
expect(doc.data('$formController') instanceof FormController).toBe(true);
});
@ -78,15 +78,15 @@ describe('form', function() {
});
it('should publish form to scope', function() {
it('should publish form to scope when name attr is defined', function() {
doc = $compile('<form name="myForm"></form>')(scope);
expect(scope.myForm).toBeTruthy();
expect(doc.data('$form')).toBeTruthy();
expect(doc.data('$form')).toEqual(scope.myForm);
expect(doc.data('$formController')).toBeTruthy();
expect(doc.data('$formController')).toEqual(scope.myForm);
});
it('should allow name to be an expression', function() {
it('should allow form name to be an expression', function() {
doc = $compile('<form name="obj.myForm"></form>')(scope);
expect(scope.obj).toBeDefined();
@ -108,7 +108,7 @@ describe('form', function() {
var input = child.text;
input.setValidity('MyError', false);
expect(parent.error.MyError).toEqual([input]);
expect(parent.error.MyError).toEqual([child]);
expect(child.error.MyError).toEqual([input]);
input.setValidity('MyError', true);
@ -117,6 +117,41 @@ describe('form', function() {
});
it('should support two forms on a single scope', function() {
doc = $compile(
'<div>' +
'<form name="formA">' +
'<input name="firstName" ng-model="firstName" required>' +
'</form>' +
'<form name="formB">' +
'<input name="lastName" ng-model="lastName" required>' +
'</form>' +
'</div>'
)(scope);
scope.$apply();
expect(scope.formA.error.REQUIRED.length).toBe(1);
expect(scope.formA.error.REQUIRED).toEqual([scope.formA.firstName]);
expect(scope.formB.error.REQUIRED.length).toBe(1);
expect(scope.formB.error.REQUIRED).toEqual([scope.formB.lastName]);
var inputA = doc.find('input').eq(0),
inputB = doc.find('input').eq(1);
inputA.val('val1');
browserTrigger(inputA, 'blur');
inputB.val('val2');
browserTrigger(inputB, 'blur');
expect(scope.firstName).toBe('val1');
expect(scope.lastName).toBe('val2');
expect(scope.formA.error.REQUIRED).toBeUndefined();
expect(scope.formB.error.REQUIRED).toBeUndefined();
});
it('should chain nested forms in repeater', function() {
doc = jqLite(
'<ng:form name=parent>' +
@ -141,7 +176,7 @@ describe('form', function() {
input.setValidity('myRule', false);
expect(input.error.myRule).toEqual(true);
expect(child.error.myRule).toEqual([input]);
expect(parent.error.myRule).toEqual([input]);
expect(parent.error.myRule).toEqual([child]);
input.setValidity('myRule', true);
expect(parent.error.myRule).toBeUndefined();

View file

@ -30,18 +30,18 @@ describe('NgModelController', function() {
expect(ctrl.formatters).toEqual([]);
expect(ctrl.parsers).toEqual([]);
expect(ctrl.widgetId).toBe('testAlias');
expect(ctrl.name).toBe('testAlias');
});
describe('setValidity', function() {
it('should emit $invalid only when $valid', function() {
var spy = jasmine.createSpy('$invalid');
scope.$on('$invalid', spy);
it('should propagate invalid to the parent form only when valid', function() {
var spy = jasmine.createSpy('setValidity');
ctrl.$form = {$setValidity: spy};
ctrl.setValidity('ERROR', false);
expect(spy).toHaveBeenCalledOnce();
expect(spy).toHaveBeenCalledOnceWith('ERROR', false, ctrl);
spy.reset();
ctrl.setValidity('ERROR', false);
@ -78,15 +78,17 @@ describe('NgModelController', function() {
it('should emit $valid only when $invalid', function() {
var spy = jasmine.createSpy('$valid');
scope.$on('$valid', spy);
var spy = jasmine.createSpy('setValidity');
ctrl.$form = {$setValidity: spy};
ctrl.setValidity('ERROR', true);
expect(spy).not.toHaveBeenCalled();
ctrl.setValidity('ERROR', false);
expect(spy).toHaveBeenCalledOnceWith('ERROR', false, ctrl);
spy.reset();
ctrl.setValidity('ERROR', true);
expect(spy).toHaveBeenCalledOnce();
expect(spy).toHaveBeenCalledOnceWith('ERROR', true, ctrl);
});
});
@ -118,10 +120,10 @@ describe('NgModelController', function() {
});
it('should fire $viewChange only if value changed and is valid', function() {
var spy = jasmine.createSpy('$viewChange');
scope.$on('$viewChange', spy);
it('should fire viewChangeListeners when the value changes in the view (even if invalid)',
function() {
var spy = jasmine.createSpy('viewChangeListener');
ctrl.viewChangeListeners.push(spy);
ctrl.setViewValue('val');
expect(spy).toHaveBeenCalledOnce();
spy.reset();
@ -129,13 +131,25 @@ describe('NgModelController', function() {
// invalid
ctrl.parsers.push(function() {return undefined;});
ctrl.setViewValue('val');
expect(spy).not.toHaveBeenCalled();
expect(spy).toHaveBeenCalledOnce();
});
it('should only fire $viewTouch when pristine', function() {
var spy = jasmine.createSpy('$viewTouch');
scope.$on('$viewTouch', spy);
it('should reset the model when the view is invalid', function() {
ctrl.setViewValue('aaaa');
expect(ctrl.modelValue).toBe('aaaa');
// add a validator that will make any input invalid
ctrl.parsers.push(function() {return undefined;});
expect(ctrl.modelValue).toBe('aaaa');
ctrl.setViewValue('bbbb');
expect(ctrl.modelValue).toBeUndefined;
});
it('should call parentForm.setDirty only when pristine', function() {
var spy = jasmine.createSpy('setDirty');
ctrl.$form = {$setDirty: spy};
ctrl.setViewValue('');
expect(ctrl.pristine).toBe(false);
@ -278,14 +292,14 @@ describe('input', function() {
});
it('should call $destroy on element remove', function() {
compileInput('<input type="text" ng-model="name" name="alias" ng-change="change()" />');
it('should cleanup it self from the parent form', function() {
compileInput('<input ng-model="name" name="alias" required>');
var spy = jasmine.createSpy('on destroy');
scope.$on('$destroy', spy);
scope.$apply();
expect(scope.form.error.REQUIRED.length).toBe(1);
inputElm.remove();
expect(spy).toHaveBeenCalled();
expect(scope.form.error.REQUIRED).toBeUndefined();
});
@ -450,7 +464,7 @@ describe('input', function() {
describe('number', function() {
it('should not update model if view invalid', function() {
it('should reset the model if view is invalid', function() {
compileInput('<input type="number" ng-model="age"/>');
scope.$apply(function() {
@ -467,7 +481,7 @@ describe('input', function() {
changeInputValueTo('123X');
expect(inputElm.val()).toBe('123X');
expect(scope.age).toBe(123);
expect(scope.age).toBeUndefined();
expect(inputElm).toBeInvalid();
});
@ -588,7 +602,7 @@ describe('input', function() {
expect(widget.error.EMAIL).toBeUndefined();
changeInputValueTo('invalid@');
expect(scope.email).toBe('vojta@google.com');
expect(scope.email).toBeUndefined();
expect(inputElm).toBeInvalid();
expect(widget.error.EMAIL).toBeTruthy();
});
@ -616,7 +630,7 @@ describe('input', function() {
expect(widget.error.URL).toBeUndefined();
changeInputValueTo('invalid.com');
expect(scope.url).toBe('http://www.something.com');
expect(scope.url).toBeUndefined();
expect(inputElm).toBeInvalid();
expect(widget.error.URL).toBeTruthy();
});