mobile: use Date objects as value for date picker widgets

[ben hockey]

related to #15063.

improve the mobile Date pickers to work with a Date as a value

picker.set('value', new Date());

picker.get('value'); // returns a Date

[Adam Peller]

Javascript Date objects are just horrible. Perhaps it would be good to use the ISO string representation instead? I think that's what bill is thinking for Dijit 2.0

[ben hockey]

i'll settle for anything that is the value property and (de)serializes easier than the array [2010, 9, 1]. ISO strings would be ok. consistency with dijit is the real goal here.

[Adam Peller]

ISO is nice for several reasons. It's trivial to parse (split or regexp), you can go back and forth to Date objects as needed with dojo/date/stamp or native ES5 methods, you don't have to represent months as n-1, and you can leave times and timezones out of the picture entirely. They're also much cheaper than Date objects.

You're dealing with only the date, but you'd probably want to tolerate date-times as input: yyyy-MM-ddTHH:mm:ss... I suppose that raises the issue of which timezone to use.

[ykami]

Sounds like a nice to have feature. Do you think it's worth adding extra code for this?

[ben hockey]

i believe it would be common enough that if the code isn't added to the widget it will likely exist in some form in almost every application that uses this widget. if it is left to developers to add code for serializing/deserializing values for this widget themselves it will also likely be code that gets repeated within the application.

i think adam makes a good point about ISO strings and that format would be ideal for transferring data across the wire so serializing/deserializing the value as an ISO string would be a good option.

how much extra code would it be?

[Adam Peller]

The ISO code drag shouldn't be too bad. dojo/date/stamp is small, and pretty close to something we can has-optimize out with ES5, or if we can assume ES5 going forward, there's no code drag at all.

Using ISO dates for external APIs is one thing, but if we want to use some sort of simple string representation internally also, I guess you could just steal the parsing pieces and never turn it into a date... a split or a regex, pretty simple if you don't have to worry about defaults. Of course, a big benefit is that the representation would then be date-only and not date-time with timezone issues.

Question then is, how do we deal with multiple calendar support? We've been using the Javascript Date object as an abstraction. We could use ISO dates for external APIs, but we'd need some way to abstract it in our implementation.

[ykami]

OK, that makes sense. SpinWheel is relatively a little bigger widget (taking its dependent modules into account). Adding dojo/date/stamp does not seem to have too much impact.

[Eric Durocher]

Porposed implementation of picker.set('value', '<ISO-date-string>')/picker.get('value')

[Eric Durocher]

Here is a patch that adds a 'value' attribute to the _DatePickerMixin, so you can do:

picker.set('value', '<iso-date-string>');
var iso_date_string = picker.get('value');

[Eric Durocher]

test case for previous patch

[Adam Peller]

I'm unable to find the code this patches. If the public API is still using triples (array) elsewhere, I suggest using ISO dates *instead*. This would provide a more consistent API with the rest of Dojo. Also, the n-1 month index is always a source of confusion.

[Eric Durocher]

Proposed implementation of picker.set('value', '<ISO-date-string>')/picker.get('value')

[Eric Durocher]

The code is dojox/mobile/_DatePickerMixin.js - attached a new patch created from the root, sorry.

[ykami]

neonstalwart, peller, does the patch look good to you?

[Adam Peller]

sure, I guess the only thing I'd question is failing silently on bad input. perhaps just let it throw?

[ben hockey]

can you add some assertions/tests to the test? i want to make sure this doesn't regress.

also, this whole DatePicker? code looks kind of strange

• when someone requires DatePicker?, they asynchronously get the prototype changed! this makes programmatic instantiation of DatePicker? basically impossible. see #15062
• you've got properties like slotClasses and slotProps that are "static" properties to be shared across all instances! this means that changing slotProps for one instance could possibly affect different instances.
• reorderSlots is using a private method of datelocale var a = datelocale._parseInfo().bundle["dateFormat-short"].toLowerCase().split(/[^ymd]+/, 3);
• when you set the value, you already have the string in a known format, yet it gets converted to a date and then back to a string again. isn't there a way to use the string without needing to convert it to a date and back again?
• the values property (which is expected to be an array) probably shouldn't be part of the public API since an array of strings is not a typical format for date values.

