Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

L.esri.FeatureLayer addfeature and removefeature events not fired at all #788

Closed
denydias opened this issue Jun 23, 2016 · 4 comments · Fixed by #790
Closed

L.esri.FeatureLayer addfeature and removefeature events not fired at all #788

denydias opened this issue Jun 23, 2016 · 4 comments · Fixed by #790

Comments

@denydias
Copy link

denydias commented Jun 23, 2016

  • Browser and version: Chromium 51.0.2704.84
  • Version of Leaflet (L.version): 0.7.7
  • Version of esri Leaflet (L.esri.VERSION): 1.0.3
  • I'm loading/bundling the library using: CDN

Steps to reproduce the error:

  1. Run this jsbin.
  2. In layer control, disable Neighborhoods.

What happens is that labels are kept on map even after the parent layer was disabled.

I was expecting that addfeature and removefeature could work to add and remove labels when enabling/disabling the parent layer. These events are never fired tough, as even the console.log messages aren't throw. Is there a bug with these events?

@denydias denydias changed the title L.esri.FeatureLayer addfeature removefeature events not fired at all L.esri.FeatureLayer addfeature and removefeature events not fired at all Jun 23, 2016
@jgravois
Copy link
Contributor

jgravois commented Jun 24, 2016

currently, 'add' and 'remove' events are only fired when individual features are appended or dropped from the current collection of a featureLayer that is currently present on the map. that said, it seems totally appropriate to me that the same corresponding events also fire when the entire layer is toggled on/off too.

a proposed fix can be found in #790. once its merged, it will appear in our next tagged release (2.0.1) afterward, i should be able to find some time to backport and create a 1.x release as well if it'd be helpful.

@denydias
Copy link
Author

denydias commented Jun 24, 2016

Hi, @jgravois!

That's awesome! Thank you for having the time to look into it!

It would be great to count on a 1.x backport. As Leaflet 1.x is still in RC status, many projects are still being developed in 0.7 release, as well as the natural forces keeping it around on production ones for a while, a backport would be fantastic atm.

Thank you very much for your reply and work. Say 'hi' for all the good fellow from Esri on my behalf.

@jgravois
Copy link
Contributor

jgravois commented Jul 4, 2016

the fix has been merged into master and backported to 1.0.4. thanks for reporting.

@denydias
Copy link
Author

denydias commented Jul 5, 2016

That's beautiful, @jgravois! Thank you very much for your quick action and backporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants