[OpenLayers-Dev] [OpenLayers-Trac] [OpenLayers] #1260: SelectFeature and OpenLayers.Feature.Vector.style["select"]

[Christopher Schmidt]

On Thu, Jan 17, 2008 at 02:49:06PM +0100, Andreas Hocevar wrote:
> Christopher Schmidt wrote:
> >On Thu, Jan 17, 2008 at 02:46:04PM +1100, Roald de Wit wrote:
> >  
> >
> >>- Control.SelectFeature adds the feature to the array of
> >>layer.selectedFeatures, it then calls layer.drawFeature(feature)
> >>- layer.drawFeature checks if the feature is in the list of
> >>selectedFeatures. If so, it fetches feature.style.select. If not, it
> >>takes feature.style.default.
> >>- when layer.drawFeature is called with a second parameter, that can be
> >>a style object or a name describing the render intent that is used to
> >>lookup the style from the hash.
> >>    
> >
> >Alternatively, Control.SelectFeature -- since it knows that the feature
> >is being selected -- is the one to check whether the feature has a
> >'feature.style.select', and draws it (using drawFeature) with that
> >style, rather than checking it in drawFeature. The end result is the
> >same, I think. I prefer for drawFeature to not depend on
> >selectedFeatures -- in my mind, that array is 'managed' by the
> >SelectFeature control,  so it should be the one doing the logic there.
> >
> >  
> 
> I was thinking about other classes that also draw features, like 
> Control.ModifyFeature. Many of them currently use layer.drawFeature. In 
> the future, they could all use renderer.drawFeature, and just pass the 
> renderIntent. 

Controls should not talk to Renderers. That's breaking an abstraction
barrier that I'm very much against breaking. Everything that the
controls do should go through the layer, I think. 

It's possible that this is not really true, and that the Renderer base
class -- accessible via layer.renderer -- is enough of an 'abstraction
barrier' that it can be treated as a sane abstraction level. I'm not
convinced of that, and until we make a concious decision otherwise, I
would encourage all Renderer functions to be used only within the Layer.  

> In that case, we could probably avoid code duplication 
> when doing fallback checks in renderer.drawFeature instead of every 
> other class that needs to draw features.

That's fine -- we can still do that check at the layer.drawFeature level
though, no?

> Legacy support for style hashes and OpenLayers.Style without a style map 
> should be done in Control.SelectFeature, Control.ModifyFeature etc., 
> because if there is no style map, a style instead of a renderIntent 
> string would be passed to renderer.drawFeature, and those classes might 
> have a style property (like selectStyle in Control.SelectFeature).

Hm, okay.

> Does this make sense?

I'm not sure -- I think I'm probably just sticking my nose in where it
doesn't belong. :) The only useful piece of advice I have at this time
is "don't call renderer.Anything from outside Layer.Vector". 

Regards,
-- 
Christopher Schmidt
MetaCarta