that's all i have time for right now.

[ykami]

when someone requires DatePicker??, they asynchronously get the prototype changed! this makes programmatic instantiation of DatePicker?? basically impossible. see #15062

Right, it doesn't work as you expected since it requires either of the modules asynchronously. My intention for DatePicker/TimePicker was basically for declarative use cases, because you can easily choose whichever widget programmatically, but I thought it would not be easy to do so declaratively. Also, I believe this style of programmatic instantiation should work:

require([
  ...
  "dojox/mobile/DatePicker"
], function(ready, DatePicker){
  ready(function(){
    var w = new DatePicker({}, "picker1");
    w.startup();
    ...
  });
});

That said, it would be better if your way of instantiation worked as well. Do you have any idea to achieve that?

you've got properties like slotClasses and slotProps that are "static" properties to be shared across all instances! this means that changing slotProps for one instance could possibly affect different instances.

Oops, you are correct, sorry I should have noticed earlier. (The original code came from my colleague..)

reorderSlots is using a private method of datelocale var a = datelocale._parseInfo().bundledateFormat-short?.toLowerCase().split(/[^{ymd]+/, 3);}

Right. But I couldn't find a better way to do it.

when you set the value, you already have the string in a known format, yet it gets converted to a date and then back to a string again. isn't there a way to use the string without needing to convert it to a date and back again?

For English locale, you may be right. But other locales may need conversion.

the values property (which is expected to be an array) probably shouldn't be part of the public API since an array of strings is not a typical format for date values.

It comes from the base class, whose usage is not limited to date picker, and it is used internally. You may be correct, the string array setter should be used only internally. Maybe we should document the values setter for date picker takes an integer array.

[Eric Durocher]

New patch uploaded (sorry for the mixup in patch names... :-} ):

• set('value') now throws an error on invalid input
• changed the test into a DOH unit test

[Adam Peller]

the dojo way is usually not to check the arguments, but to just let it fail (and be sure that it does) to save the extra bytes of code. There's certainly a good argument to be made for explicitly checking and throwing comprehensible error messages. I suppose such style would be up to the module owner.

[Eric Durocher]

New patch, ignore previous patches - Eric Durocher (IBM, CCLA)

[Eric Durocher]

Yes I was wondering about that actually... OK, a byte is a byte especially on mobile, I removed the explicit throw (and added the error case in the unit test). Thanks!

[ykami]

(I didn't mean to mark this ticket as low priority)

[ykami]

In [28297]:

Refs #15081 !strict. Added value setter and getter that handle ISO dates to _DatePickerMixin. Also added a test case for that. Thank you edurocher for the patch.

[ykami]

As for the "static" properties like slotClasses, I checked the code, but couldn't find any problems with that, because they are referenced only in buildRendering, that is, changing those values at runtime does not make sense. Do you have any specific scenarios that may have the problem?

[ykami]

In [28326]:

Refs #15081 It turned out that [28297] works only in locales that have month names (e.g. English). For other locales (e.g. Japanese), month is a number, and it is converted again in _setValuesAttr, and that results in n+1 month. Fixed _setValuesAttr so that users can specify month that starts from 1, not zero. !strict

[ben hockey]

Replying to ykami:

As for the "static" properties like slotClasses, I checked the code, but couldn't find any problems with that, because they are referenced only in buildRendering, that is, changing those values at runtime does not make sense. Do you have any specific scenarios that may have the problem?

i haven't had a chance to see what happens when i use this. i'm just very suspicious of "static" properties. if you've looked at it and you say its ok then i'll accept that.

[ykami]

ok, thanks. You are correct, they may be a bit confusing. Their scope should be something like 'protected' in Java, since they are intended to be used from subclasses like SpinWheelDatePicker. They are not necessary for typical programmatic usage. Maybe I should make them private to avoid confusion.

[ykami]

I think we're done.