#12857 closed defect (fixed)
Button: CSS class dijitNoIcon is not placed in the dijit.form.Button if iconClass is not defined
Reported by: | emorvill | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | Dijit - Form | Version: | 1.6.1 |
Keywords: | dijit form Button dijitIcon dijitNoIcon | Cc: | |
Blocked By: | Blocking: |
Description
I have added margin on class 'dijitIcon' on my own theme. So at the creation of dijit.form.Button, dijitNoIcon (equal to display:none) is not present in the DOM while I do not define iconClass attribute.
As iconClass is equal to "" (equals to false in javascript), the _setIconClassAttr is not launched at instanciation and dijitNoIcon is not added in the className of iconNode attach point.
_setIconClassAttr: function(/*String*/ val){ // Custom method so that icon node is hidden when not in use, to avoid excess padding/margin // appearing around it (even if it's a 0x0 sized <img> node) var oldVal = this.iconClass || "dijitNoIcon", newVal = val || "dijitNoIcon"; dojo.replaceClass(this.iconNode, newVal, oldVal); this._set("iconClass", val); }
It's works in defining by default :
iconClass: "dijitNoIcon"
You can see the problem in sample : dijit/tests/form/test_Button.html on second button (label = View)
Change History (6)
comment:1 follow-up: 2 Changed 10 years ago by
Milestone: | 1.6.2 → 1.7 |
---|---|
Owner: | changed from Douglas Hays to bill |
Summary: | dijit.form.Button :: CSS class dijitNoIcon is not placed in the dijit.form.Button if iconClass is not defined → Button: CSS class dijitNoIcon is not placed in the dijit.form.Button if iconClass is not defined |
comment:2 Changed 10 years ago by
If you modify all form templates containing a dijitIcon, the dojo.replaceClass in _setIconClassAttr will have as old value "" instead "dijitNoIcon" so dijitNoIcon will not be removed.
Maybe you can patch quickly without change the default value of iconClass like :
buildRendering: function(){ this.inherited(arguments); dojo.setSelectable(this.focusNode, false); // New Line : Force the _setIconClassAttr to add dijitNoIcon : if(this.iconClass == ""){ this.iconClass = "dijitNoIcon"; } },
Why do you not want to patch in 1.6.2 ? It's a minor bug without possible regression !
See you
comment:3 Changed 10 years ago by
If you modify all form templates containing a dijitIcon, the dojo.replaceClass in _setIconClassAttr will have as old value "" instead "dijitNoIcon" so dijitNoIcon will not be removed.
Yah, agreed.
Why do you not want to patch in 1.6.2 ? It's a minor bug without possible regression !
My experience has taught me not to be so confident. Plus, I'm not expecting a 1.6.2 release at all.
comment:4 Changed 10 years ago by
OK, I'm going to go with your suggestion. I'm a bit uncomfortable that get("iconClass") will return "dijitNoIcon" instead of "", but realistically I don't expect anyone to call that, so it seems like a waste bytes to support a transform from "" <--> "dijitNoIcon".
comment:5 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [24818]) Set default iconClass as "dijitNoIcon" so that the icon node is explicitly display:none when there's no icon. Fixes #12857 !strict.
Also:
- fixed bug in Bidi.html test (checking position of icon when there was no icon)
- fixed bug/inconsistency where testing functions isVisible()/isHidden() would report a node as hidden if it was visible in the document but scrolled out of the viewport (actually, only if it was scrolled above the viewport, not below the viewport)
I'll take this since I added dijitNoIcon in [23631].
Your suggestion works but since there's a convention that iconClass="" means no icon, maybe instead I'll modify the templates to originally have dijitNoIcon. The other consideration though is that MenuItem and Accordion have simple:
declarations, so there's no dijitNoIcon handling at all.