PDA

View Full Version : [FIXED][3.x] Menu extend bug, or need a clarification



jsakalos
29 Apr 2009, 12:34 AM
Now Ext.Menu changed to container so I would expect that I can extend it as any other component, applying items to this in initComponent. The following should work as it works for any other component:


<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<link rel="stylesheet" type="text/css" href="ext/resources/css/ext-all.css">
<script type="text/javascript" src="ext/adapter/ext/ext-base.js"></script>
<script type="text/javascript" src="ext/ext-all-debug.js"></script>
<title id="page-title">Title</title>
<script type="text/javascript">
Ext.BLANK_IMAGE_URL = 'ext/resources/images/default/s.gif';
MyMenu = Ext.extend(Ext.menu.Menu, {
initComponent:function() {
var config = {
items:[{
text:'Item 1'
},{
text:'Item 2'
},{
text:'Submenu'
,menu:{
listeners:{
itemclick:function() {
console.log(arguments);
}
}
,items:[{
text:'Subitem 1'
}]
}
}]
};
Ext.apply(this, Ext.apply(this.initialConfig, config));
MyMenu.superclass.initComponent.apply(this, arguments);
}
});
Ext.reg('mymenu', MyMenu);

Ext.onReady(function() {
var b = new Ext.Button({
text:'Menu'
,renderTo:document.body
,menu:{
xtype:'mymenu'
,listeners:{
itemclick:function() {
console.log(arguments);
}
}
// ,items:[{
// text:'Item 1'
// },{
// text:'Item 2'
// },{
// text:'Submenu'
// ,menu:{
// listeners:{
// itemclick:function() {
// console.log(arguments);
// }
// }
// ,items:[{
// text:'Subitem 1'
// }]
// }
// }]
}
});
});
</script>
</head>
<body style="margin:40px">
</body>
</html>


I expect that items are processed in the constructor of Ext.menu.Menu so the items added in initComponent are ignored.

Is it a bug or is it intentional? If it is intentional, I would advocate to make it consistent with other components and process items in initComponent. Or, is there a sound reason why items cannot be processed in initComponent?

mjlecomte
29 Apr 2009, 5:54 AM
I think the problem is MenuMgr has a limitation of not accepting xtypes. This was discussed in another thread where someone wanted to specify xtypes for menu...not even extension, just say "datemenu" for example.

I think get needs attention to recognize xtypes.



/**
* Returns a {@link Ext.menu.Menu} object
* @param {String/Object} menu The string menu id, an existing menu object reference, or a Menu config that will
* be used to generate and return a new Menu instance.
* @return {Ext.menu.Menu} The specified menu, or null if none are found
*/
get : function(menu){
if(typeof menu == "string"){ // menu id
if(!menus){ // not initialized, no menus to return
return null;
}
return menus[menu];
}else if(menu.events){ // menu instance
return menu;
// two below will always create Ext.menu.Menu, instead of accepting the xtype
}else if(typeof menu.length == 'number'){ // array of menu items?
return new Ext.menu.Menu({items:menu});
}else{ // otherwise, must be a config
//return new Ext.menu.Menu(menu);
return Ext.create(menu, 'menu');
}
},

flylaputa
29 Apr 2009, 5:59 AM
The problem has nothing to do with x-types, it fails even if you don't use x-types.

mjlecomte
29 Apr 2009, 6:05 AM
The problem has nothing to do with x-types, it fails even if you don't use x-types.

Did you try the posted code?

flylaputa
29 Apr 2009, 6:26 AM
Did you try the posted code?

Thank you for your suggestion. I tried it now and it doesn't work.

mjlecomte
29 Apr 2009, 6:32 AM
The OP posted a working showcase to test against. I tested my change against the OP's code and it worked fine. You would probably help the developers if you posted your own test case where it "doesn't work" whatever that means specifically.

Condor
29 Apr 2009, 6:48 AM
I expect that items are processed in the constructor of Ext.menu.Menu so the items added in initComponent are ignored.

I scanned the Ext.menu.Menu code quickly, but this doesn't seem the case. The only problem I see is that your code will fail if the initialConfig is an array instead of a config object.

But IMHO the isArray check should be moved to the constructor (see here (http://extjs.com/forum/showthread.php?p=323124#post323124)).

flylaputa
29 Apr 2009, 6:58 AM
The OP posted a working showcase to test against. I tested my change against the OP's code and it worked fine. You would probably help the developers if you posted your own test case where it "doesn't work" whatever that means specifically.

Thank you for your help, I have posted my original code here:
http://extjs.com/forum/showthread.php?p=323140#post323140

It seems that the code posted here is slightly different to my original problem that Saki was helping on.

jsakalos
29 Apr 2009, 7:52 AM
config is object having items property that is the array.

Nevertheless, I'd like to hear from devel team, if we should write Ext.menu.Menu extensions a different way or if this is a bug that will be fixed.


I scanned the Ext.menu.Menu code quickly, but this doesn't seem the case. The only problem I see is that your code will fail if the initialConfig is an array instead of a config object.

But IMHO the isArray check should be moved to the constructor (see here (http://extjs.com/forum/showthread.php?p=323124#post323124)).

mjlecomte
29 Apr 2009, 8:07 AM
I scanned the Ext.menu.Menu code quickly, but this doesn't seem the case. The only problem I see is that your code will fail if the initialConfig is an array instead of a config object.

But IMHO the isArray check should be moved to the constructor (see here (http://extjs.com/forum/showthread.php?p=323124#post323124)).


config is object having items property that is the array.

Nevertheless, I'd like to hear from devel team, if we should write Ext.menu.Menu extensions a different way or if this is a bug that will be fixed.

Sorry, I don't understand these remarks. The extension code will not even be recognized at all due to current code for get method.

Saki, did you try the code I posted?

Let's put this another way: instead of using an xtype Saki, in your usage example just do new yourMenuClass and it will probably work as is, it just won't work as a config blob with xtype.

jsakalos
29 Apr 2009, 8:13 AM
MJ, I believe that your code works. You see I'm not in an urgent need to find an override (if that would be the case I would probably have already written one).

I want conceptual answer from Jack or other devel team member, either:

1. Ext.menu.Menu must be extended other way as other components because ....
2. or, that is a bug an we will fix it.

Anyone?

mjlecomte
30 Apr 2009, 10:46 AM
I think the problem is MenuMgr has a limitation of not accepting xtypes. This was discussed in another thread where someone wanted to specify xtypes for menu...not even extension, just say "datemenu" for example.

That other thread popped back up on the radar:
http://extjs.com/forum/showthread.php?p=318634#post318634

evant
30 Apr 2009, 4:28 PM
Fixed in SVN, used MJ's fix here, Ext.create() to grab the menu.

jsakalos
30 Apr 2009, 11:25 PM
Thanks Evan, good to know as it makes user's life easier. :)

flylaputa
6 May 2009, 12:52 AM
Thanks, it's working now.