Feature highlighting

[yvesm]

Feature highlighting is necessary for users to see where a specific feature is. There are currently two use cases that users would need highlighting for :

• query

When a user clicks on "i" (queryonclick widget; GetFeatureinfo) a list of results is returned usually in an Ext grid (see resultextgrid widget). However, it is impossible for a user to know where he clicked as none of the features returned in the grid are highlighted on the map.

A solution is to add a marker on a vector layer. However that is a bit cumbersome as the size of the marker may be either much larger than the bbox of the features returned (e.g. low zoomLevel and small tolerance) or the other way around. It would be useful to be able to configure the queryonclick widget to also allow for feature highlighting (in replacement of/combination with using a marker).

• shortcut/recenter/initialview

A shorcut widget (or its dynamic version, the recenter widget) can be used to zoom in on a feature. In their current implementations, it is impossible for the user to know which is the feature zoomed to except in the very obvious case of one big central feature. The reason for that is that the zoomTo action will zoom with the selected feature in the center of the viewport. In most cases however, the multiplicity of features makes it impossible to know which is the one zoomed to. The same applies to the initialview widget.

All widgets that provide zoomTo type features should have a provision to highlight the feature(s) zoomed to.

See also #12

[yvesm]

I modified the behavior for initialview (patch attached). I take it for granted that people that will use an initialview widget will want to highlight the feature because it must be clear which feature it is we are zooming in on. What I've done is use the "select" OpenLayers style by default for highlighting, but integrators are allowed to override fillColor, strokeColor and strokeWidth in the initialview options.

Flaws (?) :

• I'm not using the current featureserver layer if any (I'm building a temp vector layer)
• no unhighlight mechanism is provided; once you get to the map with an initialview, your feature is highlighted until forever

For the shortcut and recenter widgets, should we have a "highlight" option in the widget and if so how do we provide the user the ability to unhighlight selected zoomed to features ?

[jlacroix]

Because of the 2 flaws you described I think this feature should be an option of the initialview widget instead of the default behavior. Can you change that?

[yvesm]

Changes made for initialview/recenter/shortcut. See 3 patches attached to this ticket.

INITIALVIEW

Instructions : test the highlight on initialview in the complex_workspace sample : comment out the line <!--highlight>true</highlight--> in the config.xml file to activate select on initialview

RECENTER

Instructions : use the BDGA sample to test that. Instead of fiddling with feature highlighting for the mapfishrecenter widget, I came across a particular recenter feature called showCenter; instead of adding a <highlight> tag in the configuration, one can just tick an option in the widget UI, thereby allowing the user to easily turn off "feature highlighting"; it's not feature highlighting, but rather an intelligent marker that vanishes when we change widgets.

I also added a config option for minChars in the UI; uncomment the minchars and maxchars tags; default minchars = 0 and maxchars = 10.

NOTE : for some reason the search is broken. Looks like dev4's FS service spits latin1 chars and that ilike doesn't work.

SHORTCUT

Instructions : test the highlight on shortcut in the shortcut sample : comment out the line <!--highlight>true</highlight--> in the activateresources sub-sample config.xml file to activate select on shortcut; same thing in the "intoolbar" sub-sample, except in this case a new vector layer is added if highlight (on cities or roads) is set to true.

