Opened 9 years ago

Closed 8 years ago

#2 closed task (fixed)

Implement LegendPanel widget

Reported by: bartvde Owned by: bartvde
Priority: major Milestone: 0.5
Component: GeoExt.LegendPanel Version:
Keywords: Cc: dwins
State: Commit

Description

I've started to work on a GeoExt.widgets.LegendPanel which will be a legend panel firstly aimed at WMS layers. There is still some work to be done though.

I'll attach a preliminary version now if anybody wants to have a peek at it.

Example usage:

{     
    title: 'Legend',
    autoScroll: true,
    xtype: 'gx_legend',
    map: map,
    ascending: false,
    labelCls: 'startwmc',
    bodyStyle: 'padding-top: 10px;',
    cls: 'popup-variant1',
    baseCls:'x-tab'
}

Attachments (9)

LegendPanel.js (11.2 KB) - added by bartvde 9 years ago.
LegendPanel widget (not in patch form yet)
legendpanel.patch (12.1 KB) - added by dwins 8 years ago.
LegendPanel_new.js (13.6 KB) - added by bartvde 8 years ago.
some bug fixes done in LegendPanel since original attachment
legendpanel.2.patch (55.6 KB) - added by dwins 8 years ago.
legendpanel.3.patch (21.6 KB) - added by dwins 8 years ago.
ticket2.patch (27.5 KB) - added by bartvde 8 years ago.
changed static to dynamic, add support for changing legendURL property, more tests
ticket2.2.patch (27.9 KB) - added by bartvde 8 years ago.
updated patch for displayInLayerSwitcher
ticket2.3.patch (26.6 KB) - added by bartvde 8 years ago.
updated patch for different layer order management
2.patch (26.5 KB) - added by tschaub 8 years ago.
legend panel

Download all attachments as: .zip

Change History (34)

Changed 9 years ago by bartvde

LegendPanel widget (not in patch form yet)

comment:1 Changed 8 years ago by dwins

I'm attaching a patch (for geoext, not for the .js file already attached here) based on the provided patch, but with a couple of changes:

  • moved from GeoExt.widgets.LegendPanel to GeoExt.LegendPanel
  • added support for customizing the configuration of the individual layer legend panels (via a new configuration option, childDefaults)
  • resolved some layout problems (ie, collapsible legendpanels would leave floating headers) by defining GeoExt.LegendPanel = Ext.extend(... rather than defining the constructor independently of the extend() call.

Changed 8 years ago by dwins

Changed 8 years ago by bartvde

some bug fixes done in LegendPanel since original attachment

comment:2 Changed 8 years ago by bartvde

I'll try and work on this in the next couple of days.

comment:3 Changed 8 years ago by dwins

  • Cc dwins added

I'm updating the patch I attached earlier to include support for layer reordering.

Changed 8 years ago by dwins

Changed 8 years ago by dwins

comment:4 Changed 8 years ago by dwins

Attaching a further updated patch. this one:

  • includes tests and examples
  • pulls in changes from barts update (except for some that depend on mapready events, which don't seem to be in trunk yet. i haven't heard anything about these...)

comment:5 Changed 8 years ago by bartvde

Thanks David, sorry the mapready events were something temporary and it never made it to trunk. It was discussed on the ML. I'll have a look at your patch as soon as I find an hour.

comment:6 Changed 8 years ago by bartvde

  • Owner set to bartvde

comment:7 Changed 8 years ago by bartvde

  • State changed from Needs more work to Review

The new patch depends on:

1) GeoExt.MapPanel.guess from ticket:22 2) ticket:65 3) ticket:66

Please review.

comment:8 follow-up: Changed 8 years ago by tcoulter

Hey, do we know "static" is a Javascript reserved word? It doesn't mean anything to Javascript interpreters, though some editors seem to flag it as erroneous (Eclipse, for example). Should we change that variable?

More info here: http://javascript.about.com/library/blreserved.htm http://www.isi.edu/div3/helpdesk/html-dictionary/netscape/javascr/reserve.HTM http://www.javascripter.net/faq/reserved.htm

