Opened 8 years ago
Closed 8 years ago
#17040 closed defect (fixed)
dijit/form/Select not an instance of dijit/_WidgetBase
Reported by: | Brian Arnold | Owned by: | Douglas Hays |
---|---|---|---|
Priority: | undecided | Milestone: | 1.9 |
Component: | Dijit - Form | Version: | 1.9.0b2 |
Keywords: | Cc: | cjolif | |
Blocked By: | Blocking: |
Description (last modified by )
While I realize that instanceof
is not reliable for mixins and such, it is typically reliable to do things like object instanceof _WidgetBase
to determine that something is a widget, and in 1.8 and prior, this worked quite well pretty much across the board.
However, part of r30620 was a refactoring of _KeyNavMixin
and such, and it changed the way that dijit/form/Select
is declared to the following:
declare("dijit.form.Select" + (has("dojo-bidi") ? "_NoBidi" : ""), [_KeyNavMixin, _FormSelectWidget, _HasDropDown], {
You'll notice that now, the base class for dijit/form/Select
is _KeyNavMixin
, and not _FormSelectWidget
as it had been previously.
I can use object.isInstanceOf
and that works just fine, but this seems kind of broken. If I swap the order of _KeyNavMixin
and _FormSelectWidget
, the robot tests break, so it's not as trivial as a one line change, unfortunately.
Attachments (2)
Change History (7)
comment:1 Changed 8 years ago by
Description: | modified (diff) |
---|
Changed 8 years ago by
Attachment: | 17040.patch added |
---|
comment:2 Changed 8 years ago by
I tried to do a cleaner (but slightly bigger) patch where the inheritance order doesn't matter, and also there's no issue with the order of operations in postCreate(), which as a lifecycle creation method should really call this.inherited() before doing it's own thing. The main part of the change was rolling back the [31124] change to _HasDropDown.
It led to a strange problem in test_Select.html where mouseup on the first Select automatically selected Tennessee. Turns out it was that the mouseup listener setup in _FormWidgetMixin::_onFocus() calls this.focus(), which (after my change) calls _KeyNavMixin::focus() rather than _FormWidgetMixin::focus(), which then calls focusFirstChidl(). This is because Select fragilely extends _KeyNavMixin even though SelectMenu is the container of the items being navigated.
So then I had to workaround that problem, and things are working. See attached patch, does it look good to you? Although the change is bigger is seems safer long term, and given that the combination of _KeyNavMixin and _HasDropDown is a new thing for this release, I think it should go into 1.9.
comment:3 Changed 8 years ago by
Cc: | cjolif added |
---|
comment:4 Changed 8 years ago by
Bill, I tested this on several browsers and it's working well on each. Go ahead and commit this when you've finished testing.
add _WidgetBase to inheritance chain so instanceof works