Opened 7 years ago

Closed 7 years ago

#15684 closed enhancement (fixed)

Migrate dojox/gantt to AMD syntax

Reported by: dylan Owned by: dylan
Priority: blocker Milestone: 1.8
Component: DojoX Widgets Version: 1.8.0b1
Keywords: gantt Cc:
Blocked By: Blocking:

Description

I may be able to get an AMD migration patch done before 1.8 feature freeze. If I can get it tested and land it before Wednesday, I will.

Change History (16)

comment:1 Changed 7 years ago by dylan

I'm about 75% of the way there on this conversion... hope to wrap this up by the 1.8 feature freeze date.

comment:2 Changed 7 years ago by dylan

In [29313]:

refs #15684, AMD and modern Dojo overhaul of dojox/gantt, !strict

comment:3 Changed 7 years ago by dylan

Landed a patch, tested in FF and Chrome latest.

Completely lacks docs, but wanted to land this before 1.8 freeze.

comment:4 Changed 7 years ago by dylan

Resolution: fixed
Status: newclosed

Closing this out, will add docs as time permits.

comment:5 Changed 7 years ago by bill

Resolution: fixed
Status: closedreopened

Docs are orthogonal but there are build errors:

error(311) Missing dependency. module: dojox/gantt/TabMenu; dependency: dojox/gantt/contextMenuTab
error(311) Missing dependency. module: dojox/gantt/full/TabMenu; dependency: dojox/gantt/contextMenuTab

Looks like you made a new file but forgot to check it in. I think we should make an exception and allow this checkin before the RC.

Also note that you should use relative paths to relative modules, ie instead of:

define([
	"dojox/gantt/contextMenuTab",

it should be

define([
	"./contextMenuTab",

(obviously it should also be a capital C but I guess you were maintaining the previous bad naming of the class)

comment:6 Changed 7 years ago by dylan

Resolution: fixed
Status: reopenedclosed

Yes, was preserving the names. Added the missing file. I'll refrain from renaming since we're past freeze.

comment:7 Changed 7 years ago by dylan

In [29330]:

missed a file added for Gantt, refs #15684, !strict

comment:8 Changed 7 years ago by bill

Priority: lowblocker
Resolution: fixed
Status: closedreopened

TabMenu.js is referencing an undefined symbol GanttTaskControl:

show: function(elem, object){
	if(object.constructor == GanttTaskControl){

So you get an exception when mousing over the labels in the left part of the gantt.

Also, TabMenu strangely tries to declare GanttTaskControl:

define([
	...
], function(contextMenuTab,
		Dialog, Button, Form,
		registry, declare, arrayUtil, lang, locale, request, on,
		dom, domClass, domConstruct, domStyle, domAttr, domGeometry, keys, parser){
	return declare("dojox.gantt.GanttTaskControl", [], {
    ...

(Marking this as a blocker since the gantt chart is now broken.)

comment:9 Changed 7 years ago by dylan

Resolution: fixed
Status: reopenedclosed

In [29395]:

fixes #15684, wrong backwards compat declaration for dojox/gantt/TabMenu

comment:10 Changed 7 years ago by bill

Resolution: fixed
Status: closedreopened

OK, it's not failing anymore on hover, but surely TabMenu.js is wrong since it's accessing an undeclared variable GanttTaskControl.

comment:11 Changed 7 years ago by dylan

Resolution: fixed
Status: reopenedclosed

In [29397]:

fixes #15684, missing requires for GanttTaskControl? and GanttProjectControl? in dojox/gantt/TabMenu

comment:12 Changed 7 years ago by dylan

Hopefully that's everything and I've not missed more with this

comment:13 Changed 7 years ago by bill

Resolution: fixed
Status: closedreopened

No, GanntProjectControl also references GanttTaskControl. Also, GanttTaskItem in GanttChart. And GanntTaskControl in contextMenuTabs.

comment:14 Changed 7 years ago by bill

In [29398]:

lint fixes, refs #15684 !strict

comment:15 Changed 7 years ago by dylan

In [29399]:

refs #15684, add more missing Gantt cross-module dependencies, !strict

comment:16 Changed 7 years ago by bill

Resolution: fixed
Status: reopenedclosed
Note: See TracTickets for help on using tickets.