[FIXED-1140]Ext.menu.Separator -> layout-issue (icon-config)

[tobiu]

hi team,

i found a code snippet in our app, where a menu-item looked like it had 2 icons at once, flying above each other.
so, i build a small testcase to isolate the issue:

just drop it into menu.js in the examples folder -> menu


Ext.onReady(function(){

var tb = new Ext.Toolbar();

tb.add({
text:'Button w/ Menu',
menu: {
defaults: {
icon: 'menu-show.gif'
},
items: [{
text: 'Aero Glass',
icon: 'list-items.gif'
},
'-',
{
text: 'Vista Black',
icon: 'list-items.gif'
}, {
text: 'Gray Theme'
}, {
text: 'Default Theme'
}]
}
});

tb.render('toolbar');
});


and take a close look at the aero-glass icon -> firebug.

the funny thing is, that with the default-config of icon, the Ext.menu.Separator also gets that config. Although it is not listed in the API, it is there since Ext.menu.Separator is an extension of Ext.menu.BaseItem.

so, i suggest that icon (and iconCls?) should be checked in the initComponent and be resetted if needed. if there are no plans to display them properly at least :)


kind regards
tobiu

edit: replaced toolbar with menu =) hope it got clear anyway.

[Jamie Avins]

Your issue has been entered in Trac for review. Thank you for the report.

[tobiu]

hi jamie,

i spent way to much time tracking it down.
The problem is not in the chain
Separator -> BaseItem -> Component

but comes from Menu -> Container

more in detail:

Ext.menu.Menu


applyDefaults : function(c){
if(!Ext.isString(c)){
c = Ext.menu.Menu.superclass.applyDefaults.call(this, c);
var d = this.internalDefaults;
if(d){
if(c.events){
Ext.applyIf(c.initialConfig, d);
Ext.apply(c, d);
}else{
Ext.applyIf(c, d);
}
}
}
return c;
}


so, here is the suggestion:


Ext.override(Ext.Container, {
applyDefaults : function(c){
var d = this.defaults;
if(d){
if(Ext.isFunction(d)){
d = d.call(this, c);
}
if(Ext.isString(c)){
c = Ext.ComponentMgr.get(c);
Ext.apply(c, d);
}else if(!c.events){
Ext.applyIf(c, d);
}else{
// bugfix für menu separators
// Ext.apply(c, d);
Ext.applyIf(c, d);
}
}
return c;
}
});

Ext.intercept(Ext.menu.Separator.prototype, 'initComponent', function() {
this.icon = '';
this.iconCls = '';
});


i guess the problem was, that applyDefauls was designed to serve classes too, that had no configs, so apply would fit. but with the possibility to add configs via defauls of a parent object, this can cause issues.

the main question i have now is, if changing this issue on component-level may cause negative side effects. i don't think so, but if the apply was chosen (instead of the applyIf) for good reasons, please let me know ;)



kind regards
tobiu