-
Notifications
You must be signed in to change notification settings - Fork 11
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
Optimize bbox queries #129
Comments
Actually, I disagree. I find turf to be more performant than the ArcGIS JS API, and Turf is logically easier to follow. Maybe we can set up a simple test and see. |
I'm not saying Turf is slower than the JS API. I'm saying in this case, the way we're loading both is unnecessary and that this particular problem does not require the 128kb package that is Turf. In fact, this problem can be solved using simple math because we're only checking if the points are within a given bbox. |
I also don't think this is urgent, was just bookmarking for the next time we're working on the stationsMap.js or are looking to trim some dependencies. |
actually we are checking if the points are in the box of the map extent. We do this to avoid presenting the entire list of stations when the user is viewing a county. During usability testing particpants did not exepect the entire list, they expected the list to only contain the stations within the map extent. |
That still doesn't require a geospatial library. |
In stationsMap.js we load a JSON file of points (lat/long) into ArcGIS JS API, then (later) convert them to geoJSON and then use Turf.js to do point-in-polygon math. We can improve this by using the built-in functionality of the ArcGIS JS API (queryFeatures) or even just use simple math operations. This will save loading Turf.js(128kb package), and save copying the original points several times.
The text was updated successfully, but these errors were encountered: