Conversation
1.0.0 RC3: release push into main
1.0.0 Release Sync to main
ISSUE-404 for 1.3.0: Advance search fixes that are also needed on 1.3.0
ISSUE-408: backwards 1.3.0
…mouse wheel zoom'. Also reduced the minimum map height to 62px, which guarantees that the +/- buttons will display.
…or when the 'display.default.display_options.fields.field_descriptive_metadata.settings.disable_mouse_zoom' configuration is not set. Add field.formatter.settings.strawberry_map_formatter.mapping.disable_mouse_zoom to format_strawberryfield.schema.yml.
… in the format_strawberryfield schema.
…-options Issue-414--leaflet-options --> 1.3.0
Sync main to 1.3.0
1.4.0: release sync
1.6.0 sync to main
DiegoPino
left a comment
There was a problem hiding this comment.
Small performance review/check
| map.addLayer(markers).fitBounds(markers.getBounds()); | ||
| } else { | ||
| map.addLayer(markers).setView(markers.getBounds().getCenter(), $initialzoom); | ||
| map.addLayer(markers); |
There was a problem hiding this comment.
Hi. Small performance idea here
geojsonLayer.getBounds() can be expensive if we have many points and polygons and the method/class does not cache calculated version. So every call re-calculates. You have two calls. So I would suggest you do the following instead
const bounds = geojsonLayer.getBounds();
if (bounds.isValid()) {then you don't need to re-call get bounds again or assign to a var bounds at all.
One feature (will an open issue, this one can go in anyways) I would love to preserve via a formatter setting/override is the idea of having still a way of setting a fixed zoom, or at least one that respects one issue we have found. Some tile sources don't have enough detail for certain zoom levels. So, if your polygon is very small, the automatic map.fitBounds(bounds) might respect the desired need of fitting it into the map, but might (for certain users that want the "context" basically render nothing as tile. I know its a fine grained need so will follow up with a different ISSUE that can wait until next release and won't affect by default this one at all
|
@markpbaggett I tagged you a few weeks ago. All looks good but the const v/s var change request would be great for performance needs. Let me know if you have any questions about it. Thanks for your contributions! |
|
Ok, did not get a response. Will merge as it is BC if not I will start adding code over it and then we will have to rebase. I will do on top later on the performance optimizations |
What Does This Do
This attempts to address ISSUE-574, but my logic is a little different than I originally suggested. Instead:
In other words:
geojsonLayerinstead of marker cluster groups. This means polygons, multipolygons, and lines would contribute to the viewport calculation as well.Also, I added an
isValid()guard so the map doesn't attempt to zoom on invalid data.Here is a single marker:

Here is a rushed together polygon:
