Button: CSS class dijitNoIcon is not placed in the dijit.form.Button if iconClass is not defined

[emorvill]

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)

[bill]

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:

_setIconClassAttr: { node: "iconNode", type: "class" },

declarations, so there's no dijitNoIcon handling at all.

[emorvill]

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

[bill]

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.

[bill]

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".

[bill]

(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)

[bill]

(In [25556]) Fix display of checkmark icon for CheckedMenuItem, refs #12857, fixes #13260 !strict.