#12278 closed enhancement (fixed)
_WidgetBase: fix inheritance issues with attributeMap
Reported by: | bill | Owned by: | billl |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | Dijit | Version: | 1.6.0b1 |
Keywords: | 1.7-mobile | Cc: | |
Blocked By: | Blocking: |
Description (last modified by )
attributeMap has issues with inheritance, in that it's hard for a mixin to add/override an attribute in an already existing attributeMap. In particular, the dojo.delegate() call hardcodes the name of the superclass, but mixins can't know the superclass' name.
This can be solved by using custom setters for everything, rather than attributeMap. However, that makes the code longer. So, one solution is to provide a custom setter generator as part of _WidgetBase. Same basic syntax as a single key/value entry in attributeMap. For example:
attributeMap: dojo.delegate(dijit.form._FormValueWidget.prototype.attributeMap, { maxLength: "focusNode" })
can instead be written as:
_setMaxLengthAttr: dijit.das("maxLength", "focusNode")
The downside of this approach is that we lose the meta-data, which would be a problem if (like in cujo) attributeMap could run in reverse, reflecting DOMNode changes (like clicking a checkbox or typing into an input) back into the widget javascript object.
Doug's suggestion was to change:
attributeMap: {foo: "", bar: "focusNode"}
into something like:
_fooAttrMap: "", _barAttrMap: "focusNode"
Attachments (1)
Change History (9)
Changed 11 years ago by
Attachment: | generatedSetter2.patch added |
---|
comment:1 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Summary: | _WidgetBase: custom setter generator → _WidgetBase: fix inheritance issues with attributeMap |
comment:2 Changed 11 years ago by
(In [23941]) Support for _mapXXXAttr attributes that specifying a mapping from a widget's XXX attribute to a DOMNode attribute/style/innerHTML etc.
Each _mapXXXAttr attribute/value is equivalent to a single key/value pair from attributeMap. The _mapXXXAttr API supercedes attributeMap, which is deprecated and will be removed in 2.0.
For backwards-compatibility, the widget attributes with attributeMap entries are processed first, followed by the attributes with _mapXXXAttr objects and _setXXXAttr() methods. The _mapXXXAttr objects / _setXXXAttr() methods are called according to the order the attributes are declared in the widget.
Refs #12278 !strict.
comment:3 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:4 follow-up: 5 Changed 11 years ago by
(In [23946]) Rather than having _mapXXXAttr: { ... } to replace attributeMap, allow _setXXXAttr to be defined as a hash/string/array value, in addition to being defined as a method. This avoids confusion when a widget has both a _mapXXXAttr and a _setXXXAttr.
There was some weirdness with "class" and "style" where the _setClassAttr() and _setStyleAttr() methods depended on _mapClassAttr/attributeMapclass? and _mapStyleAttr/attributeMapstyle? that I had to work around. Note: it seems like "style" is always applied to the widget root node.
Refs #12278 !strict.
comment:5 Changed 11 years ago by
just a thought... if this relationship was bi-directional, ie a property of a node mapped back to a widget property then using _setXXXAttr for this mapping might not be a good choice. for example, if you added support for _getXXXAttr to do the reverse of _setXXXAttr then you would need to repeat this map in _getXXXAttr. i don't happen to do this at all but just looking towards future possibilities.
Replying to bill:
(In [23946]) Rather than having _mapXXXAttr: { ... } to replace attributeMap, allow _setXXXAttr to be defined as a hash/string/array value, in addition to being defined as a method. This avoids confusion when a widget has both a _mapXXXAttr and a _setXXXAttr.
There was some weirdness with "class" and "style" where the _setClassAttr() and _setStyleAttr() methods depended on _mapClassAttr/attributeMapclass? and _mapStyleAttr/attributeMapstyle? that I had to work around. Note: it seems like "style" is always applied to the widget root node.
Refs #12278 !strict.
comment:6 Changed 11 years ago by
I am thinking about bi-directional support, although note that "bi-directional support" can mean multiple things.
Also, note that this.set("foo", bar)
also sets this.foo = bar
, so you don't need a _getFooAttr() method at all.
Anyway, I agree that _setXXXAttr() probably isn't an ideal name, but I needed to maintain backwards-compatibility and didn't want to hit that templatePath/templateString duality confusion where widgets have two variables that conflict with each other.
comment:7 Changed 11 years ago by
Keywords: | 1.7-mobile added |
---|
patch from before _Widget was refactored into _WidgetBase and _Widget