Ticket #1596 (closed feature: fixed)

Opened 6 years ago

Last modified 5 years ago

should selectFeature control allow for seperate hover and click events

Reported by: hpbrantley Owned by:
Priority: major Milestone: 2.8 Release
Component: Control.SelectFeature Version: 2.6
Keywords: hover click Cc:
State: Commit

Description

I am in the need to have seperate actions on hover and click.

See  http://brantleys.com/maps/newSelectFeature.html for a working model.

Should/could this be a patch or an add in?

Attachments

patch-1596-r9052-A0.diff Download (15.7 KB) - added by elemoine 6 years ago.
patch-1596-r9170-A0.diff Download (15.0 KB) - added by pgiraud 5 years ago.
patch updated
highlight.patch Download (15.4 KB) - added by ahocevar 5 years ago.
highlight.2.patch Download (15.3 KB) - added by ahocevar 5 years ago.

Change History

Changed 6 years ago by euzuro

  • priority changed from minor to critical

Changed 6 years ago by euzuro

  • priority changed from critical to major

Changed 6 years ago by euzuro

  • milestone changed from 2.7 Release to 2.8 Release

lack of activity

Changed 6 years ago by igrcic

Hi,

i needed this funcionality also..as many other i believe. So after i've investigated a bit i found out 2 solutions to this problem:

1) by using selectFeature control's callback methods its pretty much possible to do what we want

