#12863 closed enhancement (fixed)
Convert dojox packages to amd
Reported by: | Chris Mitchell | Owned by: | Tom Trenka |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | Dojox | Version: | 1.6.1 |
Keywords: | 1.7-mobile | Cc: | ben hockey |
Blocked By: | Blocking: |
Description (last modified by )
Using this ticket for continued work around dojox project conversions to AMD.
Sign up for a conversion by putting your trac id next to the modules below. If you are not a committer, you can still sign up, but attach your patch for each module as u convert and verify by running it's tests.
IMPORTANT: All modules should be converted to use "Base-less" AMD style, with minimal dependencies. See the comments in 12844 about proper style for modules, so that api-doc tool works with your package.
[H] indicates higher priority
Note: As templated Dijit-based widgets in dojox are converted to amd, we should use the new text! dependency injection, rather than dojo.cache. This should be a simple fix, see changeset r24784 for examples of how Bill did this recently for dijit.
color - ttrenka, cjolif - COMPLETE/COMMITTED
collections - ttrenka COMPLETE/COMMITTED
grid - COMPLETE
html - chrism - COMPLETE/COMMITTED
math - COMPLETE/COMMITTED
mobile - dmachi - COMPLETE/COMMITTED
timing - ttrenka COMPLETE/COMMITTED
string - ttrenka COMPLETE/COMMITTED
json - neonstalwart - COMPLETE/COMMITTED
geo - erwan - COMPLETE/COMMITTED (See #12844)
charting - cjolif - COMPLETE/COMMITTED (See #12844, #12642)
gauges - etissandier - COMPLETE/COMMITTED (See #12844)
gfx - eugene - COMPLETE/COMMITTED (See #12844)
gfx3d - etissandier - COMPLETE/COMMITTED
analytics - edchat - COMPLETE/COMMITTED
[H]dtl - edchat - COMPLETE/COMMITTED
[H]lang - ttrenka - IN PROGRESS
[H]css3 - nic COMPLETE/COMMITTED
[H]fx - chrism COMPLETE/COMMITTED
[H]date - peller - COMPLETE / COMMITTED
encoding - BryanForbes - COMPLETE/COMMITTED
io - BryanForbes - COMPLETE / COMMITTED
mdnd - chrism - COMPLETE/COMMITTED
uuid - BryanForbes - COMPLETE/COMMITTED
validate - BryanForbes - COMPLETE/COMMITTED
xml - ttrenka, BryanForbes - COMPLETE/COMMITTED
store - ttrenka - COMPLETE/COMMITTED
sketch - ttrenka - COMPLETE/COMMITTED
embed - TRT
atom
av
calc
cometd
data
[H]CsvStore?.js
[H]HtmlStore?.js
[H]JsonRestStore?.js
[H]KeyValueStore?.js
[H]QueryReadStore?.js
[H]XmlStore?.js
AndOrReadStore?.js
AndOrWriteStore?.js
AppStore?.js
AtomReadStore?.js
CdfStore?.js
ClientFilter?.js
CouchDBRestStore.js
css.js
CssClassStore?.js
CssRuleStore?.js
dom.js
FileStore?.js
FlickrRestStore?.js
FlickrStore?.js
GoogleFeedStore?.js
GoogleSearchStore?.js
HtmlTableStore?.js
ItemExplorer?.js
JsonQueryRestStore?.js
OpenSearchStore?.js
OpmlStore?.js
PersevereStore?.js
PicasaStore?.js
RailsStore?.js
restListener.js
S3Store.js
ServiceStore?.js
SnapLogicStore?.js
StoreExplorer?.js
WikipediaStore?.js
dnd
drawing
editor
flash
form
[H]CheckedMultiSelect - eldon COMPLETE/COMMITTED
[H]MultiComboBox?
[H]RangeSlider?
[H]Rating
[H]TimeSpinner?
[H]TriStateCheckBox - siqi COMPLETE/COMMITTED
[H]Uploader
_FormSelectWidget
_HasDropDown
_SelectStackMixin
BusyButton?
DateTextBox?
DropDownSelect?
DropDownStack?
FileInput - liucougar COMPLETE/COMMITTED
FileInputAuto - liucougar COMPLETE/COMMITTED
FileInputBlind?
FilePickerTextBox?
FileUploader?
ListInput?
Manager
PasswordValidator?
RadioStack?
gantt
help
highlight
image
jq
jsonPath
layout
[H]ContentPane?
[H]ResizeHandle?
[H]GridContainer? - chrism - COMPLETE
[H}GridContainerLite? - chrism - COMPLETE
RotatorContainer?
ScrollPane?
DragPane?
TableContainer?
ExpandoPane?
RadioGroup?
ToggleSplitter?
FloatingPane?
[L]BorderContainer?
NodeList?
rails
robot
rpc
secure
socket
sql
storage
testing
widget
[H]ColorPicker? - chrism - COMPLETE/COMMITTED
[H]Dialog
AutoRotator?
Calendar
CalendarFx?
CalendarViews?
DataPresentation?
DialogSimple?
DocTester?
DynamicTooltip?
FeedPortlet?
FilePicker?
FisheyeList?
FisheyeLite?
Iterator
Loader
Pager
PlaceholderMenuItem - liucougar COMPLETE/COMMITTED
Portlet
Roller
RollingList?
Rotator
SortList?
Standby
TitleGroup?
Toaster
UpgradeBar?
Wizard
[L]AnalogGauge?
[L]BarGauge?
wire
xmpp
Attachments (16)
Change History (249)
comment:1 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:3 Changed 10 years ago by
Keywords: | 1.7-mobile added |
---|---|
Priority: | normal → high |
Type: | defect → enhancement |
comment:4 Changed 10 years ago by
Cc: | ben hockey added |
---|
comment:5 Changed 10 years ago by
Summary: | Convert dojox packages to gfx → Convert dojox packages to amd |
---|
comment:6 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:7 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:8 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:9 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:10 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:11 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:12 Changed 10 years ago by
comment:13 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:14 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:15 Changed 10 years ago by
comment:16 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:17 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:18 Changed 10 years ago by
comment:19 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:20 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:21 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:22 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:23 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:24 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:25 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:26 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:27 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:28 Changed 10 years ago by
comment:29 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:30 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:31 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:32 Changed 10 years ago by
comment:33 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:34 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:35 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:37 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:39 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:41 Changed 10 years ago by
I have attached a patch for AMD loader for gfx3d. Please review before commit. I have not reformated the code so that the review of the patch is easier.
comment:42 Changed 10 years ago by
Not sure why the ref message didn't make it, but converted the entry points for collections, color and timing (the three js files in the root of DojoX).
comment:43 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:44 Changed 10 years ago by
@all - has anyone done any testing with a build after the conversion? if i'm not mistaken (ref #12673) we can't do any testing with a build yet. are people planning to run the tests again once a build is possible?
comment:45 Changed 10 years ago by
there is no build yet, we're running existing test cases against the newly converted modules. Will need to do additional testing of the build system once it's available.
comment:46 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:47 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:48 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:49 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:50 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:51 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:52 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:54 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:55 Changed 10 years ago by
comment:56 Changed 10 years ago by
(In [24743]) refs #12863 added async testcase for dojox.widget.ColorPicker?
comment:57 Changed 10 years ago by
comment:59 Changed 10 years ago by
comment:60 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:61 Changed 10 years ago by
comment:63 Changed 10 years ago by
As templated Dijit-based widgets in dojox are converted to amd, we should use the new text! dependency injection, rather than dojo.cache. This should be a simple fix, see changeset r24784 for examples of how Bill did this recently for dijit.
comment:64 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:65 Changed 10 years ago by
(In [24785]) refs #12863 update ColorPicker? to use dojo/text template AMD injection !strict
comment:66 Changed 10 years ago by
(In [24787]) refs #12863 updated MultiSelectCalendar? to use dojo/text template and added async testcase !strict
comment:67 Changed 10 years ago by
(In [24788]) refs #12863 updated MultiSelectCalendar? to use dojo/main rather than dojo !strict
comment:68 Changed 10 years ago by
comment:69 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:72 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:73 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:74 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:75 Changed 10 years ago by
comment:76 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:77 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:79 Changed 10 years ago by
comment:80 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:81 Changed 10 years ago by
comment:82 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:84 Changed 10 years ago by
comment:85 Changed 10 years ago by
comment:86 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:87 Changed 10 years ago by
comment:88 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:90 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:91 Changed 10 years ago by
comment:93 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:94 Changed 10 years ago by
comment:95 Changed 10 years ago by
comment:96 Changed 10 years ago by
comment:100 Changed 10 years ago by
comment:101 Changed 10 years ago by
comment:102 Changed 10 years ago by
comment:103 Changed 10 years ago by
comment:104 Changed 10 years ago by
comment:105 Changed 10 years ago by
comment:106 Changed 10 years ago by
comment:107 Changed 10 years ago by
comment:110 Changed 10 years ago by
comment:111 Changed 10 years ago by
comment:112 Changed 10 years ago by
comment:113 Changed 10 years ago by
comment:114 Changed 10 years ago by
comment:115 Changed 10 years ago by
comment:116 Changed 10 years ago by
comment:117 Changed 10 years ago by
comment:119 Changed 10 years ago by
Changed 10 years ago by
Attachment: | dojox.gfx.amdfix.patch added |
---|
dojox.gfx._base is using dojox without getting a reference on it
comment:120 Changed 10 years ago by
comment:121 Changed 10 years ago by
comment:122 Changed 10 years ago by
comment:123 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:124 Changed 10 years ago by
comment:126 Changed 10 years ago by
comment:127 Changed 10 years ago by
comment:129 Changed 10 years ago by
comment:130 Changed 10 years ago by
comment:133 Changed 10 years ago by
comment:134 Changed 10 years ago by
(In [25743]) refs #12863: convert FileInput/FileInputAuto? to AMD
comment:135 Changed 10 years ago by
Description: | modified (diff) |
---|
Changed 10 years ago by
Attachment: | dojox.data.OpenSearchStore.patch added |
---|
amd conversion for dojox.data.OpenSearchStore?
Changed 10 years ago by
Attachment: | dojox.data.OpenSearchStore.2.patch added |
---|
dojox.data.OpenSearchStore? conversion
comment:136 Changed 10 years ago by
The dojox.data.OpenSearchStore.2.patch
is the correct file; I failed to correctly check the “override attachment” checkbox. ttrenka, this file has been updated to reflect the review you provided in IRC.
dojox.atom.widget
has some bugs, but they are not caused by the AMD conversion, so I did not fix them.
comment:137 Changed 10 years ago by
Oops, I forgot to mention, I did check all tests to make sure things passed and were functional, though only in Firefox 5.
I will pick up the high priority stuff later tonight/tomorrow and hopefully get most of it cleared by tomorrow evening.
Changed 10 years ago by
Attachment: | dojox.layout.patch added |
---|
ContentPane, ResizeHandle. Also updates dojox.html._base which is a dependency for ContentPane
Changed 10 years ago by
Attachment: | dojox.embed.patch.tar.bz2 added |
---|
dojox.embed.QuickTime and dojox.embed.!Object, w/ new resource to fix QuickTime test, & cleanup of unused Eolas stuff
comment:138 Changed 10 years ago by
comment:139 Changed 10 years ago by
Please note that dojox.form.PasswordValidator is broken; this is not due to the conversion afaik, it is broken in 1.6 too due to change from getDescendants
to _getDescendantFormWidgets
. I fixed some parts so it at least is partially functioning in 1.7 but the oldPassword box does not work right and many tests still fail.
comment:140 Changed 10 years ago by
That should read, “broken in 1.6 too due to change from getDescendants
to _getDescendantFormWidgets
in dijit._FormMixin._getValueAttr
.”
(would love to be able to edit my comments since I make lots of mistakes ;))
Changed 10 years ago by
Attachment: | dojox.form.patch added |
---|
All remaining unconverted dojox.form widgets, plus a fix to dojox/data/demos/stores/filestore_dojotree.php for strict mode php5.3 which is necessary for some dojox.form tests
Changed 10 years ago by
Attachment: | CheckedMultiSelect.TemplatedMixin.patch added |
---|
CheckedMultiSelect? now uses _TemplatedMixin and _WidgetsInTemplateMixin
comment:141 Changed 10 years ago by
Ugh, apologies for attaching that patch to this completely unrelated issue. I'll make a proper issue for it when I get a moment.
comment:142 Changed 10 years ago by
comment:143 Changed 10 years ago by
comment:144 Changed 10 years ago by
comment:145 Changed 10 years ago by
comment:146 Changed 10 years ago by
comment:147 Changed 10 years ago by
comment:148 Changed 10 years ago by
(In [25813]) Patch from csnover (CLA on file) to convert dojox/layout/ContentPane and ResizeHandle to AMD format. Also updates dojox.html._base which is a dependency for ContentPane?. Thanks! Refs #12863 !strict.
comment:149 Changed 10 years ago by
comment:150 Changed 10 years ago by
comment:151 Changed 10 years ago by
(In [25817]) Patch from csnover (CLA on file) to convert dojox.embed.QuickTime and dojox.embed.Object to AMD format, w/new resource to fix QuickTime? test, & cleanup of unused Eolas stuff Thanks! Refs #12863 !strict.
comment:152 Changed 10 years ago by
comment:153 Changed 10 years ago by
(In [25826]) Various fixes to dojox.date AMD refactor, including:
- fix missing dojo.declare dependency
- some code loaded dojo/_base/kernel but expected all of dojo/_base; changed that code to load dojo/main
- fixed code that loaded dojo/_base/kernel as "d" rather than "dojo", but then acessed "dojo" as a global
Refs #12863 !strict.
comment:154 Changed 10 years ago by
comment:155 Changed 10 years ago by
comment:156 Changed 10 years ago by
comment:157 Changed 10 years ago by
comment:159 Changed 10 years ago by
comment:160 Changed 10 years ago by
comment:161 Changed 10 years ago by
comment:162 Changed 10 years ago by
comment:163 Changed 10 years ago by
comment:164 Changed 10 years ago by
comment:165 Changed 10 years ago by
comment:167 Changed 10 years ago by
comment:168 Changed 10 years ago by
comment:169 Changed 10 years ago by
comment:186 follow-up: 187 Changed 10 years ago by
chrism, your last patch violates many many code conventions. Please see my comment on #13699 and review all of your last patch set for violations. Also kgf pointed out you may have broken the doc parser? Please check preview.php to make sure that docs still work correctly.
comment:187 Changed 10 years ago by
Replying to csnover:
chrism, your last patch violates many many code conventions.
to be fair to chrism... it doesn't break "many many code conventions." afaict it breaks 1 convention - many times. only constructors start with capitals and abbreviations and acronyms should not be uppercase - although this has not been followed strictly throughout dojo. (maybe that's 2 conventions)
Changed 10 years ago by
Attachment: | dojox.charting.tooltip_number.patch added |
---|
Potential patch to be able to use dojo/number and dijit/Tooltip condittionaly. That is only if they have been loaded. If not then do not use them.
Changed 10 years ago by
Attachment: | dojox.charting.themesamd.patch added |
---|
Additiona fixes on AMD for charting themes
Changed 10 years ago by
Attachment: | dojox.charting.amdtests.patch added |
---|
duplicate some charting tests to AMD syntax (+async for some of theme) to better test charting AMD
Changed 10 years ago by
Attachment: | dojox.charting.tooltip_number.2.patch added |
---|
Patch rework after mailing list discussion to avoid try { require} catch for axis where I can dynamically load the tooltip as the truncation is not on by default
comment:205 Changed 10 years ago by
This last patch to demos and gauges is from Emmanuel Tissandier @ IBM. Thanks!
Changed 10 years ago by
Attachment: | analytics-12863.patch added |
---|
renamed patch from edchat @ ibm to make viewable with trac
comment:207 Changed 10 years ago by
I looked over the analytics-12863.patch but since I know nothing about analytics it doesn't mean much Our policy to have module owners review patches has gone out the window for 1.7, partially because many dojox modules are abandoned. Anyway, the patch looks generally good although I didn't run the tests; someone definitely should. Issues I saw are:
- _base.js references a global dojox variable, it should be calling getObject("dojox.analytics")
- base.js changed dojo._toArray(arguments) to ._toArray(arguments), although doesn't matter since it's in a comment
- the return statements in many modules still hardcode the dojox variable, for example: return dojox.analytics.plugins.mouseClick. Also the return statements could be simplified, instead of doing: foo = ..., return foo just do return (foo = ...)
- mouseOver.js adds a "true" final parameter to on() that seems unnecessary, were you confusing on() with aspect.after()?
Also, about dojo.js: With the conversion of the rest of the code base to AMD not sure this works anymore. There might not even be a dojo variable in extreme cases. That's a separate issue from the conversion of analytics to AMD, but worth mentioning.
comment:209 Changed 10 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Changed 10 years ago by
Attachment: | analytics-12863-patch added |
---|
Updated analytics amd patch, with updates from Bill's comments. I ran the test_analytics.html successfully and added test_analytics-async.html, I also verified that the GoogleAnalytics? tests got thru the Urchin.js code and they failed in the same way they were prior to these changes.
comment:211 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
This completes the dojox AMD ports for 1.7 under this ticket. We will be opening up individual enhancement tickets for any dojox packages that are still in a partially migrated state, or have not yet been migrated to AMD with minimal dependencies.
Changed 9 years ago by
Attachment: | dojox.NodeList.delegate-AMD.js.patch added |
---|
[CLA] Convert dojox.NodeList?.delegate to AMD format (include async test)
comment:218 follow-up: 219 Changed 9 years ago by
comment:219 follow-up: 220 Changed 9 years ago by
comment:220 Changed 9 years ago by
Replying to cjolif:
Replying to kgf:
Replying to cjolif:
In [27211]:
Should this go into 1.7 branch as well? (Speaking of which, thanks for your other AMD fix yesterday, btw)
Indeed it should go in 1.7 as well. Do I need to create a new ticket for that or can I just commit with that ticket as reference?
Separate ticket would technically be best because it makes it trackable under the 1.7.1 milestone, though I realize that's laborious for something this small. I've already noticed a couple other 1.7.1 fixes batched with tickets that are not specific to 1.7.1.
comment:226 Changed 9 years ago by
In [28907]:
PS: note that AV is broken both before and after this checkin.
My last attachment is for #12844 instead... Sorry.