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

eachActiveFeature() to return only layers currently in view #936

Merged
merged 2 commits into from
Mar 13, 2017

Conversation

jgravois
Copy link
Contributor

@jgravois jgravois commented Mar 11, 2017

resolves #933.

not only did _currentSnapshot not already isolate features currently in view, i found that it just was just growing in size as duplicate keys were appended throughout the browser session.

@keithpower i'd be curious to see how the approach i'm proposing here compares to your own technique if you can spare the time to take a look.

@jgravois jgravois changed the title create method that returns only layers currently in view eachActiveFeature() to return only layers currently in view Mar 11, 2017
@keithpower
Copy link
Contributor

Thanks @jgravois , good catch on the duplicate keys. Your check for duplicates wasn't catching them in my testing. I used this instead and it appears to work. Note, the this_cache was also getting duplicates

if (-1 === this._currentSnapshot.indexOf(id)) {
    this._currentSnapshot.push(id);
}
if (-1 === this._cache[key].indexOf(id)) {
    this._cache[key].push(id);
}

The eachActiveFeature is very similar to how I implemented it on the application level. I will switch to this once I get time. I used contains instead of intersect because I was only dealing with points. I tihnk you'll want the check for the currentSnapshot in this function too. If a where is set at some point then this function will return features that may have been filtered out by the where. So it could look like this instead.

 eachActiveFeature: function (fn, context) {
     // figure out roughly which layers are in view
     if (this._map) {
       var activeBounds = this._map.getBounds();
       for (var i in this._layers) {
         if (activeBounds.intersects(this._layers[i].getBounds()) &&  -1 !== this._currentSnapshot.indexOf(this._layers[i].feature.id)) {
           fn.call(context, this._layers[i]);
         }
       }
     }
     return this;
   },

I have some other potential improvements around the whole setWhere functionality that I will create a pull request for in the next week or so once I get past some deadlines.

@jgravois
Copy link
Contributor Author

@keithpower much obliged!

@jgravois jgravois merged commit 4fb2078 into Esri:master Mar 13, 2017
@jgravois jgravois deleted the each-active-feature branch March 13, 2017 20:52
jgravois added a commit to jgravois/esri-leaflet that referenced this pull request Apr 23, 2022
* create method that returns only layers currently in view

* fix snapshot logic and test against it
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.

should FeatureLayer.eachFeature() be limited to returning the current snapshot?
2 participants