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

fix #1289 and fix #1298 #1301

Merged
merged 1 commit into from Dec 6, 2021
Merged

fix #1289 and fix #1298 #1301

merged 1 commit into from Dec 6, 2021

Conversation

patrickarlt
Copy link
Contributor

This fixes both #1289 and #1298. Specifically:

  • The internal _currentSnapshot which is a snapshot of all layers the feature layer has on the map was't properly being cleared when the layer went outside the zoom range. This is the root cause of both issues since it is used in eachActiveFeature and to determine which feature removefeature should fire for.
  • There is also a missing addfeature event when previously removed layers were added back to the map.

https://github.com/Esri/esri-leaflet/compare/fix-feature-layer-regressions?expand=1#diff-775f68f7ed2cadfe77df5f560b04512990231859cb3e0cfee6a8c79f305eb54aR523

https://github.com/Esri/esri-leaflet/compare/fix-feature-layer-regressions?expand=1#diff-1d3459775056d23611b051bf252a9d1f457744484695c2ac7ada4f0390f0451dR189-R195

Copy link
Contributor

@gavinr gavinr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

Copy link
Contributor

@jwasilgeo jwasilgeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, looks good, thanks @patrickarlt

@gavinr gavinr merged commit d6d48dd into master Dec 6, 2021
@gavinr gavinr deleted the fix-feature-layer-regressions branch December 6, 2021 20:15
jgravois pushed a commit to jgravois/esri-leaflet that referenced this pull request Apr 23, 2022
jgravois pushed a commit to jgravois/esri-leaflet that referenced this pull request Apr 23, 2022
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 this pull request may close these issues.

None yet

3 participants