@tomhughes commented on this pull request.
> +OSM.MapLibre.Styles.ShortbreadColorful = (options) =>
> `https://vector.openstreetmap.org/styles/shortbread/${options.styleName}`;
+OSM.MapLibre.Styles.ShortbreadEclipse = (options) =>
`https://vector.openstreetmap.org/styles/shortbread/${options.styleName}`;
+OSM.MapLibre.Styles.OpenMapTiles = (options) =>
`https://api.maptiler.com/maps/openstreetmap/style.json?key=${options.apikey}`;
Isn't this creating these as raster layers rather than vector layers?
> + if (layer.options.leafletOsmId === "OpenMapTiles") {
+ OSM.MapLibre.setMapLanguage(miniMap);
+ }
So this is setting the language (which currently only affects the OpenMapTiles
layer) but only if the main map is showing OpenMapTiles which seems wrong as
the behaviour of the minimap for OpenMapTiles will depend on what main map is
doing?
Plus it's changing global state anyway, and once at page load time (once the
main map is initialised) so it depends on the initial layer.
As the language is global state I suspect it should just be set once when
MapLibre is initialised and regardless of what layers are active?
> + function animateZoomToMatchMainMap() {
+ miniMap.zoomTo(getMiniMapZoom(), { duration: 300 });
+ }
+ function setZoomToMatchMainMap() {
+ miniMap.setZoom(getMiniMapZoom());
+ }
+ function getMiniMapZoom() {
Can add a blank line before each of these functions please, for consistency
with the other functions here if nothing else.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6683#pullrequestreview-3675409610
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6683/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
