Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

put corrent value in the value attribute for date fields #58

Merged
merged 1 commit into from

2 participants

@tellnes

No description provided.

lib/widgets.js
@@ -88,7 +88,20 @@ exports.text = input('text');
exports.password = input('password');
exports.hidden = input('hidden');
exports.color = input('color');
-exports.date = input('date');
+
+var dateWidget = input('date');
+exports.date = function (opt) {
+ var w = dateWidget(opt);
+ w.format = function (value) {
+ if (!value) return null;
@ljharb Collaborator
ljharb added a note

Please make sure all blocks are surrounded by curly braces - also, please be explicit about what you're checking for with !value - ie, not just falsiness, but length, undefined, etc.

@tellnes
tellnes added a note

I realy want all falsy values, because this is just a shortcut to make the logic in w.toHTML more readable, but I'll rewrite it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/widgets.js
@@ -88,7 +88,20 @@ exports.text = input('text');
exports.password = input('password');
exports.hidden = input('hidden');
exports.color = input('color');
-exports.date = input('date');
+
+var dateWidget = input('date');
+exports.date = function (opt) {
+ var w = dateWidget(opt);
+ w.format = function (value) {
+ if (!value) return null;
+ if (!(value instanceof Date)) value = new Date(value);
@ljharb Collaborator
ljharb added a note

Please don't reuse variables - we might as well create a new one for the known Date object. ie something like var date = value instanceof Date ? value : new Date(value);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/widgets.js
@@ -88,7 +88,20 @@ exports.text = input('text');
exports.password = input('password');
exports.hidden = input('hidden');
exports.color = input('color');
-exports.date = input('date');
+
+var dateWidget = input('date');
+exports.date = function (opt) {
+ var w = dateWidget(opt);
+ w.format = function (value) {
+ if (!value) return null;
+ if (!(value instanceof Date)) value = new Date(value);
+ value = [value.getFullYear(), value.getMonth() + 1, value.getDate()];
@ljharb Collaborator
ljharb added a note

There's nothing here checking that value is a valid Date object?

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

Why is this formatting necessary? The specific formatting is up to the browser, and conforming browsers will always submit in 'yyyy-mm-dd' format.

@tellnes

This is necessary if you pass inn a Date instance for the value. With the old implementation would forms render something like this 'Mon Mar 18 2013 13:02:34 GMT+0100 (CET)' which the browser would fail to recognize as valid value.

I have updated the pull request as @ljharb commented and also updated the test cases to test the changes better.

lib/widgets.js
@@ -71,7 +71,7 @@ var input = function (type) {
name: name,
id: f.id || true,
classes: w.classes,
- value: f.value || null
+ value: f.value ? (w.format ? w.format(f.value) : f.value) : null
@ljharb Collaborator
ljharb added a note

Instead of this ternary, what do you think about "w.formatValue(f.value)", and make w.formatValue always a defined function that returns its first arg || null?

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

Thanks, the tests are much clearer. Just the one thought about always having a "formatValue" function, and simply customizing it for date widgets?

@ljharb
Collaborator

(also a test is still failing, but it's a minor breakage)

@tellnes

I like the formatValue proposal, so I have updated the pull request.

I have also added some more test and refactored the old tests I added.

The test you se fails, is that maybe #59 ?

@ljharb
Collaborator

No, I hadn't yet upgraded to node 0.10 when I ran those tests. I now get 3 failures, 2 of which are not from #59.

AssertionError: '2013-03-16' == '2013-03-17' on line 35 of test-widgets.js

lib/widgets.js
((12 lines not shown))
+ }
+
+ var date = value instanceof Date ? value : new Date(value);
+
+ if (isNaN(date.getTime())) {
+ return null;
+ }
+
+ var str = [date.getFullYear(), date.getMonth() + 1, date.getDate()];
+ if (str[1] < 10) {
+ str[1] = '0' + str[1];
+ }
+ if (str[2] < 10) {
+ str[2] = '0' + str[2];
+ }
+ return str.join('-');
@ljharb Collaborator
ljharb added a note

This technique is totally fine and very explicit, but I did find this one: date.toISOString().replace(/T[\d:\.]+Z$/, '') which is a one-liner.

@tellnes
tellnes added a note

I actually used date.toISOString().slice(0, 10) first, but for some reason I changed it. I change it back.

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

Could you provide some more info? I'm running on v0.8.22 and se none failing tests.

@ljharb
Collaborator

What timezone are you in? getFullYear pulls based on the timezone, whereas getUTCFullYear gets it in UTC. Perhaps that's the issue?

@tellnes

I'm in GMT+1. That could be the issue. I'll try to reproduce it tomorrow. I must go now.

@tellnes tellnes add widget.formatValue and implement custom formatValue for date fields
this puts the corrent value in the value attribute for date fields
74abee9
@tellnes

This is not perfect. You has to be sure that if you pass inn a Date object, then it is UTC. Eg. new Date(2013, 2, 21) will fail in some timezones. You must do new Date(Date.UTC(2013, 2, 21))

Date handling is hard.

@ljharb
Collaborator

lol no kidding. thanks for the improvements.

@ljharb ljharb merged commit 946e139 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 21, 2013
  1. @tellnes

    add widget.formatValue and implement custom formatValue for date fields

    tellnes authored
    this puts the corrent value in the value attribute for date fields
This page is out of date. Refresh to see the latest.
Showing with 47 additions and 3 deletions.
  1. +23 −2 lib/widgets.js
  2. +24 −1 test/test-widgets.js
View
25 lib/widgets.js
@@ -64,6 +64,9 @@ var input = function (type) {
}
return attrs;
}, {});
+ w.formatValue = function (value) {
+ return value || null;
+ };
w.toHTML = function (name, f) {
if (!f) { f = {}; }
return singleTag('input', [{
@@ -71,7 +74,7 @@ var input = function (type) {
name: name,
id: f.id || true,
classes: w.classes,
- value: f.value || null
+ value: w.formatValue(f.value)
}, userAttrs]);
};
w.getDataRegExp = function () {
@@ -88,7 +91,25 @@ exports.text = input('text');
exports.password = input('password');
exports.hidden = input('hidden');
exports.color = input('color');
-exports.date = input('date');
+
+var dateWidget = input('date');
+exports.date = function (opt) {
+ var w = dateWidget(opt);
+ w.formatValue = function (value) {
+ if (!value) {
+ return null;
+ }
+
+ var date = value instanceof Date ? value : new Date(value);
+
+ if (isNaN(date.getTime())) {
+ return null;
+ }
+
+ return date.toISOString().slice(0, 10);
+ };
+ return w;
+};
exports.checkbox = function (opt) {
if (!opt) { opt = {}; }
View
25 test/test-widgets.js
@@ -18,6 +18,8 @@ var test_input = function (type) {
'<input type="' + type + '" name="field1" id="id_field1" value="some value" />'
);
test.equals(forms.widgets[type]().type, type);
+ test.equals(forms.widgets[type]().formatValue('hello'), 'hello');
+ test.strictEqual(forms.widgets[type]().formatValue(false), null);
test.done();
};
};
@@ -26,7 +28,28 @@ exports.text = test_input('text');
exports.password = test_input('password');
exports.hidden = test_input('hidden');
exports.color = test_input('color');
-exports.date = test_input('date');
+
+exports.date = function (test) {
+ var w = forms.widgets.date();
+ test.equals(w.formatValue(new Date(Date.UTC(2013, 2, 1))), '2013-03-01');
+ test.equals(w.formatValue('2013-03-02'), '2013-03-02');
+ test.strictEqual(w.formatValue('invalid'), null);
+
+ test.equals(w.type, 'date');
+
+ test.equals(
+ w.toHTML('field1'),
+ '<input type="date" name="field1" id="id_field1" />'
+ );
+
+ test.equals(
+ w.toHTML('field1', {value: '2013-03-03'}),
+ '<input type="date" name="field1" id="id_field1" value="2013-03-03" />'
+ );
+
+
+ test.done();
+};
exports.checkbox = function (test) {
test.equals(
Something went wrong with that request. Please try again.