Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Objects Field (nested forms) #77

Merged
merged 4 commits into from
Aug 25, 2013
Merged

Objects Field (nested forms) #77

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5fcc78e
objects field
lukeburns Jul 24, 2013
19da5d6
compatibility
lukeburns Jul 27, 2013
28618ea
take object literals as nested fields
lukeburns Jul 27, 2013
9aaf0b2
style corrections
lukeburns Aug 25, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions example/simple.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,22 @@ var reg_form = forms.create({
required: true,
validators: [validators.matchField('password')]
}),
email: fields.email()
personal: {
name: fields.string({required: true, label: 'Name'}),
email: fields.email({required: true, label: 'Email'}),
address: {
address1: fields.string({required: true, label: 'Address 1'}),
address2: fields.string({label: 'Address 2'}),
city: fields.string({required: true, label: 'City'}),
state: fields.string({required: true, label: 'State'}),
zip: fields.number({required: true, label: 'ZIP'})
}
}
});


http.createServer(function (req, res) {
reg_form.handle(req, {
success: function (form) {
var req_data = require('url').parse(req.url, 1).query;
res.writeHead(200, {'Content-Type': 'text/html'});
res.write('<h1>Success!</h1>');
res.end('<pre>' + util.inspect(form.data) + '</pre>');
Expand Down
4 changes: 3 additions & 1 deletion lib/fields.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ exports.string = function (opt) {
return f;
};


exports.number = function (opt) {
if (!opt) { opt = {}; }
var f = exports.string(opt);
Expand Down Expand Up @@ -167,3 +166,6 @@ exports.date = function (opt) {
return f;
};

exports.object = function (fields, opts) {
return forms.create(fields || {});
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please always add trailing newlines to files

27 changes: 21 additions & 6 deletions lib/forms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

var async = require('async'),
http = require('http'),
querystring = require('querystring'),
querystring = require('qs'),
parse = require('url').parse;


Expand All @@ -14,6 +14,10 @@ exports.validators = require('./validators');

exports.create = function (fields) {
Object.keys(fields).forEach(function (k) {
// if it's not a field object, create an object field.
if(typeof fields[k].toHTML !== 'function' && typeof fields[k] == 'object') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please always use "===" and never "==" - also, please always put a space between "if" and the opening paren.

fields[k] = exports.fields.object(fields[k]);
}
fields[k].name = k;
});
var f = {
Expand All @@ -23,13 +27,17 @@ exports.create = function (fields) {
b.toHTML = f.toHTML;
b.fields = {};
Object.keys(f.fields).forEach(function (k) {
b.fields[k] = f.fields[k].bind(data[k]);
b.fields[k] = (data[k])? f.fields[k].bind(data[k]): f.fields[k].bind('');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the unnecessary parens around the ternary condition, and please ensure there is always a single space surrounding the ? and : in a ternary.

});
b.data = Object.keys(b.fields).reduce(function (a, k) {
a[k] = b.fields[k].data;
return a;
}, {});
b.validate = function (callback) {
b.validate = function (obj, callback) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obj isn't actually used anywhere in this function. i assume that's not intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fundamentally, the approach is emulate the field methods. When a nested form is validated, it is treated as a field and expects it to handle two arguments: form and callback. I'm not currently using the first argument, but it can be used in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to do var form = obj || b so that we are using the passed form? Emulating the field methods isn't useful if it's not going to emulate the behavior as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is useful. If we remove the first argument, it will break. If we need the form object in the future, it's there. I'm open to alternative approaches, but I think there's a certain elegance to this solution.

if(typeof obj == 'function') {
callback = obj;
}

async.forEach(Object.keys(b.fields), function (k, callback) {
b.fields[k].validate(b, function (err, bound_field) {
b.fields[k] = bound_field;
Expand All @@ -52,7 +60,8 @@ exports.create = function (fields) {
(callbacks.empty || callbacks.other)(f);
} else if (obj instanceof http.IncomingMessage) {
if (obj.method === 'GET') {
f.handle(parse(obj.url, 1).query, callbacks);
var qs = parse(obj.url).query;
f.handle(querystring.parse(qs), callbacks);
} else if (obj.method === 'POST' || obj.method === 'PUT') {
// If the app is using bodyDecoder for connect or express,
// it has already put all the POST data into request.body.
Expand Down Expand Up @@ -82,10 +91,16 @@ exports.create = function (fields) {
throw new Error('Cannot handle type: ' + typeof obj);
}
},
toHTML: function (iterator) {
toHTML: function (name, iterator) {
var form = this;

if(typeof name == 'function') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the polymorphic signatures? if name is optional, it should always come last, and if there's more than one optional arg, please pass it in an options hash.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The form object treats nested forms as fields, so I'm following the same convention as the fields.

iterator = name;
}

return Object.keys(form.fields).reduce(function (html, k) {
return html + form.fields[k].toHTML(k, iterator);
var kname = (typeof name == 'string')? name+'['+k+']': k;
return html + form.fields[k].toHTML(kname, iterator);
}, '');
}
};
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
"test-browser": "./node_modules/.bin/browserify test-browser.js | ./node_modules/.bin/testling"
},
"dependencies": {
"async": "~0.2.9"
"async": "~0.2.9",
"qs": "~0.6.5"
},
"licenses": [
{
Expand Down