[openstreetmap/openstreetmap-website] Minimap migration from Leaflet to MapLibre (PR #6683)
Copilot
notifications at github.com
Wed Jan 7 02:52:34 UTC 2026
@Copilot commented on this pull request.
## Pull request overview
This PR migrates the minimap functionality from Leaflet to MapLibre GL, introducing a new modular architecture for map styles while maintaining backward compatibility with the existing layer system. The changes extract inline style definitions into reusable functions and adapt the minimap rendering to use MapLibre instead of Leaflet.
- Extracts map style definitions into a centralized `maplibre.styles.js` file with factory functions
- Refactors language setting logic into a shared `OSM.MapLibre.setMapLanguage` utility
- Replaces Leaflet-based minimaps with MapLibre GL minimaps in the layer switcher
### Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
<details>
<summary>Show a summary per file</summary>
| File | Description |
| ---- | ----------- |
| app/assets/javascripts/maplibre.styles.js | Introduces centralized style definitions for all map layers (raster and vector), with factory functions that generate MapLibre style objects |
| app/assets/javascripts/maplibre.map.js | Updates require statements to include the new styles and i18n modules (previously had inline Mapnik style definition) |
| app/assets/javascripts/maplibre.i18n.js | Adds shared `setMapLanguage` function for setting map language based on user preferences, with support for asynchronous map loading |
| app/assets/javascripts/leaflet.maptiler.js | Refactors to use the new shared `setMapLanguage` utility instead of duplicated language-setting code |
| app/assets/javascripts/leaflet.map.js | Stores the layer type identifier in `layerOptions.leafletOsmId` for use in minimap style selection |
| app/assets/javascripts/leaflet.layers.js | Migrates minimap implementation from Leaflet to MapLibre GL, using the new style factory functions and MapLibre's map initialization API |
</details>
---
💡 <a href="/openstreetmap/openstreetmap-website/new/master/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
> OSM.MapLibre.Locale = {
"GeolocateControl.FindMyLocation": OSM.i18n.t("javascripts.map.locate.title"),
"GeolocateControl.LocationNotAvailable": OSM.i18n.t("javascripts.map.locate.not_available"),
"NavigationControl.ZoomIn": OSM.i18n.t("javascripts.map.zoom.in"),
"NavigationControl.ZoomOut": OSM.i18n.t("javascripts.map.zoom.out"),
"Popup.Close": OSM.i18n.t("javascripts.map.popup.close")
};
+
The `OSM.MapLibre.setMapLanguage` function lacks documentation. Consider adding a JSDoc comment to explain its purpose, parameters, and the logic for matching preferred languages with supported languages. This is especially important since the function has recursive behavior (calling itself after the map loads) and complex language matching logic.
```suggestion
/**
* Sets the map's label language based on the user's preferred languages and
* the languages supported by the current map style.
*
* If the map style has not finished loading, this function waits for the
* `"load"` event and then calls itself again once the style is available.
*
* The function retrieves the list of supported languages from the style
* (`map.style.getLanguage()`), then iterates over `OSM.preferred_languages`
* in order. Each preferred language is normalized by lowercasing and
* replacing `-` with `_` before being compared to the supported languages.
* The first exact match found is applied via `map.setLanguage`.
*
* @param {Object} map - The MapLibre map instance whose style and language
* will be updated. It is expected to expose `style.loaded()`,
* `style.getLanguage()`, `once(event, handler)` and `setLanguage(language)`.
*/
```
> @@ -0,0 +1,36 @@
+OSM.MapLibre.Styles.Mapnik = () => createRasterStyle(19, "https://tile.openstreetmap.org/{z}/{x}/{y}.png");
+OSM.MapLibre.Styles.CyclOSM = () => createRasterStyle(20, "https://{abc}.tile-cyclosm.openstreetmap.fr/cyclosm/{z}/{x}/{y}.png");
+OSM.MapLibre.Styles.CycleMap = (options) => createRasterStyle(21, `https://{abc}.tile.thunderforest.com/cycle/{z}/{x}/{y}.png?apikey=${options.apikey}`);
+OSM.MapLibre.Styles.TransportMap = (options) => createRasterStyle(21, `https://{abc}.tile.thunderforest.com/transport/{z}/{x}/{y}.png?apikey=${options.apikey}`);
+OSM.MapLibre.Styles.TransportDarkMap = (options) => createRasterStyle(21, `https://{abc}.tile.thunderforest.com/transport-dark/{z}/{x}/{y}.png?apikey=${options.apikey}`);
+OSM.MapLibre.Styles.OPNVKarte = () => createRasterStyle(18, "https://tileserver.memomaps.de/tilegen/{z}/{x}/{y}.png");
+OSM.MapLibre.Styles.HOT = () => createRasterStyle(20, "https://tile-{abc}.openstreetmap.fr/hot/{z}/{x}/{y}.png");
+OSM.MapLibre.Styles.TracestrackTopo = (options) => createRasterStyle(19, `https://tile.tracestrack.com/topo__/{z}/{x}/{y}.webp?key=${options.apikey}`);
+OSM.MapLibre.Styles.GPS = (options) => createRasterStyle(21, "https://gps.tile.openstreetmap.org/lines/{z}/{x}/{y}.png");
The `GPS` style function is defined but doesn't use the `options` parameter. Since this function accepts `options` as a parameter (unlike `Mapnik` which doesn't), either the parameter should be removed if not needed, or if it's intended for future use with API keys, this should be documented.
```suggestion
OSM.MapLibre.Styles.GPS = () => createRasterStyle(21, "https://gps.tile.openstreetmap.org/lines/{z}/{x}/{y}.png");
```
> @@ -42,8 +52,14 @@ L.OSM.layers = function (options) {
setView();
}
- function setView(options) {
- miniMap.setView(map.getCenter(), Math.max(map.getZoom() - 2, 0), options);
+ function setView(animate = true) {
+ const center = map.getCenter();
+ const zoom = Math.max(Math.floor(map.getZoom() - 3), -1);
The zoom level is calculated as `Math.floor(map.getZoom() - 3)`, which can result in negative values (minimum -1). While MapLibre might handle negative zoom gracefully, the previous Leaflet implementation used `Math.max(map.getZoom() - 2, 0)` to ensure a minimum zoom of 0. The change from `-2` to `-3` and allowing negative zoom should be intentional, but the inconsistency in allowing zoom -1 might cause unexpected behavior.
```suggestion
const zoom = Math.max(Math.floor(map.getZoom() - 3), 0);
```
> + const style = OSM.MapLibre.Styles[styleId](layer.options);
+ const miniMap = new maplibregl.Map({
+ container,
+ style,
+ interactive: false,
+ attributionControl: false,
+ fadeDuration: 0
+ });
The minimap should handle errors when the style fails to load (e.g., missing API keys for layers that require them). Since the style is retrieved dynamically via `OSM.MapLibre.Styles[styleId](layer.options)`, if the styleId doesn't exist or the function throws an error, the minimap creation will fail without proper error handling. Consider adding a try-catch block or error event listener.
```suggestion
// Ensure the style function exists before calling it
const styleFn = OSM.MapLibre && OSM.MapLibre.Styles && OSM.MapLibre.Styles[styleId];
if (typeof styleFn !== "function") {
console.error("Failed to initialize minimap: unknown or invalid styleId", {
styleId: styleId,
layerOptions: layer.options
});
return;
}
let miniMap;
try {
const style = styleFn(layer.options);
miniMap = new maplibregl.Map({
container,
style,
interactive: false,
attributionControl: false,
fadeDuration: 0
});
} catch (e) {
console.error("Failed to create minimap instance", e);
return;
}
// Log runtime style/map errors instead of failing silently
if (miniMap && typeof miniMap.on === "function") {
miniMap.on("error", function (event) {
console.error("Minimap encountered an error", event);
});
}
```
> + if (OSM.isDarkMap() && L.OSM[leafletOsmDarkId]) {
+ layerOptions.leafletOsmId = leafletOsmDarkId;
+ } else if (L.OSM[leafletOsmId]) {
+ layerOptions.leafletOsmId = leafletOsmId;
+ } else {
+ layerOptions.leafletOsmId = "TileLayer";
+ }
+
+ const layerConstructor = L.OSM[layerOptions.leafletOsmId];
The variable `leafletOsmId` is stored in `layerOptions` but this creates confusion since the value is now used for MapLibre styles, not Leaflet layers. Consider renaming this to `maplibreStyleId` or `styleId` to better reflect its new purpose in the MapLibre migration.
```suggestion
let leafletLayerId;
if (OSM.isDarkMap() && L.OSM[leafletOsmDarkId]) {
leafletLayerId = leafletOsmDarkId;
} else if (L.OSM[leafletOsmId]) {
leafletLayerId = leafletOsmId;
} else {
leafletLayerId = "TileLayer";
}
const layerConstructor = L.OSM[leafletLayerId];
```
> @@ -0,0 +1,36 @@
+OSM.MapLibre.Styles.Mapnik = () => createRasterStyle(19, "https://tile.openstreetmap.org/{z}/{x}/{y}.png");
+OSM.MapLibre.Styles.CyclOSM = () => createRasterStyle(20, "https://{abc}.tile-cyclosm.openstreetmap.fr/cyclosm/{z}/{x}/{y}.png");
+OSM.MapLibre.Styles.CycleMap = (options) => createRasterStyle(21, `https://{abc}.tile.thunderforest.com/cycle/{z}/{x}/{y}.png?apikey=${options.apikey}`);
+OSM.MapLibre.Styles.TransportMap = (options) => createRasterStyle(21, `https://{abc}.tile.thunderforest.com/transport/{z}/{x}/{y}.png?apikey=${options.apikey}`);
+OSM.MapLibre.Styles.TransportDarkMap = (options) => createRasterStyle(21, `https://{abc}.tile.thunderforest.com/transport-dark/{z}/{x}/{y}.png?apikey=${options.apikey}`);
+OSM.MapLibre.Styles.OPNVKarte = () => createRasterStyle(18, "https://tileserver.memomaps.de/tilegen/{z}/{x}/{y}.png");
+OSM.MapLibre.Styles.HOT = () => createRasterStyle(20, "https://tile-{abc}.openstreetmap.fr/hot/{z}/{x}/{y}.png");
+OSM.MapLibre.Styles.TracestrackTopo = (options) => createRasterStyle(19, `https://tile.tracestrack.com/topo__/{z}/{x}/{y}.webp?key=${options.apikey}`);
+OSM.MapLibre.Styles.GPS = (options) => createRasterStyle(21, "https://gps.tile.openstreetmap.org/lines/{z}/{x}/{y}.png");
+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}`;
+
The `createRasterStyle` function lacks documentation. Consider adding a JSDoc comment explaining the purpose of the function, the parameters (maxzoom, tileUrl), and the return value (MapLibre style object). This would improve code maintainability, especially since this function is central to the raster style creation.
```suggestion
/**
* Creates a MapLibre raster style configuration for a given raster tile source.
*
* If the tile URL template contains the `{abc}` placeholder, it will be expanded
* into separate URLs for the `a`, `b`, and `c` subdomains.
*
* @param {number} maxzoom - Maximum zoom level supported by the raster tile source.
* @param {string} tileUrl - URL template for raster tiles, optionally containing `{abc}`.
* @returns {object} A MapLibre Style Specification object configured for a single raster source.
*/
```
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6683#pullrequestreview-3633170084
You are receiving this because you are subscribed to this thread.
Message ID: <openstreetmap/openstreetmap-website/pull/6683/review/3633170084 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260106/46f2c9ca/attachment-0001.htm>
More information about the rails-dev
mailing list