resetStyle() suggestion

[rowanwins]

Hi there,

I'm just wondering the best way to achieve a resetStyle on a click event, I'm sure its just my amateur js skills at play. I'm trying to have it so that only one feature is styled at a time however I dont seem to be able to pass through the last clicked feature id before the new feature gets styled.

I've got the following

trees.on('click', function(e){
    trees.resetStyle(selectedFeature);
    selectedFeature = e.layer.feature.id;
    trees.setFeatureStyle(e.layer.feature.id, {
      color: '#9D78D2',
      weight: 3,
      opacity: 1
    });
});

It would perhaps be handy if you could do something like resetStyle(all) or run some sort of js comparison statement like resetStyle(!=354) then I think I could get around my problem more easily so Im not sure if that's a viable suggestion...

Cheers
Rowan

[patrickarlt]

What you want is pretty much a version of http://esri.github.io/esri-leaflet/examples/simplifying-complex-features.html.

I made a version with a click event here. http://codepen.io/patrickarlt/pen/DfExG?editors=100

What you are running into is called variable hoisting. Hoisting make the function you run when a feature gets clicked on look like this.

function(e){
  var selectedFeature;
  trees.resetStyle(selectedFeature);
  selectedFeature = e.layer.feature.id;
  trees.setFeatureStyle(e.layer.feature.id, {
    color: '#9D78D2',
    weight: 3,
    opacity: 1
  }
}

This means that when you trees.resetStyle(selectedFeature); it isn't actually e.layer.feature.id its undefined. So you aren't actually resetting the feature. If you add var selectedFeature; before calling layer.on('click', ... ) selectedFeature wont be hoisted anymore and it will persist on subsequent click event.

[patrickarlt]

In reference to your feature request about resetStyle('all') you can actually do. layer.setStyle(layer.options.style); which will loop over every feature and set the original style for that layer on them. This isn't recommended though because your layer could have 1000's of features.

[rowanwins]

Thanks @patrickarlt , I had used your hover example as I starting point but I must have somehow missed something sigh , the life of a know-nothing beginner! Thanks for the info about variable hoisting, good to know. PS I promise I am making progress and learning outside of asking you questions :)

[rowanwins]

Just as another note the other thing I realised in implementing the solution was that when you add the layer originally you need to define a style on it, presumably so that the click function knows what to return it to. So you can't just use the default styling, when you bring in your layer you need to do something like

  blah = L.esri.featureLayer('http://services.arcgis.com/P3ePLMYs2RVChkJx/arcgis/rest/services/USA_Congressional_Districts/FeatureServer/0',{
    style: function(feature) {
      return {
        color: '#000',
        weight: 2,
        opacity: 1
      }}
  }).addTo(map);

Hope that helps someone else out there!

[patrickarlt]

Actually I'm going to reopen this now. If you call resetStyle on a layer that does not have a style it should return it to the default style instead of doing nothing.

So this is actually a bug.

[jgravois]

agreed. i can take a stab at fixing this if it would be helpful.

[patrickarlt]

@jgravois assigned to you then. If it helps you can get the defaults by going new L.Path().options.

[jgravois]

i can't repro the problem @rowanwins is reporting. in this application, even when no style is defined for the featureLayer in code, clicking on individual features allows you to update style, and clicking on a second feature resets the first to its default.

unless @rowanwins can provide a repro case or more information, vote to close.

[rowanwins]

Hmmm I think there was/is some caching issue responsible. I managed to reproduce it only once and that was when I tried the code in a new browser but on refresh it seemed to sort itself out. So I think the bug can be closed...

[kneemer]

I have been able to get this to happen occasionally but not consistently. The application that @jgravois linked to exhibits the behavior for me if I click on the Alameda polygon. When I click away, Alameda does not reset.

[jgravois]

nice find @kneemer!

looks like there's a problem with multipart geometries specifically (i can repro the same error with the islands off the west coast of san francisco).

the problem is caused by the fact that no defaultOptions are accessible within resetStyle() here.

ill see if i can figure out why.

[jgravois]

i was able to fix this problem specific to MultiPolygons by updating setFeatureStyle() in FeatureLayer.js to define an explicit style to be used for MultiPolygons.

setFeatureStyle: function (id, style) {
  var layer = this._layers[id];

  if (typeof style === 'function') {
    style = style(layer.feature);
  }

  else if (!style && !layer.defaultOptions) {
    var dummyPath = new L.Path();
    style = {
      color: dummyPath.options.color,
      weight: dummyPath.options.weight,
      opacity: dummyPath.options.opacity
    }
  }

  if (layer.setStyle) {
    layer.setStyle(style);
  }
}

@patrickarlt i'm not sure if this is an acceptable approach, but i guess it could be (since MultiPolylines will also need a similar style and there doesn't seem to be such a thing in Leaflet as Multipoints). Let me know what you think. I am happy to submit a PR.

[patrickarlt]

@jgravois I think MultiPolygon and MuliPolyline actually inherit from FeatureGroup in Leaflet 0.7.3 which means they are REALLY weird so I'm not surprised you hit this.

Why don't you open up a PR just so we have a fix for it if we feel like it needs to be merged. I think this will occur in Leaflet 1.0 though.