Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#272 closed enhancement (fixed)

create a tree loader for WMS GetCapabilities

Reported by: bartvde Owned by: bartvde
Priority: major Milestone: 1.0
Component: GeoExt.tree Version: trunk
Keywords: Cc:
State: Review

Description

so people can easily build GUI components to add WMS layers (in a nested structure) from a TreePanel.

Attachments (3)

ticket272.patch (11.5 KB) - added by bartvde 7 years ago.
ticket252.patch (8.8 KB) - added by bartvde 7 years ago.
new patch incorporating ahocevar's comments
ticket272.2.patch (12.0 KB) - added by ahocevar 7 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by bartvde

  • Owner changed from ahocevar to bartvde
  • Status changed from new to assigned

Changed 7 years ago by bartvde

comment:2 Changed 7 years ago by bartvde

  • State changed from None to Review

Tests pass in IE6 and FF 3.6, please review.

comment:3 follow-ups: Changed 7 years ago by ahocevar

  • State changed from Review to Needs more work

Great addition! One could argue that this loader should use a WMSCapabilitiesStore, but if someone just wants a tree representation of the capabilities doc instead of a grid representation, it makes sense to not have them create a store first.

Only two small things I would like you to change:

  • The name is not verbose enough. Can you change it to GeoExt.tree.WMSCapabilitiesLoader?
  • The way you build the tree in the example is a bit misleading. Can you instead do something like
    var root = new Ext.tree.Node({
        loader: new GeoExt.tree.WMSCapabilitiesLoader({...})
    });
    var tree = new Ext.tree.TreePanel({
        root: root
    });
    

comment:4 in reply to: ↑ 3 Changed 7 years ago by bartvde

Replying to ahocevar:

Great addition! One could argue that this loader should use a WMSCapabilitiesStore, but if someone just wants a tree representation of the capabilities doc instead of a grid representation, it makes sense to not have them create a store first.

I decided against this, since the structure can be hierarchical, i.e. nested, and Ext stores are not good for representing these structures AFAIK. The example is a flat list, but it can be nested and the OpenLayers 2.9 capabilities parser now supports nested structures, whereas 2.8 did not.

I'll incorporate your suggestions, thanks.

comment:5 Changed 7 years ago by tschaub

One way to bring a store into the mix would be to have the caps parser maintain the layer "path." I can't recall without looking if we do this currently. But a tree could be constructed if records in the store included a "path" field with layer names (ideally an array, but if that caused record trouble, then a string delimited by "," or something [assuming commas in layer names were encoded]).

comment:6 Changed 7 years ago by bartvde

  • State changed from Needs more work to Needs Discussion

If you look at this post/reply from bmoeskau he does not seem to recommend using a store: http://stackoverflow.com/questions/1689828/extjs-data-store-for-tree

Changed 7 years ago by bartvde

new patch incorporating ahocevar's comments

comment:7 in reply to: ↑ 3 Changed 7 years ago by bartvde

  • State changed from Needs Discussion to Review

Replying to ahocevar:

Great addition! One could argue that this loader should use a WMSCapabilitiesStore, but if someone just wants a tree representation of the capabilities doc instead of a grid representation, it makes sense to not have them create a store first.

Only two small things I would like you to change:

  • The name is not verbose enough. Can you change it to GeoExt.tree.WMSCapabilitiesLoader?
  • The way you build the tree in the example is a bit misleading. Can you instead do something like
    var root = new Ext.tree.Node({
        loader: new GeoExt.tree.WMSCapabilitiesLoader({...})
    });
    var tree = new Ext.tree.TreePanel({
        root: root
    });
    

Changed in my latest patch (The patch name is misleading, my mistake, i.e. 252 instead of 272).

Tests pass in IE6 and Firefox 3, please review.

Changed 7 years ago by ahocevar

comment:8 Changed 7 years ago by ahocevar

  • State changed from Review to Commit

Bart, your patch came without the unit tests. I added and modified them from your sandbox. Also, I played around with the example. Please decide whether you prefer my example with the checkboxes or yours to be committed.

Great work, please commit.

Note, however, that at some point before the next release, we should change the WMSCapabilitiesLoader under to hood to use a WMSCapabilitiesStore, which should have a "path" property on the LayerRecords. One advantage would be that we wouldn't need the createWMSLayer method and the layer attribute on the nodes, and could use LayerNodes instead. I have discussed this in the past with Tim already. It would also make it easier to build trees from maps stored using OpenLayers.Format.OWSContext. Can you create a ticket for that please?

comment:9 Changed 7 years ago by bartvde

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

(In [2210]) create a tree loader for WMS GetCapabilities, r=ahocevar (closes #272)

comment:10 Changed 7 years ago by fredj

  • Resolution fixed deleted
  • State changed from Commit to Review
  • Status changed from closed to reopened

The example is not displayed in http://www.geoext.org/examples.html, please add a docstring in examples/wms-tree.js something like:

Index: examples/wms-tree.js
===================================================================
--- examples/wms-tree.js	(revision 2210)
+++ examples/wms-tree.js	(working copy)
@@ -1,3 +1,17 @@
+/**
+ * Copyright (c) 2008-2010 The Open Source Geospatial Foundation
+ *
+ * Published under the BSD license.
+ * See http://svn.geoext.org/core/trunk/geoext/license.txt for the full text
+ * of the license.
+ */
+
+/** api: example[wms-tree]
+ *  WMS Capabilities Tree
+ *  ---------------------
+ *  Create a tree loader from WMS capabilities documents.
+ */
+
 var tree, mapPanel;
 
 Ext.onReady(function() {

comment:11 Changed 7 years ago by bartvde

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

(In [2211]) add docstring to the wms-tree example, thanks to fredj for the catch (closes #272)

comment:12 Changed 7 years ago by ahocevar

  • Type changed from task to enhancement
Note: See TracTickets for help on using tickets.