Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13374 closed defect (worksforme)

hasDownArrow property of dijit.form.FilteringSelect does not propagate correctly when build a sub-class.

Reported by: timdp Owned by: Douglas Hays
Priority: lowest Milestone: future
Component: Dijit - Form Version: 1.6.0
Keywords: dijit form Cc:
Blocked By: Blocking:

Description

I'd like to extend FilteringSelect? a little, but I've hit a problem. The hasDownArrow property can be used to disable the drop-down arrow, but if I set this to false in my extended version of FilteringSelect?, it is ignored. Bizarrely, if I pass hasDropDown: false into the constructor (which is inherited), the arrow is suppressed.

I've attached a test case, just uncomment hasDownArrow: false, to see the correct behaviour.

Attachments (1)

hasDropDown.html (1.8 KB) - added by timdp 8 years ago.
test case

Download all attachments as: .zip

Change History (7)

Changed 8 years ago by timdp

Attachment: hasDropDown.html added

test case

comment:1 Changed 8 years ago by ben hockey

this is because visually, the starting state is to assume hasDownArrow is true. _setHasDownArrowAttr changes that state but doesn't get called unless a value is passed to the constructor. if you change the default value for your class then you need to do something to update the default visual state to match.

you could add a buildRendering similar to this to your custom widget:

buildRendering: function () {
  this.inherited(arguments);
  this._buttonNode.style.display = "none";
}

comment:2 Changed 8 years ago by Douglas Hays

Resolution: invalid
Status: newclosed

comment:3 Changed 8 years ago by timdp

Resolution: invalid
Status: closedreopened

I implemented the above, but had to use postCreate, not buildRendering (can't style a non-existent DOM node). I see no reason why as a developer I should need to understand the implementation of a class I wish to extend before I can extend it. That severely compromises object oriented principles.

I get the feeling there may be an endemic problem here as the same problem exists for fetchProperties. If I set that as an attribute of my sub-class, no alphabetisation, if I set the exact same attribute in my sub-class (this.fetchProperties) in postCreate, it works. I really think there's a problem with the mixin somewhere, please can you take another look?

comment:4 Changed 8 years ago by Douglas Hays

Milestone: tbdfuture
Priority: normallowest
severity: normaltrivial

comment:5 in reply to:  3 Changed 8 years ago by ben hockey

Resolution: worksforme
Status: reopenedclosed

Replying to timdp:

I implemented the above, but had to use postCreate, not buildRendering (can't style a non-existent DOM node). I see no reason why as a developer I should need to understand the implementation of a class I wish to extend before I can extend it. That severely compromises object oriented principles.

here's a working version using buildRendering - http://jsfiddle.net/PvmvR/ did you properly copy my code above?

if you extend a class and change the default values for a property its not unreasonable that you might need to understand the implications of making that change. while it is possible to implement a widget in such a way that it checks every single property during its initialisation, its not very efficient. certain efficiency can be gained by making assumptions about the default state of the widget and then respond to any changes to that default state. you can take a look at these for some general help about widgets and their lifecycles. http://dojotoolkit.org/reference-guide/quickstart/writingWidgets.html#quickstart-writingwidgets http://dojotoolkit.org/reference-guide/dijit/_Widget.html#lifecycle

I get the feeling there may be an endemic problem here as the same problem exists for fetchProperties. If I set that as an attribute of my sub-class, no alphabetisation, if I set the exact same attribute in my sub-class (this.fetchProperties) in postCreate, it works. I really think there's a problem with the mixin somewhere, please can you take another look?

i'm not following what your problem is here. if you need help you can use the mailing list. if you find a bug then open another ticket. so far i haven't seen anything that's a bug. i know people get offended when bugs are closed but this discussion is more appropriate on the mailing list. after discussing this on the mailing list it will be easier to determine if you have a bug or if its just a misunderstanding.

comment:6 Changed 8 years ago by bill

Note the comment in _WidgetBase.applyAttributes(), which runs on widget creation:

Skips over blank/false attribute values, unless they were explicitly specified
as parameters to the widget, since those are the default anyway,
and setting tabIndex="" is different than not setting tabIndex at all.

I sometimes consider changing that the call all custom setters, or just to call set("foo", ...) for every single parameter, but it would be a big change and I'm not sure if it would be better or worse, or even if it would work (especially given the issue with attributes like tabIndex). So I consider the current behavior an implementation decision rather than "an endemic problem".

Note: See TracTickets for help on using tickets.