NOTES : in the activateresources sub-sample, If you want to see the results, comment out maxScale and minScale for testing in R_BDGA_ROUTE_L_ARC and BDGA_WFS_CITIES; maybe there should be an autoactivate of the layers then ?; depending on zoom level you might have to zoom in more to see the highlighted feature show up (e.g. zoom in on road 138 it should be highlighted; shortcut on road 111 and you won't see it highlighted unless you zoom in once

MAIN ISSUE WITH FEATURE HIGHLIGHTING : we need a button to unselect everything (or someone has to complete my code).

Changes also made in i18n for recenter (not in patch)

[adube]

Comments on the initialviewhilite.patch

• NameSpace : there should be more use of GeoPrisma namespaces for the function names, like 'selectFeature' could be 'GeoPrisma.Widget.InitialView.SelectFeature'
  

• The select feature control should not be always activated. It should be deactivated as soon as the feature is unselected (by any mean (automatically and/or manually)).
  

• Bug : while the FeatureControl is active, it could be possible to 'click' on a new feature to select an other one. It mustn't be the case since it could conflict with other tools. Deactivating the handler of the SelectFeature control should do the trick.
  

• That's just my opinion, but the widget could have its InitialView.js file since more and more features (lines of code) are added. It would be cleaner and simpler.
  

• The 'loadend' event should be unregistered as soon a the map is recentered, else the selection of the same feature will always be made on every 'loadend'. We need to do this only once.
  

That's about it. More comments coming for the other patches. Thanks a lot Yves for your work.

[yvesm]

More recent patch

[yvesm]

More recent patch

[yvesm]

More recent patch

[adube]

Comments on the shortcuthilite.patch

• The detection of the vector layer should be done on widget creation.
  
• The creation of a new SelectFeature control or !Vector layer should be done on widget creation.
  
• Style should be configurable : currently hardcoded
  
• Functions could be part of the Class object and not inside of other functions, like 'callback'. Here, unlike in the InitialView widget, it doesn't really matter but it would be cleaner to have them as members of the class
  
• As Yves mention, no 'unselect' support. We could register a "mapmove" and unselect it as soon as the map moves. Otherwise, I consider this a blocker (we can't leave those features like that).

Again, thanks for the work.

[yvesm]

Partial fix (deactivate OK; unregister no)

[yvesm]

• NameSpace : there should be more use of GeoPrisma namespaces for the function names, like 'selectFeature' could be 'GeoPrisma.Widget.InitialView.SelectFeature'
  

I tried a dumb search/replace and it did not work; need some coaching on that

• The select feature control should not be always activated. It should be deactivated as soon as the feature is unselected (by any mean (automatically and/or manually)).
  

• Bug : while the FeatureControl is active, it could be possible to 'click' on a new feature to select an other one. It mustn't be the case since it could conflict with other tools. Deactivating the handler of the SelectFeature control should do the trick.
  

The last initialview patch solves this

• That's just my opinion, but the widget could have its InitialView.js file since more and more features (lines of code) are added. It would be cleaner and simpler.
  

Will do, but later

• The 'loadend' event should be unregistered as soon a the map is recentered, else the selection of the same feature will always be made on every 'loadend'. We need to do this only once.
  

I haven't been able to do that. I tried unregistering in the callback and the selectFeature function to no avail. I don't know how to do that.

[yvesm]

Looking at the JS code I notice there is also the highlight/unhighlight functions that could be used instead of the select/unselect. I tested it with this (InitialView.xslt lines c. 166)

 FeatureSelectControl.highlight(layer.features[i]);
 alert("Unhighlighting");
 FeatureSelectControl.unhighlight(layer.features[i]);
 break; 

instead of this

 FeatureSelectControl.select(layer.features[i]);
 break; 

and it works. All we'd need to do is listen to whatever map move (zoom/pan etc.) and unlighlight then. IMO, it makes more sense to highlight/unhighlight than to select/unselect. I know the features aren't in the table of selected features if we use highlight but I think that's a better behaviour because we really do not want to select features, just highlight them. I have to find now how to listen to events.

[yvesm]

I think I have something acceptable now. I added a callback2 that unhighlights on movenend. See patch InitialView-callback2.diff for details. Is it fin to immediately unhighlight the feature ?

[adube]

Yves,

I tested the lasted patch. I confirm that the wanted behavior works well. Here are my comments about it (with some that are the same as before) :

• no need for the SelectFeature control activation at all (remove all .activate() and .deactivate() methods). While testing, I found out that when we manually trigger .select() or .highlight function, we don't need the control to be active. So we'll always keep it deactivated.
• in zoomInitialViewCallback and the other 2 callback functions, the name oFeatures is used. For the zoomInitialViewCallback it's ok since it stays in scope, but it's not the case in the other 2 callback functions. oFeatures is not specific enough to be used as a "global" variable name. Currently, it's not a problem since it's working well and nothing else uses oFeatures but it could become one in the future. An easy way to work around this issue is to give a more unique name to the variable (with namespaces for example).
• since we use a oFeatures variable, it is true that it's ok to use .highlight() instead of .select() since we manually keep track of the features we're working with, but either would be ok. Even more, if we used .select(), we could avoid the use of oFeatures in callback2 and use the selectedFeatures array directly in the layer.
• namespaces should be mandatory (same)
• unregistering events should be mandatory (same). Currently, the callback2 function is called every time the map moves and does nothing, which is an issue.
• one last thing more InitialView specific : since it's only possible to use the InitialView widget on load, would it be useful to destroy all the objects and events it uses right after use ? Julien ?

That's close to an end. Thanks a lot for your work.

[yvesm]

Replying to adube:

Yves,

I tested the lasted patch. I confirm that the wanted behavior works well. Here are my comments about it (with some that are the same as before) :

• no need for the SelectFeature control activation at all (remove all .activate() and .deactivate() methods). While testing, I found out that when we manually trigger .select() or .highlight function, we don't need the control to be active. So we'll always keep it deactivated.
• in zoomInitialViewCallback and the other 2 callback functions, the name oFeatures is used. For the zoomInitialViewCallback it's ok since it stays in scope, but it's not the case in the other 2 callback functions. oFeatures is not specific enough to be used as a "global" variable name. Currently, it's not a problem since it's working well and nothing else uses oFeatures but it could become one in the future. An easy way to work around this issue is to give a more unique name to the variable (with namespaces for example).

OK thanx. Do you suggest a namespace in particular ?

• since we use a oFeatures variable, it is true that it's ok to use .highlight() instead of .select() since we manually keep track of the features we're working with, but either would be ok. Even more, if we used .select(), we could avoid the use of oFeatures in callback2 and use the selectedFeatures array directly in the layer.

Wouldn't using the selection of a feature be 1) misleading and 2) potentially interfering ? What if we have other objects selected in the layer ? That's the reason I wanted to use highlight.

• namespaces should be mandatory (same)
• unregistering events should be mandatory (same). Currently, the callback2 function is called every time the map moves and does nothing, which is an issue.

How do i unregister the event after *one* moveend ? Should I keep an array or some global boolean somewhere ?

• one last thing more InitialView specific : since it's only possible to use the InitialView widget on load, would it be useful to destroy all the objects and events it uses right after use ? Julien ?

That's close to an end. Thanks a lot for your work.

[adube]

OK thanx. Do you suggest a namespace in particular ?

Geoprisma.Widget.InitialView

Wouldn't using the selection of a feature be 1) misleading and 2) potentially interfering ? What if we have other objects selected in the layer ? That's the reason I wanted to use highlight.

Technically, it won't make any difference. Two select feature controls should never be activated at the same time (two that actually 'select') without being able to work together. In this case, no other SelectFeature control will ever be active at the same time so it doesn't matter. Keeping the .highlight() method is okay.

How do i unregister the event after *one* moveend ? Should I keep an array or some global boolean somewhere ?

Normally, it should be done inside the callback functions.

[yvesm]

With namespace but without unregistering movened

[yvesm]

I've added namespacing, but I can't unregister callback2 : attachment:initialview-ns.patch​

Any idea how I can do the final unregister ?

[yvesm]

r768

Committed changes for :

• InitialView : remaining issue = the callback2 function is called every time the map moves and does nothing

• MapFish Recenter : see commit message

[yvesm]

r798