var control= new OpenLayers.Control.SelectFeature(vectorLayer,
	{
	clickout: true,
	toggle: true,
	hover: true,
	callbacks: {
	'over':feature_hover, 'out':feature_out
	});

Usually what we need is to apply different style to a feature when we hover over it, without selecting it. We do that by redrawing that feature with another style - inside callback functions, something like this:

function feature_hover(feature){
	    var hoverStyle={
                externalGraphic: ...,
		graphicWidth: ...
	        graphicOpacity:1,
		cursor:'pointer',
                ....
            };

	vectorLayer.drawFeature(feature, hoverStyle);	
	//some additional functionality, like extracting attributes from eature	
	OpenLayers.Util.getElement('output').innerHTML = extractAttributes(feature);
}

We render it back to old style the same way in feature_out function. So thats the first solution to this problem.

2) second one would be to add one more boolean option to selectFeature api, for an example selectOnHover, and depending on its value we select/unselect feature in SelectFeature.js control functions overFeature/outFeature
if(this.selectOnHover) added

 overFeature: function(feature) {
...
if(this.selectOnHover)this.select(feature); //currently in my app i have this line commented out so i dont get feature selected on hover
...
}

    outFeature: function(feature) {
        if(this.hover) {
           if(this.selectOnHover) this.unselect(feature);
    ...
    }

For me its not a problem any more to do this things when i learned how, but i think it would be usefull for users to have this option included...dont you agree?

Changed 6 years ago by crschmidt

  • milestone changed from 2.8 Release to 2.9 Release

I feel that this is a discussion that is still ongoing on the list, and should be targeted towards a future release, not 2.8.

Changed 6 years ago by elemoine

Changed 6 years ago by adube

That last change is sweet. It accomplish everything we need to have two or more SelectFeatures controls at the same time, on the same layer. And the new "highlight" events make it possible to have temporary things happening at the same time, like displaying a popup on hover.

Great work Eric. You're going to make a lot of people happy with this fix.

Changed 6 years ago by elemoine

  • owner changed from tschaub to elemoine
  • state changed from Needs Discussion to Needs More Work
  • milestone changed from 2.9 Release to 2.8 Release

I'll try to give a complete description of this patch and have it review for 2.8. Chris, tell me if it's too late to change ticket Milestone to 2.8.

Changed 5 years ago by elemoine

  • state changed from Needs More Work to Review

Thanks for taking a look at the patch Alexandre. I'm marking this ticket as Review for 2.8.

Some notes on the patch:

A lot of people want to use two SelectFeature controls on the same layer, one selecting features on "click" and the other just highlighting features on "hover". This is for example to be able to (a) select features on "click" for future operations dealing with all the selected features, and (b) get information on individual features on "hover", both for unselected and selected features.

So far, we've been telling these people "use a select feature control for actually selecting features, and implement your own Feature handler-based control for highlighting and displaying feature information". I think this isn't an acceptable answer, as correctly implementing such a Feature handler-based control is not trivial to most users.

Alexandre and I have been working on finding an acceptable solution to that. We've been bouncing patches back and forth, and the one we've finally come up with basically boils down to this:

  • add a highlightOnly boolean property to the SelectFeature control, this property applies only in hover is true. If highlightOnly is true then hovered features are just highlighted (redrawn with the style configured in the control), they're not selected
  • with highlightOnly set to true, when moving out of a feature, the feature is not unhighlighted if the control is not the last highlighter of the feature. This to avoid this case:
Control #1, select on click, style: blue
Control #2, highlight on hover, style: yellow

Mouse over feature -> feature color changed to yellow (OK)
Click on feature -> feature color changed to blue (OK)
Mouse out of feature -> feature color changed to default orange (NOT OK)
  • add three event types in the SelectControl: beforefeaturehighlighted, featurehighlighted and featureunhighlighted. This is useful when the SelectControl is configured with highlighOnly set to true and user wants to be notified when the mouse goes over and out of features (for example to open and close popups)

With this patch, the SelectFeature control sets the properties _lastHighlighter and _prevHighlighter in the features it highlights. These two properties aren't defined in the prototype of OpenLayers.Feature.Vector as they are completely private to the SelectFeature. The Save Strategy also does this (with the _original property), so I didn't feel to guilty in doing it.

It is to be noted that the patch does the maximum not to keep references to controls within features, by setting the _lastHighlighter and _prevHighlighter to undefined when their content is no longer needed.

Changed 5 years ago by elemoine

  • state changed from Review to Needs More Work

Patch needs adjustments (because of [9116])

Changed 5 years ago by pgiraud

patch updated

Changed 5 years ago by pgiraud

  • state changed from Needs More Work to Review

(Minor) adjustments made in the proposed patch built against trunk (r9170). Please review. Relevant tests (functional and unit) pass in FF3 and IE6.

Changed 5 years ago by elemoine

  • owner elemoine deleted

Changed 5 years ago by ahocevar

Changed 5 years ago by ahocevar

  • state changed from Review to Commit

pgiraud: The thing I don't like about your patch is that it tags a control onto a feature (_prevHighlighter, _lastHighlighter). This will lead to memory leaks, since the control also references a dom element. In general, I don't think that tagging things onto objects is good practice, but I understand that it was a good option here to keep the patch as non-intrusive as possible.

I think that the highlight and unhighlight methods are useful, but I would not look for them on the selectFeature control (rather on a separate highlighter control or on the layer). And as they are now, it seems they only work if there is just one highlighting SelectFeature control and one selecting SelectFeature control. So I guess a more generic solution to this problem would be to have a feature keep track of the renderIntents it has been drawn with (e.g. in an array), instead of using this _prevHighlighter and _lastHighlighter tags. But this would be a lot of effort, and for now I think we are good with this solution.

I made a few changes to the patch to make things a bit nicer:

  • only tag the control's id to the feature, not the control itself
  • changed drawFeature in unhighlight to also work with feature.style and layer.style, not just with renderIntent.

If you're ok with these changes, please commit. I would just like to add that I am not super happy with this change, but I find a highlight method useful and see that a more generic solution would require more resources than there are available now.

Changed 5 years ago by ahocevar

  • state changed from Commit to Needs More Work

Wait, I just saw that using unhighlight without outFeature will cause unexpected results. New patch to come.

Changed 5 years ago by ahocevar

Changed 5 years ago by ahocevar

  • state changed from Needs More Work to Commit

My latest patch makes one more change: it moves code from outFeature into unhighlight, so unhighlight can also be used standalone (like select, unselect and highlight). And is a better counterpart to highlight.

pgiraud, if you're ok with this change plus the above, please commit  highlight.2.patch

Changed 5 years ago by ahocevar

pgiraud, please disregard highlight.2.patch. My thinking about unhighlight being the right counterpart of highlight was wrong. So please commit highlight.patch.

Changed 5 years ago by pgiraud

  • status changed from new to closed
  • resolution set to fixed

Closed in [9176].

Note: See TracTickets for help on using tickets.