Ticket #215 (new enhancement)

Opened 6 months ago

Last modified 3 months ago

automatic GeoExt.Action map from GeoExt.MapPanel

Reported by: fredj Assigned to:
Priority: minor Milestone: 1.0
Component: GeoExt.Action Version: trunk
Keywords: Cc:
State: Needs Discussion

Description

It should be possible to write something like: 

var action = new GeoExt.Action({
    text: "max extent",
    control: new OpenLayers.Control.ZoomToMaxExtent()
});

var mapPanel = new GeoExt.MapPanel({
    border: false,
    renderTo: "div-id",    
    map: {
        maxExtent: new OpenLayers.Bounds(-90, -45, 90, 45)    
    },
    tbar: [action]
});

'action' is constructed without the map argument: the MapPanel should build the OpenLayers.Map and pass it to the action.

Attachments

geoext-215.patch (3.1 kB) - added by ahocevar on 02/02/10 19:43:17.
patch-test-215-A0.diff (2.2 kB) - added by elemoine on 02/03/10 09:39:11.

Change History

02/02/10 19:40:06 changed by ahocevar

  • state changed from None to Review.

Tests still pass. Thanks for any review.

02/02/10 19:43:17 changed by ahocevar

  • attachment geoext-215.patch added.

(follow-up: ↓ 3 ) 02/03/10 08:59:07 changed by elemoine

  • state changed from Review to Needs Discussion.

Andreas, your patch doesn't work if what's passed in the tbar option is a Toolbar including a menu, including menu items associated with GeoExt.Action. So the patch doesn't deal with all the cases, so I'm wondering what we should do. What do you think?

(in reply to: ↑ 2 ) 02/03/10 09:01:43 changed by ahocevar

Replying to elemoine:

Andreas, your patch doesn't work if what's passed in the tbar option is a Toolbar including a menu, including menu items associated with GeoExt.Action. So the patch doesn't deal with all the cases, so I'm wondering what we should do. What do you think?

We could bubble up the ownerCt hierarchy until we find a MapPanel, instead of assuming comp.ownerCt.ownerCt to be a map panel.

02/03/10 09:02:28 changed by ahocevar

Eric, can you provide a test or example with the cases that are not covered?

02/03/10 09:39:11 changed by elemoine

  • attachment patch-test-215-A0.diff added.

02/03/10 09:41:42 changed by elemoine

See patch-test-215-A0.diff. But for some reason the first test ("action set in a button") doesn't pass either. It looks like the button has no ownerCt yet when the render listener is called, so comp.ownerCt instanceof Ext.Toolbar nevers evaluates to true. I've no time to research this more at the moment.

02/03/10 09:56:43 changed by fredj

see also findParentByType() in Ext.Component

http://www.extjs.com/deploy/dev/docs/?class=Ext.Component

02/03/10 16:19:19 changed by ahocevar

  • owner set to ahocevar.

Thanks elemoine for the patch, and thanks fredj for the pointer. I'm looking into it again now.

(follow-ups: ↓ 9 ↓ 10 ) 02/03/10 17:07:27 changed by ahocevar

Ok, the reason for the 1st test failing is that in Ext 2.2.1, buttons and menu items in a toolbar don't get an ownerCt assigned.

Would it be ok to provide this new functionality, but document that it requires Ext >= 3?

(in reply to: ↑ 8 ) 02/03/10 17:11:23 changed by elemoine

Replying to ahocevar:

Ok, the reason for the 1st test failing is that in Ext 2.2.1, buttons and menu items in a toolbar don't get an ownerCt assigned. Would it be ok to provide this new functionality, but document that it requires Ext >= 3?

that's fine by me.

(in reply to: ↑ 8 ; follow-up: ↓ 14 ) 02/03/10 17:17:33 changed by tschaub

Replying to ahocevar:

Ok, the reason for the 1st test failing is that in Ext 2.2.1, buttons and menu items in a toolbar don't get an ownerCt assigned.

Glad you confirmed the same. Thought I was going nuts yesterday when I suggested this looked Ext 3 specific.

Would it be ok to provide this new functionality, but document that it requires Ext >= 3?

I think that sounds good as well.

02/03/10 19:19:54 changed by ahocevar

  • owner deleted.

Well, things are not that bright anyway. As soon as our action is part of a menu, it's a no-go also with Ext 3.0.0. But it works with Ext 3.1.0. I guess we need a different approach here. One thing that comes to my mind would be to have the MapPanel check its toolbars and look for actions, but this feels a bit weird. Sorry, I don't have time to investigate this further.

02/16/10 12:05:41 changed by ahocevar

Would it also be ok to write something like: 

var zoomToMaxExtent = new OpenLayers.Control.ZoomToMaxExtent();

var mapPanel = new GeoExt.MapPanel({
    border: false,
    renderTo: "div-id",    
    map: {
        controls: [zoomToMaxExtent],
        maxExtent: new OpenLayers.Bounds(-90, -45, 90, 45)    
    },
    tbar: [new GeoExt.Action({
        text: "max extent",
        control: zoomToMaxExtent
    }]
});

According to the API docs, this should be possible without any changes to the code.

03/03/10 18:22:00 changed by ahocevar

  • milestone changed from 0.7 to 1.0.

(in reply to: ↑ 10 ) 04/22/10 18:33:47 changed by mike3050

Replying to tschaub:

Replying to ahocevar:

Ok, the reason for the 1st test failing is that in Ext 2.2.1, buttons and menu items in a toolbar don't get an ownerCt assigned.

Glad you confirmed the same. Thought I was going home insurance quotes nuts yesterday when I suggested this looked Ext 3 specific.

Would it be ok to provide this new functionality, but document that it requires Ext >= 3?

I think that sounds good as well.

Yes it sounds ok to me.