Added 'validates' utility #49

Merged
merged 5 commits into from Jan 10, 2014

Conversation

Projects
None yet
3 participants
Contributor

willcumbie commented Jan 9, 2014

No description provided.

@jpbochi jpbochi commented on an outdated diff Jan 10, 2014

spec/validates-spec.js
@@ -0,0 +1,34 @@
+describe('validation using the "validates" utility', function () {
+ var viewModel, checkFirstName;
+
+ beforeEach(function () {
+ viewModel = {};
+ viewModel.checkFirstName = function () {return checkFirstName}
+ viewModel.firstName = ko.observable('abcdefg').extend({
+ 'onlyIf': [viewModel.checkFirstName, { 'required': [ 'First name' ] }]
+ });
+ viewModel.lastName = ko.observable('hijlkmno').extend({
+ 'minLength': ['Min Length', 5],
@jpbochi

jpbochi Jan 10, 2014

Contributor

there's no minLength on the library anymore

@jpbochi jpbochi commented on an outdated diff Jan 10, 2014

src/ko-validation.js
@@ -170,6 +170,15 @@ ko.validation.registerValidator = function (name, validatorFactory) {
return observable;
};
+ ko.extenders['validates'] = function (observable, param) {
+ ko.utils.arrayForEach(param, function (dependencyObservable) {
@jpbochi

jpbochi Jan 10, 2014

Contributor

param is not a good name for a param. 😛

I see that validatesAfter is using the same name. Still, I would prefer something more descriptive.

@jpbochi jpbochi commented on an outdated diff Jan 10, 2014

spec/validates-spec.js
@@ -0,0 +1,34 @@
+describe('validation using the "validates" utility', function () {
+ var viewModel, checkFirstName;
+
+ beforeEach(function () {
+ viewModel = {};
+ viewModel.checkFirstName = function () {return checkFirstName}
+ viewModel.firstName = ko.observable('abcdefg').extend({
+ 'onlyIf': [viewModel.checkFirstName, { 'required': [ 'First name' ] }]
+ });
+ viewModel.lastName = ko.observable('hijlkmno').extend({
+ 'maxLength': ['Max Length', 5],
@jpbochi

jpbochi Jan 10, 2014

Contributor

The argument order is incorrect. No validators expect a field name, and all of them expect a message as its last argument.

I'd just remove this validator as it's not relevant to the test.

@jpbochi jpbochi commented on an outdated diff Jan 10, 2014

src/ko-validation.js
@@ -170,6 +170,15 @@ ko.validation.registerValidator = function (name, validatorFactory) {
return observable;
};
+ ko.extenders['validates'] = function (observable, dependentObservables) {
+ ko.utils.arrayForEach(dependentObservables, function (dependentObservable) {
@jpbochi

jpbochi Jan 10, 2014

Contributor

can you change the validatesAfter extension, too? just for consistency.

@jpbochi jpbochi and 1 other commented on an outdated diff Jan 10, 2014

src/ko-validation.js
@@ -162,10 +162,19 @@ ko.validation.registerValidator = function (name, validatorFactory) {
ko.bindingHandlers.text.update(element, observable.validationMessage);
}
- ko.extenders['validatesAfter'] = function (observable, param) {
- ko.utils.arrayForEach(param, function (dependencyObservable) {
- dependencyObservable.__validates__ = dependencyObservable.__validates__ || [];
- dependencyObservable.__validates__.push(observable);
+ ko.extenders['validatesAfter'] = function (observable, dependentObservables) {
+ ko.utils.arrayForEach(dependentObservables, function (dependentObservables) {
+ dependentObservables.__validates__ = dependentObservables.__validates__ || [];
+ dependentObservables.__validates__.push(observable);
@jpbochi

jpbochi Jan 10, 2014

Contributor

I didn't mean to be picky, but the dependentObservables inside the for is actually one observable.

@willcumbie

willcumbie Jan 10, 2014

Contributor

can't believe I missed that, fixing now

Contributor

jpbochi commented Jan 10, 2014

just fix that dependentObservables inside the for and :shipit:

Contributor

jpbochi commented Jan 10, 2014

👍

@willcumbie willcumbie added a commit that referenced this pull request Jan 10, 2014

@willcumbie willcumbie Merge pull request #49 from racker/add-validates-utility
Added 'validates' utility
4d72348

@willcumbie willcumbie merged commit 4d72348 into master Jan 10, 2014

1 check passed

default The Travis CI build passed
Details
Contributor

luizfar commented Jan 10, 2014

Can you please also updated the README file to add this? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment