Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13814 closed defect (fixed)

[regression] dijit.Calendar no longer inherits dijit._Widget

Reported by: Kenneth G. Franqueiro Owned by: bill
Priority: high Milestone: 1.7
Component: Dijit Version: 1.7.0b1
Keywords: calendar Cc:
Blocked By: Blocking:

Description

If I'm reading the source correctly, presently the inheritance chain of dijit.Calendar is as follows:

dijit.Calendar <- dijit.CalendarLite <- dijit._WidgetBase
dijit.Calendar <- dijit._CssStateMixin

However, prior to 1.7, dijit.Calendar directly inherited dijit._Widget, not _WidgetBase.

Should we also mix dijit._Widget into dijit.Calendar? I'm not sure if anything within the toolkit would break without it, but I presume it's possible that user code may be relying on features of it that will be missing otherwise.

Change History (10)

comment:1 Changed 8 years ago by bill

Cc: bill removed
Owner: set to bill
Status: newassigned

Good point, probably I should split out the deferred-connect stuff and the deprecated methods from _Widget into separate mixins (making _Widget nothing but a combination of other classes), and then include those mixins in Calendar.

comment:2 Changed 8 years ago by Kenneth G. Franqueiro

I was wondering if you'd say that. I don't think it should actually require that effort (and I'm not sure how high-risk that is at this point in the cycle, as simple as it probably is). declare should be able to mix _Widget directly into Calendar anyway (in addition to CalendarLite), due to how it linearizes multiple inheritance chains, right?

comment:3 Changed 8 years ago by bill

I'm not sure, that would mean that Calendar was inheriting _WidgetBase twice, which sounds dangerous. I thought Eugene said not to do that, but I can't be sure.

comment:4 Changed 8 years ago by Kenneth G. Franqueiro

I reckon it should be avoided where possible, but if I'm reading this right, it ought to work:

http://dojotoolkit.org/reference-guide/dojo/declare.html#id2

comment:5 Changed 8 years ago by bill

I guess you are right. I tried it and it seems to work (the regression tests pass). I'll check that in.

comment:6 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

In [26442]:

Make Calendar inherit from _Widget, like in 1.6, so it still get deprecated methods like setAttribute(), plus the deferred-connect event handlers like onMouseMove. Fixes #13814 !strict.

comment:7 Changed 8 years ago by Kenneth G. Franqueiro

Resolution: fixed
Status: closedreopened

I'm a bit confused but maybe I'm missing something: why was _CssStateMixin taken out of the inheritance chain? It was in the chain in 1.6.

I didn't think _Widget ever mixed that in, and if I'm reading correctly, it doesn't now, either - and neither does CalendarLite - so doesn't Calendar still need to mix that in?

comment:8 Changed 8 years ago by bill

Oops, I thought it was getting pulled in by _Widget, but you are right. I'll add it back.

comment:9 Changed 8 years ago by bill

Resolution: fixed
Status: reopenedclosed

In [26449]:

[26442] accidentally removed _CssStateMixin, putting it back, fixes #13814 !strict.

comment:10 Changed 8 years ago by Kenneth G. Franqueiro

In [26450]:

Updating comment. Refs #13814

Note: See TracTickets for help on using tickets.