The attachment of popups and tooltips should be something like a nice list of checkboxes for which layers they're enabled on. Also, tooltip should probably be merged with pop up, since they mean the same thing to end users and a 'hover or click' dropdown would be far more clear and reflect the functionality far better.

Files: 
CommentFileSizeAuthor
#7 710770-popups-per-layer-07.patch7.12 KBzzolo
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 710770-popups-per-layer-07.patch.
[ View ]

Comments

phayes’s picture

Hmm.. Also note that pop-ups / tooltips need to reference an attribute from which to get content. This might change across layers.

Does the current IA allow for multiple instances of the same behavior type? If not, then we might run into this kind of a problem, where you want the same behavior with different parameters attached to different layers.

I disagree re: naming. I think a tooltip is pretty well understood as something you get when you hover-over, and a popup is something you get when you click. Although yes they could be combined with just the trigger being different.

tmcw’s picture

Yes, there's a possibility that popups should introspect views in order to determine where they can get their content. They will, however, support customization via javascript overriding of the function that makes their contents, most likely, and the choice of what appears as the title and what as the body text is exposed in the views OpenLayers Data display.

And, yeah, it's kind of a toss-up whether Pop up and Tooltip are geekwords or not. I think it might make sense to just have one thing, though, because that would make some further customizability (like choosing whether popups close when you click on a new one) more obvious and not-duplicated in both elements.

Currently there is no way to have multiple copies of one behavior on a map. How exactly this would work is a little difficult to determine.

zzolo’s picture

Component:OpenLayers Behaviors» OpenLayers API

I think tooltips and popups should be different. They are pretty distinct visually and in behavior. I think giving the user too many options will be confusing. I would say 99% of users want a popup or tooltip and an easy way to style/theme it.

Let's keep this issue to how to assign popups and tooltips to different layers and if they should be separate behaviors.

* This would probably mean some behavior instances: #719942: Multiple Behaivors on a single type per map
* As far as popup attributes: #719952: 2.x: popup behavior

tmcw’s picture

Status:Active» Postponed

This needs to be done after the multiple behavior instances ticket.

zzolo’s picture

Status:Postponed» Active

Instead of the multiple behavior instances, why not provide a set of checkboxes of layers in the popup behaviors form that allows users to choose which layers in that preset get popups? (same for tooltips)

tmcw’s picture

Yep, I think that's a good idea.

zzolo’s picture

Status:Active» Needs review
StatusFileSize
new7.12 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 710770-popups-per-layer-07.patch.
[ View ]

Heres a patch that allows for popups per vector layer. It should support backwards compatibility: if no layers are selected, then it stays the same. Also, this patch includes major cleaning up of the popup code.

tmcw’s picture

Okay, there are just a few things to work on in this patch. First off, it seems like a bad idea to insert code blocks like

+Drupal.openlayers = Drupal.openlayers || {};
+Drupal.openlayers.popup = Drupal.openlayers.popup || {};
+Drupal.openlayers.popup.popupSelect = Drupal.openlayers.popup.popupSelect || {};
+Drupal.openlayers.popup.selectedFeature = Drupal.openlayers.popup.selectedFeature || {};
+

Just so that the popup global can be namespaced. If more globals are namespaced the same block is required? Ideally there shouldn't even be a global, but the popup should be stored in a data() wrapper.

+      for (var i in options.layers) {
+        var selectedLayer = map.getLayersBy('drupalID', options.layers[i]);
+        if (typeof selectedLayer[0] != 'undefined') {
+          layers.push(selectedLayer[0]);
+        }
+      }

for ( x in a ) blocks are serious performance bottlenecks and unnecessary if you're dealing with an array.

This also introduces a few new misspellings - compatiability should be compatibility, defintion should be definition.

Besides that, this looks good. Despite my like for underscores, camelCase is the defacto for JavaScript.

zzolo’s picture

Committed.
http://drupal.org/cvs?commit=384888

I left the global namespace variables. I do not like them either, but there are other places we are doing this, and I am not entirely sure what is the best way to do this.

The for() is a good point but the unfortunately the conversion from PHP to JS casts them as objects, not arrays.

I have also done the same thing for tooltips.

Do we want to continue this ticket, as you mentioned originally we should merge tooltips and popups?

tmcw’s picture

Status:Needs review» Fixed

No, I think that there should be another ticket for merging tooltips and popups, and that OpenLayers tooltips are not going to cut it for the majority of uses. In that round of development, hopefully we can clear up the globals thing too.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.