comment:9 follow-up: Changed 8 years ago by tcoulter

Note: This appears to be dependent on #66.

comment:10 in reply to: ↑ 9 Changed 8 years ago by bartvde

Replying to tcoulter:

Note: This appears to be dependent on #66.

Yes and a part of #22 and #65 as well as I wrote 2 comments above.

comment:11 in reply to: ↑ 8 Changed 8 years ago by bartvde

Replying to tcoulter:

Hey, do we know "static" is a Javascript reserved word? It doesn't mean anything to Javascript interpreters, though some editors seem to flag it as erroneous (Eclipse, for example). Should we change that variable?

More info here: http://javascript.about.com/library/blreserved.htm http://www.isi.edu/div3/helpdesk/html-dictionary/netscape/javascr/reserve.HTM http://www.javascripter.net/faq/reserved.htm

Makes sense to change it then for sure, thanks for the catch, how about dynamic with a default of true?

comment:12 Changed 8 years ago by tcoulter

"dynamic" sounds good to me.

Changed 8 years ago by bartvde

changed static to dynamic, add support for changing legendURL property, more tests

comment:13 Changed 8 years ago by tschaub

Please confirm that the new legend panel 1) reorders itself when layers are reordered on the map (as the old one did), and that 2) layers with displayInLayerSwitcher: false are *not* shown (at least by default).

I haven't looked closely, but a quick glance at the behavior of this panel in an old app shows these two changes.

Thanks for the work on this.

comment:14 Changed 8 years ago by tschaub

  • Component changed from data to widgets.LegendPanel

comment:15 Changed 8 years ago by tschaub

Ok, guessing this is #65. Apologies for not reading about dependencies.

comment:16 Changed 8 years ago by bartvde

Tim, you're right about displayInLayerSwitcher, I'll add that. Indeed, reordering depends on #65

Changed 8 years ago by bartvde

updated patch for displayInLayerSwitcher

comment:17 Changed 8 years ago by dwins

After applying the patches from #65 and #66, I am seeing some JavaScript errors in examples/legendpanel.html in FF3.0.10. I'll dig a bit more to see if I can characterize them.

comment:18 Changed 8 years ago by dwins

The problem was that I mis-applied the patch for #65 (I was applying manually since GNU patch had problems with the one provided). With properly applied patch, the legendpanel looks good to me.

comment:19 Changed 8 years ago by bartvde

  • State changed from Review to Needs more work

Changed 8 years ago by bartvde

updated patch for different layer order management

comment:20 Changed 8 years ago by bartvde

  • State changed from Needs more work to Review

Changed 8 years ago by tschaub

legend panel

comment:21 Changed 8 years ago by tschaub

This looks great Bart. Thanks for all the work.

What do you think about keeping the same "module" names as Ext for now? Instead of adding a GeoExt.legend module and only putting Image and WMS in there, all of LegendPanel, LegendImage, and LegendWMS could go in the GeoExt module. Alternatively, if we want to create a new module, I think the LegendPanel belongs in there as well.

I put up an alternative patch for #66. I'll look tomorrow with a clear head at the "order" change - not sure we actually need the update there.

comment:22 Changed 8 years ago by bartvde

  • State changed from Review to Commit

Thanks Tim, your changes look good to me, thanks for the catches. Please commit.

comment:23 Changed 8 years ago by tschaub

Hey Bart. I meant to mark this commit for you - as it is largely your work. Thanks for putting it together. I'll commit in a couple days if you don't get to it, but I think you should take this.

comment:24 Changed 8 years ago by bartvde

Hey Tim, great I'll commit this tomorrow (today I am on the road).

comment:25 Changed 8 years ago by bartvde

  • Resolution set to fixed
  • Status changed from new to closed

(In [867]) add LegendPanel component with currently support for WMS layers r=tschaub thanks to dwins and tcoulter (closes #2)

Note: See TracTickets for help on using tickets.