2.x: popup behavior [#719952]

I'm looking at the 2.x javascript rewrite of the pop-up behavior and I don't really how it's hardcoded to certain attributes. If everyone agrees, I'd like to rewrite it to have an attribute property (or an array of attribute properties) from which to draw the pop-up content. I none are specified, i'll have it default to the name / description attributes as it is currently doing.

I realize that the openlayers_behavior_popup_popup_content function can be overriden, which is good, but the default implementation should be a little more general.

Comments

Comment #1

phayes CreditAttribution: phayes commented 20 February 2010 at 01:15

Assigned:

Unassigned

» phayes

P.S If people are amenable to this, I will code up a patch.

Comment #2

zzolo CreditAttribution: zzolo commented 20 February 2010 at 14:54

I agree with this. It is pretty critical to having behavior instances, and it just allows a flexibility that can be used in many ways. I do think its important to make sure defaults are set to what it currently is.

Comment #3

tmcw CreditAttribution: tmcw commented 20 February 2010 at 16:06

If there's a good way to figure this out, sure. I think it's very complex, and that simply typing in a field name in the behavior settings is a hack. Possibly another route is having the OpenLayers Data configuration let you select multiple fields for display, or openlayers data theming out part of what popups show, rather than javascript doing it.

Comment #4

phayes CreditAttribution: phayes commented 22 February 2010 at 18:11

Generally I think we should be concentrating on a good extensible API, and then work on how that applies to a user-interface.

A good solution for this might be to allow layers to declare their attributes. I would imagine something like:

    'projection' => array('4326'),
    'layer_type' => 'openlayers_layer_type_vector',
    'options' => array(
      'maxExtent' => openlayers_get_extent('4326'),
    ),
    'attributes' => array('title','description','whatever'),

Some layers might have this filled out by the layer type (ie wfs), other might have it done manually on a per-layer basis (ie. Vector), and others it might be too much of a pain to do it at all (ie. kml), while others it might not apply to (ie. wms). But for those that do implement it, it would provide enough of a hint to make creating a user-interface for behaviors that need this information quite a bit easier.

Comment #5

tmcw CreditAttribution: tmcw commented 24 February 2010 at 18:19

Component:

OpenLayers Behaviors

» OpenLayers API

I agree that that looks like a better way to do it. I think it'll be somewhat hard to do, because the vector layer type doesn't currently know about its attributes until the render() method is called, and that method isn't called on form updates, etc. Also, there might be a bit of work to be done as far as making sure that these attributes update automatically when views change. And, yes, I think that the biggest immediate concern is the API over the interface - and would like to add in that we should focus on exportability, and ensure that everything gets documented.

Comment #6

phayes CreditAttribution: phayes commented 24 February 2010 at 18:27

Awsome - i'll code up a patch in a few days.

Regarding the Vector type and attributes. I would say that for Vector, since it's such a broad layer type, the layer-type object should not be responsible for filling in the attribute array. Rather that would be the responsibility of the layer object. For WFS, we might make it the responsibility of the layer-type object, since it might reasonably be able to know this. Although yes I see that for WFS there might need to be some changes to how it gets built, and when in the chain it does this kind of attribute gathering.

Comment #7

zzolo CreditAttribution: zzolo commented 6 March 2010 at 21:56

Note that having attributes in features (especially from views layers) means we can easy have variable styles with the built in OpenLayers attribute replaces radius = '${some_field}'. I understand that this might be a performace issue. Dynamic styling: #710908: Support dynamic styles