Opened 9 years ago
Closed 9 years ago
#15062 closed defect (fixed)
dojox/mobile/DatePicker needs a new architecture
Reported by: | ben hockey | Owned by: | Eric Durocher |
---|---|---|---|
Priority: | high | Milestone: | 1.8 |
Component: | DojoX Mobile | Version: | 1.7.2 |
Keywords: | Cc: | Eric Durocher, ykami, Ed Chatelain, cjolif | |
Blocked By: | Blocking: |
Description
because dojox/mobile/DatePicker tries to lazyload either "dojox/mobile/ValuePickerDatePicker"
or "dojox/mobile/SpinWheelDatePicker"
asynchronously, it is not possible to use the DatePicker? programmatically and there is also no certainty that it won't fail when being parsed.
the following code should be possible:
require(['dojox/mobile/DatePicker', 'dojox/mobile/deviceTheme'], function (DatePicker) { var picker = new DatePicker().placeAt(document.getElementById('picker')); });
see http://jsfiddle.net/neonstalwart/PTxw6/ for a live example
Attachments (2)
Change History (27)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Cc: | Eric Durocher added |
---|
(excerpt from #15081)
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?
comment:3 Changed 9 years ago by
here's a suggestion - change the names to whatever you want but the architecture is the important part
dojox/mobile/DatePicker.js
define(['./_DatePickerChooser!'], function (DatePicker) { return DatePicker; });
dojox/mobile/_DatePickerChooser.js
define(function (require) { return { load: function (id, parentRequire, loaded) { if (id != null) { parentRequire([id], loaded); } else { require([dm.currentTheme === "android" ? "./ValuePickerDatePicker" : "./SpinWheelDatePicker"], loaded); } } }; });
comment:4 Changed 9 years ago by
Cool, thanks.
Then, how about the code below?
I changed _DatePickerChooser to _PickerChooser to be able to handle both date picker and time picker.
dojox/mobile/_PickerChooser.js
define(["dojo/_base/lang"], function(lang){ return{ load: function(id, parentRequire, loaded){ var dm = lang.getObject("dojox.mobile", true); parentRequire([(dm.currentTheme === "android" ? "./ValuePicker" : "./SpinWheel") + id], loaded); } }; });
dojox/mobile/DatePicker.js
define(["./_PickerChooser!DatePicker"], function(DatePicker){ return DatePicker; });
dojox/mobile/TimePicker.js
define(["./_PickerChooser!TimePicker"], function(TimePicker){ return TimePicker; });
comment:5 Changed 9 years ago by
Milestone: | tbd → 1.8 |
---|---|
Priority: | undecided → high |
Status: | new → assigned |
comment:7 Changed 9 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
are you going to add a build plugin as well? how will the build include the right modules?
comment:9 Changed 9 years ago by
Replying to ykami:
What do you mean by build plugin? build profile file?
not a profile - a plugin. the build plugin works with the builder to make sure that modules that are normally loaded by plugins are properly included in the build. see http://bugs.dojotoolkit.org/browser/dojo/util/trunk#build/plugins. i would think that in this case, someone might set the theme in their profile and then be able to do theme specific builds. the build plugin for _PickerChooser would make sure that the proper module for the specific theme gets included in the build. without this plugin to include the appropriate module in the right layers there will be extra HTTP requests since these modules won't be included in any of the layers produced by the build.
comment:10 follow-up: 11 Changed 9 years ago by
Can you point us to some documentation of the build plugin system?
As an alternative, the appropriate class (SpinWheel/ValuePickerDatePicker) could simply be added by the user to his theme-specific profile (assuming this is clearly documented of course)?
comment:11 Changed 9 years ago by
Replying to edurocher:
Can you point us to some documentation of the build plugin system?
i don't know of any offhand. try searching livedocs or looking at the code - that's all i'd be doing.
comment:12 Changed 9 years ago by
Owner: | changed from ykami to Eric Durocher |
---|---|
Status: | reopened → assigned |
edurocher, I think this ticket should be closed and a new one should be opened, but anyway please handle this.
comment:13 Changed 9 years ago by
Cc: | ykami Ed Chatelain cjolif added |
---|
The current code just returns the theme-dependent picker class, but never actually declares the theme-independent class, so any markup using for example dojox.mobile.DatePicker? will fail since the class does not exist. The patch declares these classes as empty wrapper classes derived from the theme-dependent classes.
comment:14 Changed 9 years ago by
if you just need a global then rather than declaring a new class, you might want to use lang.setObject
rather than declare
comment:15 Changed 9 years ago by
Right, I thought of doing that at first, but I think it is better to actually declare the class so that the class is compatible with the previous version (same base class, etc...).
Changed 9 years ago by
Attachment: | 15062.patch added |
---|
Fix to actually declare the dojox.mobile.Date/TimePicker classes - Eric Durocher (IBM, CCLA)
comment:16 Changed 9 years ago by
I just changed the previous patch to require "./common" from _PickerChooser.js: this module makes sure dm.currentTheme is correct even if deviceTheme was loaded using a <script> tag. Now test_DatePicker.html and test_TimePicker.html run OK and use the correct theme depending on the device.
comment:18 Changed 9 years ago by
edurocher, thank you for investigating the problems (global object and access to currentTheme). Both seem to be what I introduced when I switched to the plugin. I understood the problems from your patch.
I guess both declare and setObject would work. What could be a problem if we do
return lang.setObject("dojox.mobile.TimePicker", TimePicker);
?
For accessing currentTheme, I took a bit different approach, because I do not want to make dependency on ./common whenever possible (to keep the possibility that the widgets could be used outside the mobile context).
comment:19 Changed 9 years ago by
Thanks ykami, sure setObject would work too in 99.9% of the cases. The only problem with setObject would be backward compatibility: in the (admittedly very unlikely) case an application would rely on the fact that the base class of dojox.mobile.TimePicker is dojox.mobile.[SpinWheel|ValuePicker]TimePicker, or that its declaredClass is "dojox.mobile.TimePicker", then the setObject solution creates an incompatibility while the declare solution does not.
comment:20 Changed 9 years ago by
OK, I think there are no back compat issues here because DatePicker/TimePicker are new widgets in 1.8. But that's fine. Thanks.
comment:21 Changed 9 years ago by
You are right, I did not realize these widgets are new (and I missed your last comment until now somehow). Then lang.setObject is definitely lighter and has no drawback, so let's go for that. Thanks.
Changed 9 years ago by
Attachment: | 15062.2.patch added |
---|
New patch using lang.setObject instead of declare() - Eric Durocher (IBM, CCLA)
comment:22 Changed 9 years ago by
Or you could just return DatePicker
. Not sure what your goal is with the setObject() call.
comment:23 Changed 9 years ago by
The goal is to support data-dojo-type="dojox.mobile.DatePicker". This requires the global variable to be defined. For example test_DatePicker.html fails with a simple return (which is what was there before my first patch). Of course the recommended way is now to use MIDs, and indeed data-dojo-type="dojox/mobile/DatePicker" does work with the simple return statement, but I assume we must still support the dot syntax.
comment:24 Changed 9 years ago by
OK. But just FYI, data-dojo-type="dojox.mobile.DatePicker" will work automatically if you use the old-school
dojo.require("dojox.mobile.DatePicker")
and (IIRC, even with dojox/mobile/parser), using slashes will work even with the new require() syntax:
data-dojo-type="dojox/mobile/DatePicker"
Of course, it's fine if you want to do the setObject(). Unfortunately all of our other code sets the globals (usually via dojo.declare()), so the setObject() would be consistent.
the issue is that since DatePicker? lazy-loads its implementation, the
placeAt
function is not available immediately after construction.