From 873bbd7b0fd71aafabad87de05c77c731199e632 Mon Sep 17 00:00:00 2001 From: Dongdong Kong Date: Mon, 24 Aug 2026 21:13:50 +0800 Subject: [PATCH 01/11] feat: show raster basemap thumbnails in the Basemaps panel The catalog lists OSM, OpenTopoMap, and similar rasters as name-only rows. Stamp each raster entry with a z=2 XYZ tile so they can be told apart before clicking. Style basemaps have no single-tile endpoint and are left unchanged. Failed tile loads drop the image instead of showing a broken icon. --- apps/geolibre-desktop/src/index.css | 14 ++++ .../src/plugins/maplibre-basemap-control.ts | 75 +++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/apps/geolibre-desktop/src/index.css b/apps/geolibre-desktop/src/index.css index 4f6468559..bea92003c 100644 --- a/apps/geolibre-desktop/src/index.css +++ b/apps/geolibre-desktop/src/index.css @@ -2691,6 +2691,20 @@ body, onto the app design tokens when the app is in dark mode. Scoped to the package's own root/child classes, mirroring the STAC `.pc-control-*` block above. Never edit the package's stylesheet in node_modules. */ +.basemap-control-result:has(.geolibre-basemap-thumbnail) { + grid-template-columns: 56px 1fr auto; +} + +.geolibre-basemap-thumbnail { + grid-row: 1 / span 3; + width: 56px; + height: 42px; + object-fit: cover; + border-radius: 3px; + border: 1px solid hsl(var(--border)); + background: hsl(var(--muted)); +} + .dark .basemap-control { background: hsl(var(--popover)); color: hsl(var(--popover-foreground)); diff --git a/packages/plugins/src/plugins/maplibre-basemap-control.ts b/packages/plugins/src/plugins/maplibre-basemap-control.ts index 8260e8429..d0a14b8df 100644 --- a/packages/plugins/src/plugins/maplibre-basemap-control.ts +++ b/packages/plugins/src/plugins/maplibre-basemap-control.ts @@ -134,7 +134,76 @@ export function setBasemapControlLabels(next: Partial): vo labels = { ...labels, ...next }; } +// Raster rows get a z=2 XYZ tile so OSM vs OpenTopoMap is visible before click. +// Style basemaps have no single-tile endpoint; skip them. Failed loads remove +// the img rather than showing a broken icon. +const THUMBNAIL_ZOOM = 2; +const THUMBNAIL_X = 1; +const THUMBNAIL_Y = 1; +const PREVIEW_ATTR = "data-geolibre-basemap-preview"; + +let cachedBasemaps: BasemapDefinition[] = []; + +function buildRasterPreviewUrl(basemap: BasemapDefinition): string | null { + if (basemap.source.type !== "raster") return null; + const tiles = basemap.source.tiles; + if (!tiles || tiles.length === 0) return null; + const template = tiles[0]; + if (/\{(api-key|access_token|key)\}/.test(template)) return null; + return template + .replace(/\{z\}/g, String(THUMBNAIL_ZOOM)) + .replace(/\{x\}/g, String(THUMBNAIL_X)) + .replace(/\{y\}/g, String(THUMBNAIL_Y)) + .replace(/\{s\}/g, "a"); +} + +function refreshCachedBasemaps(control: BasemapControl | null): void { + if (!control) return; + const next = control.getBasemaps(); + if (Array.isArray(next)) cachedBasemaps = next; +} + +function installThumbnailEnhancer(): () => void { + if (typeof window === "undefined" || !document.body) return () => {}; + + const enhance = () => { + document + .querySelectorAll(`.basemap-control-result:not([${PREVIEW_ATTR}])`) + .forEach((row) => { + const id = row.getAttribute("data-basemap-id"); + const basemap = id ? cachedBasemaps.find((item) => item.id === id) : undefined; + const url = basemap ? buildRasterPreviewUrl(basemap) : null; + row.setAttribute(PREVIEW_ATTR, url ? "ready" : "skip"); + if (!url) return; + const img = document.createElement("img"); + img.className = "geolibre-basemap-thumbnail"; + img.alt = ""; + img.loading = "lazy"; + img.decoding = "async"; + img.src = url; + img.addEventListener( + "error", + () => { + img.remove(); + row.setAttribute(PREVIEW_ATTR, "failed"); + }, + { once: true }, + ); + img.addEventListener("load", () => row.setAttribute(PREVIEW_ATTR, "loaded"), { + once: true, + }); + row.prepend(img); + }); + }; + + const observer = new MutationObserver(enhance); + observer.observe(document.body, { childList: true, subtree: true }); + enhance(); + return () => observer.disconnect(); +} + let basemapControl: BasemapControl | null = null; +let thumbnailEnhancerCleanup: (() => void) | null = null; // GeoLibre layer ids of registered raster basemaps, keyed by basemap id. In // multiple mode several raster basemaps can be registered at once. const registeredRasterLayers = new Map(); @@ -165,8 +234,10 @@ export const maplibreBasemapControlPlugin: GeoLibrePlugin = { activate: (app: GeoLibreAppAPI) => { if (!basemapControl) { basemapControl = new BasemapControl(getBasemapControlOptions(app)); + refreshCachedBasemaps(basemapControl); basemapControl.on("basemapchange", (event) => { handleBasemapChange(app, event); + refreshCachedBasemaps(basemapControl); }); basemapControl.on("basemapremove", (event) => { handleBasemapRemove(app, event); @@ -190,6 +261,8 @@ export const maplibreBasemapControlPlugin: GeoLibrePlugin = { // style basemap or a removal can unregister them (the module state does not // survive a new session). relinkRestoredRasterBasemaps(); + thumbnailEnhancerCleanup?.(); + thumbnailEnhancerCleanup = installThumbnailEnhancer(); // Seed the fresh control instance with every basemap already on the map — // the active style basemap plus any stacked rasters we just relinked — so // the reopened panel highlights them as active and a re-click on a stacked @@ -219,6 +292,8 @@ export const maplibreBasemapControlPlugin: GeoLibrePlugin = { // reactivation relinks them from the store via relinkRestoredRasterBasemaps // (the same path a reopened project takes). See #1113 follow-up. registeredRasterLayers.clear(); + thumbnailEnhancerCleanup?.(); + thumbnailEnhancerCleanup = null; app.removeMapControl(basemapControl); basemapControl = null; // Drop any pending style-failure fallback so a later reactivation cannot From 51f7410bf63d14b6c4b59270c10eb9a79f6c2f78 Mon Sep 17 00:00:00 2001 From: Dongdong Kong Date: Mon, 24 Aug 2026 21:16:36 +0800 Subject: [PATCH 02/11] feat: preview style basemaps with a swatch then a map snapshot Raster rows still use a z=2 XYZ tile. Keyless style rows first show the style background color, then a small offscreen MapLibre snapshot of Europe once the row is on screen. Applying a style pauses the hidden map so it does not compete with the live view. Styles that still have an API-key placeholder are skipped. --- apps/geolibre-desktop/src/index.css | 13 +- .../plugins/src/plugins/basemap-thumbnails.ts | 221 ++++++++++++++++++ .../src/plugins/maplibre-basemap-control.ts | 82 +------ 3 files changed, 240 insertions(+), 76 deletions(-) create mode 100644 packages/plugins/src/plugins/basemap-thumbnails.ts diff --git a/apps/geolibre-desktop/src/index.css b/apps/geolibre-desktop/src/index.css index bea92003c..6c6408c0a 100644 --- a/apps/geolibre-desktop/src/index.css +++ b/apps/geolibre-desktop/src/index.css @@ -2691,10 +2691,21 @@ body, onto the app design tokens when the app is in dark mode. Scoped to the package's own root/child classes, mirroring the STAC `.pc-control-*` block above. Never edit the package's stylesheet in node_modules. */ -.basemap-control-result:has(.geolibre-basemap-thumbnail) { +.basemap-control-result:has(.geolibre-basemap-thumbnail), +.basemap-control-result[data-geolibre-basemap-preview="pending"] { grid-template-columns: 56px 1fr auto; } +.basemap-control-result[data-geolibre-basemap-preview="pending"]:not( + :has(.geolibre-basemap-thumbnail) + )::before { + content: ""; + width: 56px; + height: 42px; + border-radius: 3px; + background: hsl(var(--muted)); +} + .geolibre-basemap-thumbnail { grid-row: 1 / span 3; width: 56px; diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts new file mode 100644 index 000000000..c0fcf9dc4 --- /dev/null +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -0,0 +1,221 @@ +import type { BasemapControl, BasemapDefinition } from "maplibre-gl-basemap-control"; +import { Map as MapLibreMap } from "maplibre-gl"; + +const ATTR = "data-geolibre-basemap-preview"; +const swatchCache = new Map>(); +const snapCache = new Map(); + +function needsKey(value: string): boolean { + return /\{(api-key|access_token|key)\}/.test(value); +} + +function rasterPreviewUrl(basemap: BasemapDefinition): string | null { + if (basemap.source.type !== "raster" || !basemap.source.tiles?.[0]) return null; + const template = basemap.source.tiles[0]; + if (needsKey(template)) return null; + return template + .replace(/\{z\}/g, "2") + .replace(/\{x\}/g, "1") + .replace(/\{y\}/g, "1") + .replace(/\{s\}/g, "a"); +} + +function styleUrlOf(basemap: BasemapDefinition): string | null { + if (basemap.source.type !== "style" && basemap.source.type !== "vector-style") return null; + return needsKey(basemap.source.url) ? null : basemap.source.url; +} + +function styleSwatch(url: string): Promise { + const hit = swatchCache.get(url); + if (hit) return hit; + const next = fetch(url) + .then((response) => (response.ok ? response.json() : Promise.reject(response.status))) + .then((style: { layers?: Array<{ type?: string; paint?: Record }> }) => { + const canvas = document.createElement("canvas"); + canvas.width = 112; + canvas.height = 84; + const ctx = canvas.getContext("2d"); + if (!ctx) return null; + const color = style.layers?.find((layer) => layer.type === "background")?.paint?.[ + "background-color" + ]; + ctx.fillStyle = typeof color === "string" ? color : "#d0d5dd"; + ctx.fillRect(0, 0, canvas.width, canvas.height); + return canvas.toDataURL("image/png"); + }) + .catch(() => null); + swatchCache.set(url, next); + return next; +} + +function createStyleCamera(): { + snapshot(url: string): Promise; + pause(): void; + dispose(): void; +} { + let hidden: { map: MapLibreMap; el: HTMLDivElement } | null = null; + let tail = Promise.resolve(); + let paused = false; + + const teardown = () => { + hidden?.map.remove(); + hidden?.el.remove(); + hidden = null; + }; + + const ensure = () => { + if (hidden) return hidden; + const el = document.createElement("div"); + el.style.cssText = + "position:fixed;top:0;left:0;width:168px;height:126px;opacity:0;pointer-events:none;z-index:-1"; + document.body.append(el); + hidden = { + el, + map: new MapLibreMap({ + container: el, + style: { version: 8, sources: {}, layers: [] }, + center: [8, 47], + zoom: 2, + interactive: false, + attributionControl: false, + fadeDuration: 0, + pixelRatio: 1, + canvasContextAttributes: { preserveDrawingBuffer: true }, + }), + }; + return hidden; + }; + + return { + snapshot(url) { + const cached = snapCache.get(url); + if (cached) return Promise.resolve(cached); + const job = () => + new Promise((resolve) => { + if (paused) return resolve(null); + const { map } = ensure(); + const finish = (src: string | null) => { + map.off("style.load", onLoad); + resolve(src); + }; + const timer = window.setTimeout(() => finish(null), 6000); + const onLoad = () => { + window.setTimeout(() => { + window.clearTimeout(timer); + try { + const src = map.getCanvas().toDataURL("image/jpeg", 0.72); + if (src) snapCache.set(url, src); + finish(src); + } catch { + finish(null); + } + }, 400); + }; + map.once("style.load", onLoad); + map.setStyle(url, { diff: false }); + }); + const next = tail.then(job, job); + tail = next.then(() => undefined); + return next; + }, + pause() { + paused = true; + teardown(); + tail = Promise.resolve(); + paused = false; + }, + dispose() { + paused = true; + teardown(); + }, + }; +} + +function stamp(row: HTMLElement, src: string): void { + const existing = row.querySelector(".geolibre-basemap-thumbnail"); + if (existing) { + existing.src = src; + return; + } + const img = document.createElement("img"); + img.className = "geolibre-basemap-thumbnail"; + img.alt = ""; + img.src = src; + row.prepend(img); +} + +function paint(id: string, src: string, state: string): void { + document + .querySelectorAll(`.basemap-control-result[data-basemap-id="${CSS.escape(id)}"]`) + .forEach((row) => { + row.setAttribute(ATTR, state); + stamp(row, src); + }); +} + +export function installBasemapThumbnails(control: BasemapControl): { + dispose(): void; + pause(): void; +} { + const noop = { dispose() {}, pause() {} }; + if (typeof window === "undefined" || !document.body) return noop; + + const camera = createStyleCamera(); + const root = document.querySelector(".basemap-control-panel") ?? document.body; + const visible = + typeof IntersectionObserver === "function" + ? new IntersectionObserver( + (entries) => { + for (const entry of entries) { + if (!entry.isIntersecting) continue; + const row = entry.target as HTMLElement; + visible?.unobserve(row); + const url = row.dataset.previewUrl; + const id = row.getAttribute("data-basemap-id"); + if (!url || !id) continue; + void camera.snapshot(url).then((src) => { + if (src) paint(id, src, "loaded"); + }); + } + }, + { root: root.classList.contains("basemap-control-panel") ? root : null, rootMargin: "80px" }, + ) + : null; + + const enhance = () => { + const catalog = control.getBasemaps(); + root.querySelectorAll(`.basemap-control-result:not([${ATTR}])`).forEach((row) => { + const id = row.getAttribute("data-basemap-id"); + const basemap = id ? catalog.find((item) => item.id === id) : undefined; + const raster = basemap ? rasterPreviewUrl(basemap) : null; + const jsonUrl = !raster && basemap ? styleUrlOf(basemap) : null; + if (raster) { + row.setAttribute(ATTR, "ready"); + stamp(row, raster); + return; + } + if (jsonUrl && id) { + row.dataset.previewUrl = jsonUrl; + row.setAttribute(ATTR, "pending"); + void styleSwatch(jsonUrl).then((src) => { + if (src) paint(id, src, "ready"); + }); + visible?.observe(row); + return; + } + row.setAttribute(ATTR, "skip"); + }); + }; + + const observer = new MutationObserver(enhance); + observer.observe(root, { childList: true, subtree: true }); + enhance(); + return { + pause: () => camera.pause(), + dispose() { + observer.disconnect(); + visible?.disconnect(); + camera.dispose(); + }, + }; +} diff --git a/packages/plugins/src/plugins/maplibre-basemap-control.ts b/packages/plugins/src/plugins/maplibre-basemap-control.ts index d0a14b8df..9bb5e8843 100644 --- a/packages/plugins/src/plugins/maplibre-basemap-control.ts +++ b/packages/plugins/src/plugins/maplibre-basemap-control.ts @@ -13,6 +13,7 @@ import { type ManagedRasterBasemap, } from "maplibre-gl-basemap-control"; import type { GeoLibreAppAPI, GeoLibreMapControlPosition, GeoLibrePlugin } from "../types"; +import { installBasemapThumbnails } from "./basemap-thumbnails"; const basemapEnv = ( import.meta as ImportMeta & { @@ -134,76 +135,8 @@ export function setBasemapControlLabels(next: Partial): vo labels = { ...labels, ...next }; } -// Raster rows get a z=2 XYZ tile so OSM vs OpenTopoMap is visible before click. -// Style basemaps have no single-tile endpoint; skip them. Failed loads remove -// the img rather than showing a broken icon. -const THUMBNAIL_ZOOM = 2; -const THUMBNAIL_X = 1; -const THUMBNAIL_Y = 1; -const PREVIEW_ATTR = "data-geolibre-basemap-preview"; - -let cachedBasemaps: BasemapDefinition[] = []; - -function buildRasterPreviewUrl(basemap: BasemapDefinition): string | null { - if (basemap.source.type !== "raster") return null; - const tiles = basemap.source.tiles; - if (!tiles || tiles.length === 0) return null; - const template = tiles[0]; - if (/\{(api-key|access_token|key)\}/.test(template)) return null; - return template - .replace(/\{z\}/g, String(THUMBNAIL_ZOOM)) - .replace(/\{x\}/g, String(THUMBNAIL_X)) - .replace(/\{y\}/g, String(THUMBNAIL_Y)) - .replace(/\{s\}/g, "a"); -} - -function refreshCachedBasemaps(control: BasemapControl | null): void { - if (!control) return; - const next = control.getBasemaps(); - if (Array.isArray(next)) cachedBasemaps = next; -} - -function installThumbnailEnhancer(): () => void { - if (typeof window === "undefined" || !document.body) return () => {}; - - const enhance = () => { - document - .querySelectorAll(`.basemap-control-result:not([${PREVIEW_ATTR}])`) - .forEach((row) => { - const id = row.getAttribute("data-basemap-id"); - const basemap = id ? cachedBasemaps.find((item) => item.id === id) : undefined; - const url = basemap ? buildRasterPreviewUrl(basemap) : null; - row.setAttribute(PREVIEW_ATTR, url ? "ready" : "skip"); - if (!url) return; - const img = document.createElement("img"); - img.className = "geolibre-basemap-thumbnail"; - img.alt = ""; - img.loading = "lazy"; - img.decoding = "async"; - img.src = url; - img.addEventListener( - "error", - () => { - img.remove(); - row.setAttribute(PREVIEW_ATTR, "failed"); - }, - { once: true }, - ); - img.addEventListener("load", () => row.setAttribute(PREVIEW_ATTR, "loaded"), { - once: true, - }); - row.prepend(img); - }); - }; - - const observer = new MutationObserver(enhance); - observer.observe(document.body, { childList: true, subtree: true }); - enhance(); - return () => observer.disconnect(); -} - let basemapControl: BasemapControl | null = null; -let thumbnailEnhancerCleanup: (() => void) | null = null; +let thumbnails: ReturnType | null = null; // GeoLibre layer ids of registered raster basemaps, keyed by basemap id. In // multiple mode several raster basemaps can be registered at once. const registeredRasterLayers = new Map(); @@ -234,10 +167,8 @@ export const maplibreBasemapControlPlugin: GeoLibrePlugin = { activate: (app: GeoLibreAppAPI) => { if (!basemapControl) { basemapControl = new BasemapControl(getBasemapControlOptions(app)); - refreshCachedBasemaps(basemapControl); basemapControl.on("basemapchange", (event) => { handleBasemapChange(app, event); - refreshCachedBasemaps(basemapControl); }); basemapControl.on("basemapremove", (event) => { handleBasemapRemove(app, event); @@ -261,8 +192,8 @@ export const maplibreBasemapControlPlugin: GeoLibrePlugin = { // style basemap or a removal can unregister them (the module state does not // survive a new session). relinkRestoredRasterBasemaps(); - thumbnailEnhancerCleanup?.(); - thumbnailEnhancerCleanup = installThumbnailEnhancer(); + thumbnails?.dispose(); + thumbnails = installBasemapThumbnails(basemapControl); // Seed the fresh control instance with every basemap already on the map — // the active style basemap plus any stacked rasters we just relinked — so // the reopened panel highlights them as active and a re-click on a stacked @@ -292,8 +223,8 @@ export const maplibreBasemapControlPlugin: GeoLibrePlugin = { // reactivation relinks them from the store via relinkRestoredRasterBasemaps // (the same path a reopened project takes). See #1113 follow-up. registeredRasterLayers.clear(); - thumbnailEnhancerCleanup?.(); - thumbnailEnhancerCleanup = null; + thumbnails?.dispose(); + thumbnails = null; app.removeMapControl(basemapControl); basemapControl = null; // Drop any pending style-failure fallback so a later reactivation cannot @@ -404,6 +335,7 @@ function handleBasemapChange(app: GeoLibreAppAPI, event: BasemapControlEventPayl // before touching the layer manager, so an unrecognized future source type // does not evict the raster overlays without replacing the style. if (source.type !== "style" && source.type !== "vector-style") return; + thumbnails?.pause(); // Provider style basemaps (Amazon Location, MapTiler, Mapbox, ...) carry a // templated source.url with `{api-key}`/`{aws-region}` placeholders that the // control substitutes from the user's credentials. Apply the resolved URL the From 4e55251ef6d74da7655283bcd9c84da1d57e3641 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:16:54 +0000 Subject: [PATCH 03/11] style: auto-format (ruff + oxfmt) [pre-commit.ci] --- apps/geolibre-desktop/src/index.css | 4 ++-- packages/plugins/src/plugins/basemap-thumbnails.ts | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/geolibre-desktop/src/index.css b/apps/geolibre-desktop/src/index.css index 6c6408c0a..e129f825d 100644 --- a/apps/geolibre-desktop/src/index.css +++ b/apps/geolibre-desktop/src/index.css @@ -2697,8 +2697,8 @@ body, } .basemap-control-result[data-geolibre-basemap-preview="pending"]:not( - :has(.geolibre-basemap-thumbnail) - )::before { + :has(.geolibre-basemap-thumbnail) +)::before { content: ""; width: 56px; height: 42px; diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts index c0fcf9dc4..fa963cee2 100644 --- a/packages/plugins/src/plugins/basemap-thumbnails.ts +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -178,7 +178,10 @@ export function installBasemapThumbnails(control: BasemapControl): { }); } }, - { root: root.classList.contains("basemap-control-panel") ? root : null, rootMargin: "80px" }, + { + root: root.classList.contains("basemap-control-panel") ? root : null, + rootMargin: "80px", + }, ) : null; From a41c265ed1186db8d0ad1d5df6ed8d379c7a7764 Mon Sep 17 00:00:00 2001 From: Dongdong Kong Date: Mon, 24 Aug 2026 21:18:58 +0800 Subject: [PATCH 04/11] test: cover basemap preview URL substitution and key skipping --- .../plugins/src/plugins/basemap-thumbnails.ts | 4 +- tests/basemap-thumbnails.test.ts | 59 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 tests/basemap-thumbnails.test.ts diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts index fa963cee2..2339ed69e 100644 --- a/packages/plugins/src/plugins/basemap-thumbnails.ts +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -9,7 +9,7 @@ function needsKey(value: string): boolean { return /\{(api-key|access_token|key)\}/.test(value); } -function rasterPreviewUrl(basemap: BasemapDefinition): string | null { +export function rasterPreviewUrl(basemap: BasemapDefinition): string | null { if (basemap.source.type !== "raster" || !basemap.source.tiles?.[0]) return null; const template = basemap.source.tiles[0]; if (needsKey(template)) return null; @@ -20,7 +20,7 @@ function rasterPreviewUrl(basemap: BasemapDefinition): string | null { .replace(/\{s\}/g, "a"); } -function styleUrlOf(basemap: BasemapDefinition): string | null { +export function styleUrlOf(basemap: BasemapDefinition): string | null { if (basemap.source.type !== "style" && basemap.source.type !== "vector-style") return null; return needsKey(basemap.source.url) ? null : basemap.source.url; } diff --git a/tests/basemap-thumbnails.test.ts b/tests/basemap-thumbnails.test.ts new file mode 100644 index 000000000..b32b488ed --- /dev/null +++ b/tests/basemap-thumbnails.test.ts @@ -0,0 +1,59 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import type { BasemapDefinition } from "maplibre-gl-basemap-control"; +import { + rasterPreviewUrl, + styleUrlOf, +} from "../packages/plugins/src/plugins/basemap-thumbnails"; + +function raster(tiles: string[]): BasemapDefinition { + return { + id: "osm", + name: "OSM", + provider: "osm", + type: "raster", + source: { type: "raster", tiles }, + }; +} + +function style(url: string): BasemapDefinition { + return { + id: "positron", + name: "Positron", + provider: "openfreemap", + type: "style", + source: { type: "style", url }, + }; +} + +describe("basemap preview urls", () => { + it("fills z/x/y/s on a raster template", () => { + assert.equal( + rasterPreviewUrl(raster(["https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"])), + "https://a.tile.openstreetmap.org/2/1/1.png", + ); + }); + + it("skips rasters that still need a key", () => { + assert.equal( + rasterPreviewUrl(raster(["https://tiles.example/{z}/{x}/{y}.png?key={api-key}"])), + null, + ); + }); + + it("keeps a keyless style url and skips keyed ones", () => { + assert.equal( + styleUrlOf(style("https://tiles.openfreemap.org/styles/positron")), + "https://tiles.openfreemap.org/styles/positron", + ); + assert.equal(styleUrlOf(style("https://api.maptiler.com/maps/basic/style.json?key={key}")), null); + }); + + it("ignores the other source kind", () => { + assert.equal(rasterPreviewUrl(style("https://tiles.openfreemap.org/styles/positron")), null); + assert.equal( + styleUrlOf(raster(["https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"])), + null, + ); + }); +}); From 2348dffc01673ee05d31e2ac23221d0c4dae696a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:19:44 +0000 Subject: [PATCH 05/11] style: auto-format (ruff + oxfmt) [pre-commit.ci] --- tests/basemap-thumbnails.test.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/basemap-thumbnails.test.ts b/tests/basemap-thumbnails.test.ts index b32b488ed..d05cd6f77 100644 --- a/tests/basemap-thumbnails.test.ts +++ b/tests/basemap-thumbnails.test.ts @@ -1,10 +1,7 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; import type { BasemapDefinition } from "maplibre-gl-basemap-control"; -import { - rasterPreviewUrl, - styleUrlOf, -} from "../packages/plugins/src/plugins/basemap-thumbnails"; +import { rasterPreviewUrl, styleUrlOf } from "../packages/plugins/src/plugins/basemap-thumbnails"; function raster(tiles: string[]): BasemapDefinition { return { @@ -46,14 +43,14 @@ describe("basemap preview urls", () => { styleUrlOf(style("https://tiles.openfreemap.org/styles/positron")), "https://tiles.openfreemap.org/styles/positron", ); - assert.equal(styleUrlOf(style("https://api.maptiler.com/maps/basic/style.json?key={key}")), null); + assert.equal( + styleUrlOf(style("https://api.maptiler.com/maps/basic/style.json?key={key}")), + null, + ); }); it("ignores the other source kind", () => { assert.equal(rasterPreviewUrl(style("https://tiles.openfreemap.org/styles/positron")), null); - assert.equal( - styleUrlOf(raster(["https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"])), - null, - ); + assert.equal(styleUrlOf(raster(["https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"])), null); }); }); From b10353835d0a844a8158f209cbea2972b27e6a8d Mon Sep 17 00:00:00 2001 From: giswqs Date: Mon, 24 Aug 2026 13:24:24 -0400 Subject: [PATCH 06/11] Address review feedback - Reject any unresolved `{...}` placeholder instead of three hardcoded key names, so a provider token the control substitutes (`{aws-region}`) or a tile scheme this module does not fill in (`{quadkey}`, `{-y}`) is skipped rather than fetched literally. - Drop a failed style-swatch fetch from `swatchCache` so a transient network error is not cached for the life of the page. - Finish a snapshot immediately on a map `error` raised before `style.load`, so an unreachable style no longer holds the serialized queue for the full 6s timeout. Errors after the style loaded are tile failures and are ignored. - Make `pause()` actually pause: a promise gate the queued jobs await, reopened on resume or after a bounded delay, replacing a flag that was reset synchronously before any job could observe it. Queued rows now wait rather than resolving null and losing their thumbnail. - Remove a thumbnail whose image fails to load and mark the row skipped, so a 404 raster tile falls back to the name-only row instead of the browser's broken-image glyph. - Scope both observers to the control's own panel, never `document.body`, and re-resolve the panel when the control rebuilds it (a position change), which previously left the observers watching a detached node. - Rank the preview states so a slow flat-colour swatch cannot downgrade a row that already shows the real render. - Name the mirrored `maplibre-gl-basemap-control` DOM contract in exported constants with a re-check-on-bump note, and add a test that builds a real control and fails if the panel/row/id attribute drift. Documented in CLAUDE.md alongside the other unexported-internal mirrors. --- CLAUDE.md | 1 + .../plugins/src/plugins/basemap-thumbnails.ts | 219 ++++++++++++++---- tests/basemap-thumbnails.test.ts | 95 +++++++- 3 files changed, 265 insertions(+), 50 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 010438d39..89df5e4d2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,6 +154,7 @@ The browser build proxies the sidecar at `/sidecar` (same-origin, no CORS); conf - `MAX_VECTOR_BYTES` (`packages/plugins/src/plugins/remote-file-formats.ts`) mirrors `MAX_REMOTE_FILE_BYTES`, an **internal, unexported** constant in `maplibre-gl-vector` (2 GiB — DuckDB-WASM holds remote file sizes in 32 bits). It cannot be imported, so whenever `maplibre-gl-vector` is bumped (in `packages/plugins/package.json`) — including Dependabot PRs — re-check `src/lib/utils/remote.ts` in that package and update the mirror if it moved. If it drifts, the remote-browse panels (Source Cooperative, Hugging Face) silently block GeoParquet the engine could now open, or offer an Add that is certain to fail. Updating the constant is enough: the limit the user is shown is rendered from it, not written into the copy. `remote-file-formats.ts` is the **single** home for this and the other format/reader/size rules those panels share — a per-panel copy would miss this check, so add new browse panels against that module rather than duplicating it (`source-coop-api.ts` re-exports it under its own names for compatibility). - `MAP_PANEL_SELECTOR` (`apps/geolibre-desktop/src/components/layout/RecordVideoDialog.tsx`) mirrors the **rendered** control class names from `maplibre-gl-components` — `maplibre-gl-html-control`, `maplibre-gl-legend`, `maplibre-gl-colorbar` — so the Record Video "Include map panels" option can rasterize those on-map overlays into the recording. These are the display elements, deliberately **not** the `*-gui-control` authoring editors. The classes are internal and unexported, so whenever `maplibre-gl-components` is bumped (in `packages/plugins/package.json`) — including Dependabot PRs — re-check them against the rendered controls and update the selector if they moved. If a class drifts, the option silently stops burning that panel into the video (or the checkbox never appears) with no build error. - `GLOBE_CONTROL_TOGGLE_SELECTOR` (`packages/map/src/globe-control-toggle.ts`) mirrors the class names MapLibre's own `GlobeControl` puts on its toggle button — `maplibregl-ctrl-globe` and `maplibregl-ctrl-globe-enabled`, swapped on every projection change. `MapCanvas` persists a projection change from a **click** on that button rather than from the `projectiontransition` event, because style initialization and project reconciliation emit that event too and a stale one overwrites the projection of a project that has just loaded. The classes are internal and unexported, so whenever `maplibre-gl` is bumped (including Dependabot PRs) run the frontend suite — `tests/globe-control-toggle.test.ts` builds a real `GlobeControl` and fails if the mirror stops matching. Without that check a renamed class silently stops persisting the user's projection, with no build error. +- `BASEMAP_PANEL_SELECTOR` / `BASEMAP_ROW_SELECTOR` / `BASEMAP_ROW_ID_ATTR` (`packages/plugins/src/plugins/basemap-thumbnails.ts`) mirror the DOM `maplibre-gl-basemap-control` renders — `.basemap-control-panel`, `.basemap-control-result`, `data-basemap-id` — which the Basemaps panel's thumbnails hook into to find rows and join each one back to its catalog entry. That package exports only `BasemapControl`/`BasemapDefinition`, so a renamed class fails nothing at build time: the queries stop matching and thumbnails silently stop appearing. Whenever `maplibre-gl-basemap-control` is bumped in `packages/plugins/package.json` — including Dependabot PRs — run the frontend suite; `tests/basemap-thumbnails.test.ts` builds a real control and asserts its rendered panel against the mirror. The same file's `hasUnresolvedPlaceholder` deliberately matches the **complement** of the tile tokens it substitutes rather than mirroring that package's credential placeholders (`{api-key}`, `{aws-region}`), so a new provider's placeholder is skipped instead of being fetched literally — keep it that way rather than enumerating placeholder names. - **Per-layer blend modes** (`packages/map/src/layer-blend-modes.ts`) wrap three *unexported* `maplibre-gl` internals, because MapLibre renders every layer into one canvas and ships no per-layer blend API (upstream draft: maplibre/maplibre-gl-js#8073). The wrappers are `Painter.prototype.renderLayer` (brackets one layer's draws), `Painter.prototype.useProgram` (tells the layer-opacity composite draw from the draws feeding it), and `Context.prototype.setColorMode` (the single place every draw resolves GL blend state). Fill and line layers additionally get `fill-layer-opacity` / `line-layer-opacity` pinned just under 1 by `style-mapper`, which elects MapLibre 6's render-to-texture composite so a layer blends **as a whole** rather than once per overlapping polygon. `installLayerBlendModes` feature-detects every seam and disables the feature (hiding the Style-panel control) rather than breaking the map, so drift fails *quietly* — which is why `tests/layer-blend-modes.test.ts` asserts the seams and `e2e/blend-modes.spec.ts` asserts real pixels. Run both whenever `maplibre-gl` is bumped, including Dependabot PRs. **Do not add a blend mode without checking it in the browser**: MapLibre's blend state covers the alpha channel too, and it composites a blended layer as one viewport-filling quad, so any mode that does not reduce to "leave the destination alone" at zero source alpha repaints the whole map. That is what disqualified `darken` (a `MIN` equation erased the entire basemap to transparent black) and `subtract` (a reverse subtract left the canvas at `dstA - srcA`, showing the page through the layer); the shipped list is `BLEND_MODES` in `@geolibre/core`, and both the unit test's blend simulator and the e2e spec pin their exclusion. Only `fill` and `line` have a `*-layer-opacity` in the style spec, so only they blend as a **whole layer**; `circle` and `fill-extrusion` blend per symbol and visibly double-darken where symbols overlap on screen (measured under Multiply: `rgb(23, 77, 220)` in the overlap vs `rgb(76, 136, 222)` on a single symbol). That is upstream's limitation, documented in `docs/user-guide/layers.md`; the test "has a layer-level composite for fill and line only" fails if a bump adds one of the missing properties, at which point extend `COMPOSITE_LAYER_TYPES` and `style-mapper` together and drop the caveat. The Style-panel control (`blendModeControl` in `StylePanel.tsx`, rendered in each of its terminal branches) is gated on `!pluginOwnsPaint && !controlRendersLayer`: blending only reaches layers **GeoLibre itself paints**, so anything a control renders or paints (3D Tiles, Gaussian splats, LiDAR, the COG raster control, and Add Vector Layer, which sets `customLayerType` *and* `controlOwnsPaint`) is excluded -- layer-sync never applies `fillPaint`/`linePaint` to those, so the `*-layer-opacity` that elects the composite never lands and a Blend menu there would silently do nothing. Keep `docs/user-guide/layers.md` and `tests/layer-blend-modes.test.ts` ("the layer kinds the Blend control is offered for") in step with that gate; build the test's mocks the way the real controls build their metadata, or they pass on shapes that never occur. - `GeoLibreCogRenderEngine` (`packages/plugins/src/types.ts`) mirrors the `RenderEngine` union `maplibre-gl-raster` exports (`maplibre-gl-raster` | `cog-tiler-wasm` | `titiler`). It is hand-written rather than imported because `types.ts` is the public plugin-API surface and importing there would make that package's types a hard dependency of every external plugin. Unlike the mirrors above this one is checked by the **compiler**, not a test: `CogRenderEngineMirrorIsExact` in `packages/plugins/src/plugins/maplibre-raster.ts` asserts both directions of assignability against the real imported type, so a renamed or dropped engine identifier fails `npm run typecheck`. Nothing extra to do on a `maplibre-gl-raster` bump beyond letting the build run; without it a stale identifier would reach `control.setEngine()` as a string the control no longer recognizes, silently leaving the raster unrendered. - `propertySpecFor` (`packages/core/src/expressions.ts`) fabricates the **unexported** `StylePropertySpecification` shape that `@maplibre/maplibre-gl-style-spec`'s `createExpression` uses for expected-result-type enforcement (the Expression Builder's filter → boolean / color checks). The cast hides any contract change from the compiler, so whenever `@maplibre/maplibre-gl-style-spec` is bumped (including Dependabot PRs) run the frontend suite — the "enforces an expected result type" test in `tests/expressions.test.ts` fails if the shape stops being honored. diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts index 2339ed69e..a7c5da81e 100644 --- a/packages/plugins/src/plugins/basemap-thumbnails.ts +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -1,18 +1,58 @@ import type { BasemapControl, BasemapDefinition } from "maplibre-gl-basemap-control"; import { Map as MapLibreMap } from "maplibre-gl"; +/** + * The panel class, row class and row id attribute below mirror the DOM + * `maplibre-gl-basemap-control` renders (`_createPanel`/`_renderResults`). None + * of them are exported or typed by that package — only `BasemapControl` and + * `BasemapDefinition` are — so a rename on a version bump does not fail the + * build: the queries simply stop matching and thumbnails silently stop + * appearing. Re-check them whenever `maplibre-gl-basemap-control` is bumped in + * `packages/plugins/package.json`, including Dependabot PRs; + * `tests/basemap-thumbnails.test.ts` builds a real control and fails if they + * drift. Same convention as `GLOBE_CONTROL_TOGGLE_SELECTOR` and + * `MAP_PANEL_SELECTOR`. + */ +export const BASEMAP_PANEL_SELECTOR = ".basemap-control-panel"; +export const BASEMAP_ROW_SELECTOR = ".basemap-control-result"; +export const BASEMAP_ROW_ID_ATTR = "data-basemap-id"; + const ATTR = "data-geolibre-basemap-preview"; +/** + * How long a basemap switch keeps the preview map off the GPU. The caller has + * no completion signal for the style swap, so the gate reopens on its own + * rather than stranding every row that is waiting for a snapshot. + */ +const PAUSE_MS = 1500; +/** A row only ever moves forward — see `paint`. */ +const STATE_RANK: Record = { skip: 0, pending: 1, ready: 2, loaded: 3 }; const swatchCache = new Map>(); const snapCache = new Map(); -function needsKey(value: string): boolean { - return /\{(api-key|access_token|key)\}/.test(value); +/** The placeholders `rasterPreviewUrl` substitutes below. */ +const SUBSTITUTED_TOKEN = /^\{[zxys]\}$/; + +/** + * Reject a template that still carries a placeholder nothing has filled in. + * + * `maplibre-gl-basemap-control` substitutes the user's credentials into + * provider URLs before it loads a basemap (`{api-key}` and `{aws-region}` in + * the catalog it ships today — `API_KEY_PLACEHOLDER`/`AWS_REGION_PLACEHOLDER` + * in that package). Mirroring that list would silently miss a new provider's + * placeholder and fetch a URL with the literal token still in it, so match the + * complement instead: anything other than the tile coordinates this module + * itself resolves counts as unresolved. That also covers raster templates whose + * scheme is not filled in here (`{quadkey}`, `{-y}`, ...), which would render a + * broken tile rather than a preview. + */ +function hasUnresolvedPlaceholder(value: string): boolean { + return (value.match(/\{[^{}]*\}/g) ?? []).some((token) => !SUBSTITUTED_TOKEN.test(token)); } export function rasterPreviewUrl(basemap: BasemapDefinition): string | null { if (basemap.source.type !== "raster" || !basemap.source.tiles?.[0]) return null; const template = basemap.source.tiles[0]; - if (needsKey(template)) return null; + if (hasUnresolvedPlaceholder(template)) return null; return template .replace(/\{z\}/g, "2") .replace(/\{x\}/g, "1") @@ -22,7 +62,7 @@ export function rasterPreviewUrl(basemap: BasemapDefinition): string | null { export function styleUrlOf(basemap: BasemapDefinition): string | null { if (basemap.source.type !== "style" && basemap.source.type !== "vector-style") return null; - return needsKey(basemap.source.url) ? null : basemap.source.url; + return hasUnresolvedPlaceholder(basemap.source.url) ? null : basemap.source.url; } function styleSwatch(url: string): Promise { @@ -45,6 +85,11 @@ function styleSwatch(url: string): Promise { }) .catch(() => null); swatchCache.set(url, next); + // A transient fetch failure must not be cached for the lifetime of the page, + // or the row keeps its placeholder and never retries on a later panel open. + void next.then((src) => { + if (src === null && swatchCache.get(url) === next) swatchCache.delete(url); + }); return next; } @@ -55,7 +100,21 @@ function createStyleCamera(): { } { let hidden: { map: MapLibreMap; el: HTMLDivElement } | null = null; let tail = Promise.resolve(); - let paused = false; + let disposed = false; + // Closed while the main map applies a new style. Queued jobs wait on it + // rather than resolving null, so their rows still get a thumbnail once it + // reopens — a synchronous flag would be back to `false` before any job, which + // all run in a later microtask off `tail`, could ever observe it. + let gate: Promise = Promise.resolve(); + let openGate: (() => void) | null = null; + let resumeTimer = 0; + + const resume = () => { + window.clearTimeout(resumeTimer); + resumeTimer = 0; + openGate?.(); + openGate = null; + }; const teardown = () => { hidden?.map.remove(); @@ -90,18 +149,33 @@ function createStyleCamera(): { snapshot(url) { const cached = snapCache.get(url); if (cached) return Promise.resolve(cached); - const job = () => - new Promise((resolve) => { - if (paused) return resolve(null); + const job = async (): Promise => { + await gate; + if (disposed) return null; + return new Promise((resolve) => { const { map } = ensure(); + let settled = false; + let loaded = false; const finish = (src: string | null) => { + if (settled) return; + settled = true; + window.clearTimeout(timer); map.off("style.load", onLoad); + map.off("error", onError); resolve(src); }; const timer = window.setTimeout(() => finish(null), 6000); + // An unreachable or invalid style URL emits `error` and never fires + // `style.load`. Without this the job would hold the serialized queue + // for the full timeout, delaying every later preview by 6s. Errors + // raised *after* the style loaded are individual tile failures — the + // render is still worth capturing, so they are ignored. + const onError = () => { + if (!loaded) finish(null); + }; const onLoad = () => { + loaded = true; window.setTimeout(() => { - window.clearTimeout(timer); try { const src = map.getCanvas().toDataURL("image/jpeg", 0.72); if (src) snapCache.set(url, src); @@ -112,20 +186,33 @@ function createStyleCamera(): { }, 400); }; map.once("style.load", onLoad); + map.on("error", onError); map.setStyle(url, { diff: false }); }); + }; const next = tail.then(job, job); - tail = next.then(() => undefined); + tail = next.then( + () => undefined, + () => undefined, + ); return next; }, pause() { - paused = true; + if (disposed) return; teardown(); - tail = Promise.resolve(); - paused = false; + if (!openGate) { + gate = new Promise((resolveGate) => { + openGate = resolveGate; + }); + } + window.clearTimeout(resumeTimer); + resumeTimer = window.setTimeout(resume, PAUSE_MS); }, dispose() { - paused = true; + disposed = true; + // Release anything waiting on the gate so it can observe `disposed` and + // bail instead of resurrecting the hidden map after teardown. + resume(); teardown(); }, }; @@ -140,14 +227,29 @@ function stamp(row: HTMLElement, src: string): void { const img = document.createElement("img"); img.className = "geolibre-basemap-thumbnail"; img.alt = ""; + // The guessed z=2 raster tile can 404 — a source whose minzoom is deeper, a + // dead endpoint, hotlink protection. Drop the image so the row falls back to + // the name-only layout instead of showing the browser's broken-image glyph, + // and mark the row skipped so the CSS placeholder does not linger either. + img.addEventListener("error", () => { + img.remove(); + row.setAttribute(ATTR, "skip"); + }); img.src = src; row.prepend(img); } function paint(id: string, src: string, state: string): void { document - .querySelectorAll(`.basemap-control-result[data-basemap-id="${CSS.escape(id)}"]`) + .querySelectorAll( + `${BASEMAP_ROW_SELECTOR}[${BASEMAP_ROW_ID_ATTR}="${CSS.escape(id)}"]`, + ) .forEach((row) => { + // The flat-colour swatch and the real render race independently, and + // `snapCache` outlives dispose(), so a reopened panel can paint "loaded" + // before a slow swatch fetch settles. Never let a row move backwards. + const current = row.getAttribute(ATTR); + if (current && STATE_RANK[current] >= STATE_RANK[state]) return; row.setAttribute(ATTR, state); stamp(row, src); }); @@ -161,34 +263,28 @@ export function installBasemapThumbnails(control: BasemapControl): { if (typeof window === "undefined" || !document.body) return noop; const camera = createStyleCamera(); - const root = document.querySelector(".basemap-control-panel") ?? document.body; - const visible = - typeof IntersectionObserver === "function" - ? new IntersectionObserver( - (entries) => { - for (const entry of entries) { - if (!entry.isIntersecting) continue; - const row = entry.target as HTMLElement; - visible?.unobserve(row); - const url = row.dataset.previewUrl; - const id = row.getAttribute("data-basemap-id"); - if (!url || !id) continue; - void camera.snapshot(url).then((src) => { - if (src) paint(id, src, "loaded"); - }); - } - }, - { - root: root.classList.contains("basemap-control-panel") ? root : null, - rootMargin: "80px", - }, - ) - : null; - - const enhance = () => { + let panel: HTMLElement | null = null; + let visible: IntersectionObserver | null = null; + + function onVisible(entries: IntersectionObserverEntry[]): void { + for (const entry of entries) { + if (!entry.isIntersecting) continue; + const row = entry.target as HTMLElement; + visible?.unobserve(row); + const url = row.dataset.previewUrl; + const id = row.getAttribute(BASEMAP_ROW_ID_ATTR); + if (!url || !id) continue; + void camera.snapshot(url).then((src) => { + if (src) paint(id, src, "loaded"); + }); + } + } + + function enhance(): void { + if (!panel) return; const catalog = control.getBasemaps(); - root.querySelectorAll(`.basemap-control-result:not([${ATTR}])`).forEach((row) => { - const id = row.getAttribute("data-basemap-id"); + panel.querySelectorAll(`${BASEMAP_ROW_SELECTOR}:not([${ATTR}])`).forEach((row) => { + const id = row.getAttribute(BASEMAP_ROW_ID_ATTR); const basemap = id ? catalog.find((item) => item.id === id) : undefined; const raster = basemap ? rasterPreviewUrl(basemap) : null; const jsonUrl = !raster && basemap ? styleUrlOf(basemap) : null; @@ -208,15 +304,44 @@ export function installBasemapThumbnails(control: BasemapControl): { } row.setAttribute(ATTR, "skip"); }); - }; + } + + // Every scan is scoped to the control's own panel: rooting them at + // `document.body` would rebuild the catalog and re-scan the whole document on + // every unrelated UI mutation, and would make the IntersectionObserver treat + // rows scrolled out of the panel as visible. + const scoped = new MutationObserver(() => enhance()); + + function attach(): void { + const found = document.querySelector(BASEMAP_PANEL_SELECTOR); + if (found === panel) return; + panel = found; + scoped.disconnect(); + visible?.disconnect(); + visible = null; + if (!panel) return; + scoped.observe(panel, { childList: true, subtree: true }); + if (typeof IntersectionObserver === "function") { + visible = new IntersectionObserver(onVisible, { root: panel, rootMargin: "80px" }); + } + enhance(); + } + + // The control builds a fresh panel element in every `onAdd`, so the one this + // watched can be replaced (a position change) or not exist yet. This callback + // costs a single `isConnected` check per mutation batch while the panel is + // healthy, and only then falls back to a lookup. + const bootstrap = new MutationObserver(() => { + if (!panel?.isConnected) attach(); + }); + bootstrap.observe(document.body, { childList: true, subtree: true }); + attach(); - const observer = new MutationObserver(enhance); - observer.observe(root, { childList: true, subtree: true }); - enhance(); return { pause: () => camera.pause(), dispose() { - observer.disconnect(); + bootstrap.disconnect(); + scoped.disconnect(); visible?.disconnect(); camera.dispose(); }, diff --git a/tests/basemap-thumbnails.test.ts b/tests/basemap-thumbnails.test.ts index d05cd6f77..856701593 100644 --- a/tests/basemap-thumbnails.test.ts +++ b/tests/basemap-thumbnails.test.ts @@ -1,7 +1,14 @@ import assert from "node:assert/strict"; -import { describe, it } from "node:test"; -import type { BasemapDefinition } from "maplibre-gl-basemap-control"; -import { rasterPreviewUrl, styleUrlOf } from "../packages/plugins/src/plugins/basemap-thumbnails"; +import { afterEach, beforeEach, describe, it } from "node:test"; +import { parseHTML } from "linkedom"; +import { BasemapControl, type BasemapDefinition } from "maplibre-gl-basemap-control"; +import { + BASEMAP_PANEL_SELECTOR, + BASEMAP_ROW_ID_ATTR, + BASEMAP_ROW_SELECTOR, + rasterPreviewUrl, + styleUrlOf, +} from "../packages/plugins/src/plugins/basemap-thumbnails"; function raster(tiles: string[]): BasemapDefinition { return { @@ -49,8 +56,90 @@ describe("basemap preview urls", () => { ); }); + it("skips any placeholder it does not substitute itself", () => { + // The control substitutes {api-key}/{aws-region} from the user's + // credentials; a preview must not request a URL that still carries one, nor + // a tile scheme rasterPreviewUrl leaves unresolved. + assert.equal( + styleUrlOf( + style("https://maps.geo.{aws-region}.amazonaws.com/v2/styles/Standard/descriptor"), + ), + null, + ); + assert.equal(rasterPreviewUrl(raster(["https://tiles.example/{quadkey}.png"])), null); + assert.equal(rasterPreviewUrl(raster(["https://tiles.example/{z}/{x}/{-y}.png"])), null); + }); + it("ignores the other source kind", () => { assert.equal(rasterPreviewUrl(style("https://tiles.openfreemap.org/styles/positron")), null); assert.equal(styleUrlOf(raster(["https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"])), null); }); }); + +/** + * `BASEMAP_PANEL_SELECTOR` / `BASEMAP_ROW_SELECTOR` / `BASEMAP_ROW_ID_ATTR` + * mirror DOM that `maplibre-gl-basemap-control` renders but does not export, so + * nothing but this file notices if a bump renames one — the queries would just + * stop matching and thumbnails would silently stop appearing. Rather than + * restate the strings, this builds a real `BasemapControl` and asks it for its + * rendered panel. + */ +describe("the maplibre-gl-basemap-control DOM mirror", () => { + let restoreGlobals: () => void; + + beforeEach(() => { + const { document, window } = parseHTML(""); + const previous = { + document: globalThis.document, + window: globalThis.window, + requestAnimationFrame: globalThis.requestAnimationFrame, + }; + // The control assigns `select.value`, which linkedom exposes as a getter + // only. Give it a setter so its filter row renders. + const selectValue = Object.getOwnPropertyDescriptor( + window.HTMLSelectElement.prototype, + "value", + ); + Object.defineProperty(window.HTMLSelectElement.prototype, "value", { + configurable: true, + get: selectValue?.get, + set(next: string) { + this.setAttribute("value", next); + }, + }); + Object.assign(globalThis, { + document, + window, + // linkedom ships no rAF; the control only uses it to position the panel. + requestAnimationFrame: () => 0, + }); + restoreGlobals = () => Object.assign(globalThis, previous); + }); + + afterEach(() => restoreGlobals()); + + it("matches the panel, the rows and the row id attribute", () => { + const container = document.createElement("div"); + document.body.append(container); + const control = new BasemapControl({ collapsed: false }); + control.onAdd({ + getContainer: () => container, + on: () => {}, + off: () => {}, + } as never); + + const panel = container.querySelector(BASEMAP_PANEL_SELECTOR); + assert.ok(panel, `no ${BASEMAP_PANEL_SELECTOR} — the control renamed its panel`); + const rows = [...panel.querySelectorAll(BASEMAP_ROW_SELECTOR)]; + assert.ok(rows.length > 0, `no ${BASEMAP_ROW_SELECTOR} rows — the control renamed its rows`); + + // `enhance` joins a row back to its catalog entry through this attribute, + // so the ids the rows carry must be ids `getBasemaps()` reports. + const catalog = new Set(control.getBasemaps().map((basemap) => basemap.id)); + const ids = rows.map((row) => row.getAttribute(BASEMAP_ROW_ID_ATTR)); + assert.ok( + ids.every((id) => id !== null && catalog.has(id)), + `rows no longer join the catalog through ${BASEMAP_ROW_ID_ATTR}`, + ); + }); +}); From fd392085b7bf752029ac604007b961ec4e8e92fb Mon Sep 17 00:00:00 2001 From: giswqs Date: Mon, 24 Aug 2026 13:35:01 -0400 Subject: [PATCH 07/11] Address review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Settle the in-flight snapshot when the hidden map is torn down. Its listeners were bound to the removed map, so it could only resolve through the 6s timeout, and since jobs are serialized that stalled every later preview well past PAUSE_MS. Regression from the previous commit, which stopped resetting the queue in pause(). - Flip the preview row index for a `tms` raster source (the catalog ships two), which otherwise previewed the vertically mirrored tile. - Fall back to the name-only row when a style preview produces nothing, so an unreachable style host degrades the way a failed raster tile already did instead of keeping its placeholder for good. - Reject every placeholder in a style URL, not just the ones a raster template substitutes — a style URL is fetched verbatim. - Clear the delayed capture timer in `finish` and ignore a preview that resolves after `dispose()`, so a deactivated plugin runs no callback and repaints no row. - Watch the map container's child list rather than the whole document body once the panel has been seen; the control appends its panel there, so a rebuilt panel is still caught without observing every application mutation. - Cover the tms and style-placeholder rules, and assert the panel stays a direct child of the map container, which the narrowed watch now depends on. --- .../plugins/src/plugins/basemap-thumbnails.ts | 109 ++++++++++++++---- tests/basemap-thumbnails.test.ts | 22 ++++ 2 files changed, 108 insertions(+), 23 deletions(-) diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts index a7c5da81e..ad3a3031e 100644 --- a/packages/plugins/src/plugins/basemap-thumbnails.ts +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -45,23 +45,32 @@ const SUBSTITUTED_TOKEN = /^\{[zxys]\}$/; * scheme is not filled in here (`{quadkey}`, `{-y}`, ...), which would render a * broken tile rather than a preview. */ -function hasUnresolvedPlaceholder(value: string): boolean { - return (value.match(/\{[^{}]*\}/g) ?? []).some((token) => !SUBSTITUTED_TOKEN.test(token)); +function hasUnresolvedPlaceholder(value: string, substituted?: RegExp): boolean { + return (value.match(/\{[^{}]*\}/g) ?? []).some((token) => !substituted?.test(token)); } +/** The zoom every raster preview tile is sampled at. */ +const PREVIEW_Z = 2; + export function rasterPreviewUrl(basemap: BasemapDefinition): string | null { if (basemap.source.type !== "raster" || !basemap.source.tiles?.[0]) return null; const template = basemap.source.tiles[0]; - if (hasUnresolvedPlaceholder(template)) return null; + if (hasUnresolvedPlaceholder(template, SUBSTITUTED_TOKEN)) return null; + // A `tms` source numbers rows from the bottom (MapLibre flips `{y}` for it), + // so an xyz row index would fetch the vertically mirrored tile — a different + // part of the world than the basemap actually renders there. + const y = basemap.source.scheme === "tms" ? 2 ** PREVIEW_Z - 1 - 1 : 1; return template - .replace(/\{z\}/g, "2") + .replace(/\{z\}/g, String(PREVIEW_Z)) .replace(/\{x\}/g, "1") - .replace(/\{y\}/g, "1") + .replace(/\{y\}/g, String(y)) .replace(/\{s\}/g, "a"); } export function styleUrlOf(basemap: BasemapDefinition): string | null { if (basemap.source.type !== "style" && basemap.source.type !== "vector-style") return null; + // Nothing substitutes a token in a style URL — it is fetched verbatim — so + // even the tile tokens rasterPreviewUrl fills in make it unusable here. return hasUnresolvedPlaceholder(basemap.source.url) ? null : basemap.source.url; } @@ -108,6 +117,8 @@ function createStyleCamera(): { let gate: Promise = Promise.resolve(); let openGate: (() => void) | null = null; let resumeTimer = 0; + /** Settles the job that is already waiting on the hidden map, if any. */ + let cancelInFlight: (() => void) | null = null; const resume = () => { window.clearTimeout(resumeTimer); @@ -117,6 +128,11 @@ function createStyleCamera(): { }; const teardown = () => { + // A job already past the gate has its listeners on the map about to be + // removed, so `style.load` can never fire for it: without this it could + // only settle through the 6s timeout, and since jobs are serialized that + // would stall every later preview well past PAUSE_MS. + cancelInFlight?.(); hidden?.map.remove(); hidden?.el.remove(); hidden = null; @@ -156,15 +172,19 @@ function createStyleCamera(): { const { map } = ensure(); let settled = false; let loaded = false; + let captureTimer = 0; const finish = (src: string | null) => { if (settled) return; settled = true; + if (cancelInFlight === cancel) cancelInFlight = null; window.clearTimeout(timer); + window.clearTimeout(captureTimer); map.off("style.load", onLoad); map.off("error", onError); resolve(src); }; - const timer = window.setTimeout(() => finish(null), 6000); + const cancel = () => finish(null); + const timer = window.setTimeout(cancel, 6000); // An unreachable or invalid style URL emits `error` and never fires // `style.load`. Without this the job would hold the serialized queue // for the full timeout, delaying every later preview by 6s. Errors @@ -175,7 +195,7 @@ function createStyleCamera(): { }; const onLoad = () => { loaded = true; - window.setTimeout(() => { + captureTimer = window.setTimeout(() => { try { const src = map.getCanvas().toDataURL("image/jpeg", 0.72); if (src) snapCache.set(url, src); @@ -187,6 +207,7 @@ function createStyleCamera(): { }; map.once("style.load", onLoad); map.on("error", onError); + cancelInFlight = cancel; map.setStyle(url, { diff: false }); }); }; @@ -239,20 +260,35 @@ function stamp(row: HTMLElement, src: string): void { row.prepend(img); } +function rowSelector(id: string): string { + return `${BASEMAP_ROW_SELECTOR}[${BASEMAP_ROW_ID_ATTR}="${CSS.escape(id)}"]`; +} + function paint(id: string, src: string, state: string): void { - document - .querySelectorAll( - `${BASEMAP_ROW_SELECTOR}[${BASEMAP_ROW_ID_ATTR}="${CSS.escape(id)}"]`, - ) - .forEach((row) => { - // The flat-colour swatch and the real render race independently, and - // `snapCache` outlives dispose(), so a reopened panel can paint "loaded" - // before a slow swatch fetch settles. Never let a row move backwards. - const current = row.getAttribute(ATTR); - if (current && STATE_RANK[current] >= STATE_RANK[state]) return; - row.setAttribute(ATTR, state); - stamp(row, src); - }); + document.querySelectorAll(rowSelector(id)).forEach((row) => { + // The flat-colour swatch and the real render race independently, and + // `snapCache` outlives dispose(), so a reopened panel can paint "loaded" + // before a slow swatch fetch settles. Never let a row move backwards. + const current = row.getAttribute(ATTR); + if (current && STATE_RANK[current] >= STATE_RANK[state]) return; + row.setAttribute(ATTR, state); + stamp(row, src); + }); +} + +/** + * Drop a row back to the name-only layout when no preview could be produced — + * an unreachable style host, CORS, or the snapshot timeout. Without this the + * row keeps its "pending" placeholder for good, where a failed *raster* tile + * already degrades this way through `stamp`'s error handler. Rows that did get + * an image are left alone, and a later snapshot can still upgrade a skipped row + * because "loaded" outranks "skip". + */ +function markSkipped(id: string): void { + document.querySelectorAll(rowSelector(id)).forEach((row) => { + if (row.querySelector(".geolibre-basemap-thumbnail")) return; + row.setAttribute(ATTR, "skip"); + }); } export function installBasemapThumbnails(control: BasemapControl): { @@ -263,6 +299,9 @@ export function installBasemapThumbnails(control: BasemapControl): { if (typeof window === "undefined" || !document.body) return noop; const camera = createStyleCamera(); + // Cleared by dispose() so an in-flight preview cannot repaint rows belonging + // to a control the plugin has already torn down. + let active = true; let panel: HTMLElement | null = null; let visible: IntersectionObserver | null = null; @@ -275,7 +314,9 @@ export function installBasemapThumbnails(control: BasemapControl): { const id = row.getAttribute(BASEMAP_ROW_ID_ATTR); if (!url || !id) continue; void camera.snapshot(url).then((src) => { + if (!active) return; if (src) paint(id, src, "loaded"); + else markSkipped(id); }); } } @@ -297,7 +338,9 @@ export function installBasemapThumbnails(control: BasemapControl): { row.dataset.previewUrl = jsonUrl; row.setAttribute(ATTR, "pending"); void styleSwatch(jsonUrl).then((src) => { + if (!active) return; if (src) paint(id, src, "ready"); + else markSkipped(id); }); visible?.observe(row); return; @@ -319,6 +362,7 @@ export function installBasemapThumbnails(control: BasemapControl): { scoped.disconnect(); visible?.disconnect(); visible = null; + watchForPanel(); if (!panel) return; scoped.observe(panel, { childList: true, subtree: true }); if (typeof IntersectionObserver === "function") { @@ -329,17 +373,36 @@ export function installBasemapThumbnails(control: BasemapControl): { // The control builds a fresh panel element in every `onAdd`, so the one this // watched can be replaced (a position change) or not exist yet. This callback - // costs a single `isConnected` check per mutation batch while the panel is - // healthy, and only then falls back to a lookup. + // costs a single `isConnected` check per mutation batch, and only then falls + // back to a lookup. const bootstrap = new MutationObserver(() => { if (!panel?.isConnected) attach(); }); - bootstrap.observe(document.body, { childList: true, subtree: true }); + + /** + * `BasemapControl` does not expose its panel, but it appends it as a direct + * child of the map container — so once the panel has been seen, watch that + * one element's child list rather than the whole application's DOM. A + * body-wide subtree observer would fire on every unrelated UI mutation for as + * long as the plugin is active, which is the whole session. The body is only + * a bootstrap for the window before the panel first appears. + */ + function watchForPanel(): void { + bootstrap.disconnect(); + const host = panel?.parentElement; + if (host) bootstrap.observe(host, { childList: true }); + else bootstrap.observe(document.body, { childList: true, subtree: true }); + } + attach(); + // `attach` installs the watch when it finds a panel; cover the case where it + // did not. + if (!panel) watchForPanel(); return { pause: () => camera.pause(), dispose() { + active = false; bootstrap.disconnect(); scoped.disconnect(); visible?.disconnect(); diff --git a/tests/basemap-thumbnails.test.ts b/tests/basemap-thumbnails.test.ts index 856701593..4f3127e74 100644 --- a/tests/basemap-thumbnails.test.ts +++ b/tests/basemap-thumbnails.test.ts @@ -20,6 +20,10 @@ function raster(tiles: string[]): BasemapDefinition { }; } +function tmsRaster(tiles: string[]): BasemapDefinition { + return { ...raster(tiles), source: { type: "raster", tiles, scheme: "tms" } }; +} + function style(url: string): BasemapDefinition { return { id: "positron", @@ -70,6 +74,21 @@ describe("basemap preview urls", () => { assert.equal(rasterPreviewUrl(raster(["https://tiles.example/{z}/{x}/{-y}.png"])), null); }); + it("flips the row index for a tms source", () => { + // Tencent's basemaps number rows from the bottom; an xyz row index would + // preview the vertically mirrored tile. + assert.equal( + rasterPreviewUrl(tmsRaster(["https://tiles.example/tile?z={z}&x={x}&y={y}"])), + "https://tiles.example/tile?z=2&x=1&y=2", + ); + }); + + it("skips a style url carrying any placeholder at all", () => { + // A style URL is fetched verbatim, so even a tile token nothing substitutes + // for it makes the URL unusable. + assert.equal(styleUrlOf(style("https://{s}.example.com/style.json")), null); + }); + it("ignores the other source kind", () => { assert.equal(rasterPreviewUrl(style("https://tiles.openfreemap.org/styles/positron")), null); assert.equal(styleUrlOf(raster(["https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"])), null); @@ -130,6 +149,9 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { const panel = container.querySelector(BASEMAP_PANEL_SELECTOR); assert.ok(panel, `no ${BASEMAP_PANEL_SELECTOR} — the control renamed its panel`); + // `watchForPanel` observes the panel's parent to notice a rebuilt panel, so + // the panel has to stay a direct child of the map container. + assert.equal(panel.parentElement, container, "the panel is no longer a map-container child"); const rows = [...panel.querySelectorAll(BASEMAP_ROW_SELECTOR)]; assert.ok(rows.length > 0, `no ${BASEMAP_ROW_SELECTOR} rows — the control renamed its rows`); From 5fb7d7bbf7a7284ee99ef33f1b7225fea48e1246 Mon Sep 17 00:00:00 2001 From: giswqs Date: Mon, 24 Aug 2026 13:46:11 -0400 Subject: [PATCH 08/11] Address review feedback - Return null instead of rejecting when the hidden map cannot be constructed (WebGL context failure), so a snapshot failure degrades like every other one here rather than surfacing as an unhandled rejection. - Add a regression test for the reposition cycle: install thumbnails, run the control's onRemove/onAdd pair, and assert the rebuilt panel is enhanced again. Verified it fails if the watch is anchored to the panel instead of its parent. --- .../plugins/src/plugins/basemap-thumbnails.ts | 14 +++++- tests/basemap-thumbnails.test.ts | 44 ++++++++++++++++--- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts index ad3a3031e..eb276ffa5 100644 --- a/packages/plugins/src/plugins/basemap-thumbnails.ts +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -168,7 +168,18 @@ function createStyleCamera(): { const job = async (): Promise => { await gate; if (disposed) return null; - return new Promise((resolve) => { + // `ensure()` constructs a MapLibre map, which throws when a WebGL + // context cannot be created. Every other failure here resolves to null; + // letting this one reject would surface as an unhandled rejection + // instead of the row falling back to name-only. + try { + return await capture(); + } catch { + return null; + } + }; + const capture = () => + new Promise((resolve) => { const { map } = ensure(); let settled = false; let loaded = false; @@ -210,7 +221,6 @@ function createStyleCamera(): { cancelInFlight = cancel; map.setStyle(url, { diff: false }); }); - }; const next = tail.then(job, job); tail = next.then( () => undefined, diff --git a/tests/basemap-thumbnails.test.ts b/tests/basemap-thumbnails.test.ts index 4f3127e74..d38f16db2 100644 --- a/tests/basemap-thumbnails.test.ts +++ b/tests/basemap-thumbnails.test.ts @@ -6,6 +6,7 @@ import { BASEMAP_PANEL_SELECTOR, BASEMAP_ROW_ID_ATTR, BASEMAP_ROW_SELECTOR, + installBasemapThumbnails, rasterPreviewUrl, styleUrlOf, } from "../packages/plugins/src/plugins/basemap-thumbnails"; @@ -111,6 +112,7 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { const previous = { document: globalThis.document, window: globalThis.window, + MutationObserver: globalThis.MutationObserver, requestAnimationFrame: globalThis.requestAnimationFrame, }; // The control assigns `select.value`, which linkedom exposes as a getter @@ -129,6 +131,7 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { Object.assign(globalThis, { document, window, + MutationObserver: window.MutationObserver, // linkedom ships no rAF; the control only uses it to position the panel. requestAnimationFrame: () => 0, }); @@ -137,15 +140,16 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { afterEach(() => restoreGlobals()); + /** A map stub with the surface `BasemapControl.onAdd`/`onRemove` touch. */ + function fakeMap(container: HTMLElement) { + return { getContainer: () => container, on: () => {}, off: () => {} } as never; + } + it("matches the panel, the rows and the row id attribute", () => { const container = document.createElement("div"); document.body.append(container); const control = new BasemapControl({ collapsed: false }); - control.onAdd({ - getContainer: () => container, - on: () => {}, - off: () => {}, - } as never); + control.onAdd(fakeMap(container)); const panel = container.querySelector(BASEMAP_PANEL_SELECTOR); assert.ok(panel, `no ${BASEMAP_PANEL_SELECTOR} — the control renamed its panel`); @@ -164,4 +168,34 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { `rows no longer join the catalog through ${BASEMAP_ROW_ID_ATTR}`, ); }); + + it("re-finds the panel the control rebuilds when it is repositioned", async () => { + // `setMapControlPosition` moves the control with a removeMapControl / + // addMapControl pair, and `onAdd` builds a *fresh* panel each time. The + // watch is anchored to the panel's parent — the map container, which the + // control reuses — so both the removal and the insertion land on the node + // it observes. Narrow that watch to the panel itself and thumbnails would + // silently stop appearing after a reposition, with nothing else to catch it. + const container = document.createElement("div"); + document.body.append(container); + const control = new BasemapControl({ + collapsed: false, + includeDefaultBasemaps: false, + basemaps: [raster(["https://tiles.example/{z}/{x}/{y}.png"])], + } as never); + control.onAdd(fakeMap(container)); + const thumbnails = installBasemapThumbnails(control); + const thumbnailCount = () => container.querySelectorAll(".geolibre-basemap-thumbnail").length; + + assert.equal(thumbnailCount(), 1, "the first panel was not enhanced"); + + control.onRemove(); + control.onAdd(fakeMap(container)); + assert.equal(thumbnailCount(), 0, "expected a fresh, unenhanced panel"); + + // MutationObserver callbacks are queued, not synchronous. + await new Promise((resolve) => setTimeout(resolve, 0)); + assert.equal(thumbnailCount(), 1, "the rebuilt panel was never enhanced"); + thumbnails.dispose(); + }); }); From 5debefaeb138d91ccebfbe8f8d53ee6ce7b77056 Mon Sep 17 00:00:00 2001 From: giswqs Date: Mon, 24 Aug 2026 13:55:56 -0400 Subject: [PATCH 09/11] Address review feedback - Bound the style-swatch fetch with AbortSignal.timeout on the same budget the snapshot path already used. A host that accepts the connection but never answers left the promise unsettled and the row on its placeholder for good. - Defer the raster tile request and the style JSON fetch to the same IntersectionObserver that already gated the full snapshot, so opening the Basemaps panel contacts only the providers whose rows are on screen instead of every host in the catalog at once. Rows fall back to previewing immediately where no IntersectionObserver exists. - Split the rank check out of `paint` so the raster path applies it to the row it already holds rather than re-querying the document by id. - Cover the deferral with a test that drives a stub IntersectionObserver. --- .../plugins/src/plugins/basemap-thumbnails.ts | 97 ++++++++++++------- tests/basemap-thumbnails.test.ts | 38 ++++++++ 2 files changed, 101 insertions(+), 34 deletions(-) diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts index eb276ffa5..4a3d4a6f2 100644 --- a/packages/plugins/src/plugins/basemap-thumbnails.ts +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -24,6 +24,13 @@ const ATTR = "data-geolibre-basemap-preview"; * rather than stranding every row that is waiting for a snapshot. */ const PAUSE_MS = 1500; +/** + * Caps both preview paths. A host that accepts the connection but never + * completes the response would otherwise leave the swatch promise unsettled and + * the row on its placeholder for good — the snapshot path has always had this, + * and the fetch needs it for the same reason. + */ +const PREVIEW_TIMEOUT_MS = 6000; /** A row only ever moves forward — see `paint`. */ const STATE_RANK: Record = { skip: 0, pending: 1, ready: 2, loaded: 3 }; const swatchCache = new Map>(); @@ -77,7 +84,7 @@ export function styleUrlOf(basemap: BasemapDefinition): string | null { function styleSwatch(url: string): Promise { const hit = swatchCache.get(url); if (hit) return hit; - const next = fetch(url) + const next = fetch(url, { signal: AbortSignal.timeout(PREVIEW_TIMEOUT_MS) }) .then((response) => (response.ok ? response.json() : Promise.reject(response.status))) .then((style: { layers?: Array<{ type?: string; paint?: Record }> }) => { const canvas = document.createElement("canvas"); @@ -195,7 +202,7 @@ function createStyleCamera(): { resolve(src); }; const cancel = () => finish(null); - const timer = window.setTimeout(cancel, 6000); + const timer = window.setTimeout(cancel, PREVIEW_TIMEOUT_MS); // An unreachable or invalid style URL emits `error` and never fires // `style.load`. Without this the job would hold the serialized queue // for the full timeout, delaying every later preview by 6s. Errors @@ -274,16 +281,19 @@ function rowSelector(id: string): string { return `${BASEMAP_ROW_SELECTOR}[${BASEMAP_ROW_ID_ATTR}="${CSS.escape(id)}"]`; } +function apply(row: HTMLElement, src: string, state: string): void { + // The flat-colour swatch and the real render race independently, and + // `snapCache` outlives dispose(), so a reopened panel can paint "loaded" + // before a slow swatch fetch settles. Never let a row move backwards. + const current = row.getAttribute(ATTR); + if (current && STATE_RANK[current] >= STATE_RANK[state]) return; + row.setAttribute(ATTR, state); + stamp(row, src); +} + +/** Repaint by id, for a preview that resolved long after the row was scanned. */ function paint(id: string, src: string, state: string): void { - document.querySelectorAll(rowSelector(id)).forEach((row) => { - // The flat-colour swatch and the real render race independently, and - // `snapCache` outlives dispose(), so a reopened panel can paint "loaded" - // before a slow swatch fetch settles. Never let a row move backwards. - const current = row.getAttribute(ATTR); - if (current && STATE_RANK[current] >= STATE_RANK[state]) return; - row.setAttribute(ATTR, state); - stamp(row, src); - }); + document.querySelectorAll(rowSelector(id)).forEach((row) => apply(row, src, state)); } /** @@ -315,19 +325,43 @@ export function installBasemapThumbnails(control: BasemapControl): { let panel: HTMLElement | null = null; let visible: IntersectionObserver | null = null; + /** + * Run the preview a row was prepared for. Every request a preview makes — + * the raster tile, the style JSON and the full snapshot alike — is deferred + * to here, so opening the panel contacts only the providers whose rows are + * actually on screen rather than every host in the catalog at once. + */ + function preview(row: HTMLElement): void { + const url = row.dataset.previewUrl; + const id = row.getAttribute(BASEMAP_ROW_ID_ATTR); + if (!url || !id) return; + if (row.dataset.previewKind === "raster") { + // The tile URL is ready to show, and the row is in hand — no need to go + // back through the document for it. + apply(row, url, "ready"); + return; + } + // The swatch is a flat background colour from one small JSON fetch; the + // snapshot is a real render. Both start here, and the rank in `paint` + // settles which one the row ends up showing. + void styleSwatch(url).then((src) => { + if (!active) return; + if (src) paint(id, src, "ready"); + else markSkipped(id); + }); + void camera.snapshot(url).then((src) => { + if (!active) return; + if (src) paint(id, src, "loaded"); + else markSkipped(id); + }); + } + function onVisible(entries: IntersectionObserverEntry[]): void { for (const entry of entries) { if (!entry.isIntersecting) continue; const row = entry.target as HTMLElement; visible?.unobserve(row); - const url = row.dataset.previewUrl; - const id = row.getAttribute(BASEMAP_ROW_ID_ATTR); - if (!url || !id) continue; - void camera.snapshot(url).then((src) => { - if (!active) return; - if (src) paint(id, src, "loaded"); - else markSkipped(id); - }); + preview(row); } } @@ -339,23 +373,18 @@ export function installBasemapThumbnails(control: BasemapControl): { const basemap = id ? catalog.find((item) => item.id === id) : undefined; const raster = basemap ? rasterPreviewUrl(basemap) : null; const jsonUrl = !raster && basemap ? styleUrlOf(basemap) : null; - if (raster) { - row.setAttribute(ATTR, "ready"); - stamp(row, raster); - return; - } - if (jsonUrl && id) { - row.dataset.previewUrl = jsonUrl; - row.setAttribute(ATTR, "pending"); - void styleSwatch(jsonUrl).then((src) => { - if (!active) return; - if (src) paint(id, src, "ready"); - else markSkipped(id); - }); - visible?.observe(row); + const url = raster ?? jsonUrl; + if (!url) { + row.setAttribute(ATTR, "skip"); return; } - row.setAttribute(ATTR, "skip"); + row.dataset.previewUrl = url; + row.dataset.previewKind = raster ? "raster" : "style"; + row.setAttribute(ATTR, "pending"); + // Without an IntersectionObserver there is nothing to defer to, so fall + // back to previewing every row as it is found. + if (visible) visible.observe(row); + else preview(row); }); } diff --git a/tests/basemap-thumbnails.test.ts b/tests/basemap-thumbnails.test.ts index d38f16db2..f32534cd9 100644 --- a/tests/basemap-thumbnails.test.ts +++ b/tests/basemap-thumbnails.test.ts @@ -113,6 +113,7 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { document: globalThis.document, window: globalThis.window, MutationObserver: globalThis.MutationObserver, + IntersectionObserver: globalThis.IntersectionObserver, requestAnimationFrame: globalThis.requestAnimationFrame, }; // The control assigns `select.value`, which linkedom exposes as a getter @@ -198,4 +199,41 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { assert.equal(thumbnailCount(), 1, "the rebuilt panel was never enhanced"); thumbnails.dispose(); }); + + it("defers every preview request until the row is on screen", () => { + // Opening the panel must not contact every provider in the catalog at once: + // the raster tile and the style JSON are deferred the same way the full + // snapshot always was. + const observed: HTMLElement[] = []; + let fire: (entries: IntersectionObserverEntry[]) => void = () => {}; + class FakeIntersectionObserver { + constructor(callback: (entries: IntersectionObserverEntry[]) => void) { + fire = callback; + } + observe(target: HTMLElement) { + observed.push(target); + } + unobserve() {} + disconnect() {} + } + Object.assign(globalThis, { IntersectionObserver: FakeIntersectionObserver }); + + const container = document.createElement("div"); + document.body.append(container); + const control = new BasemapControl({ + collapsed: false, + includeDefaultBasemaps: false, + basemaps: [raster(["https://tiles.example/{z}/{x}/{y}.png"])], + } as never); + control.onAdd(fakeMap(container)); + const thumbnails = installBasemapThumbnails(control); + const thumbnailCount = () => container.querySelectorAll(".geolibre-basemap-thumbnail").length; + + assert.equal(observed.length, 1, "the row was never observed"); + assert.equal(thumbnailCount(), 0, "the tile was requested before the row was visible"); + + fire([{ isIntersecting: true, target: observed[0] } as unknown as IntersectionObserverEntry]); + assert.equal(thumbnailCount(), 1, "the row was not previewed once visible"); + thumbnails.dispose(); + }); }); From 3402f642eeded07f9cd81fb600a19455fa45c997 Mon Sep 17 00:00:00 2001 From: giswqs Date: Mon, 24 Aug 2026 14:05:38 -0400 Subject: [PATCH 10/11] Address review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Reuse a snapshot already queued for the same style url. `snapCache` only fills in once a job resolves, so a second request before then queued a redundant full style load; this is the reuse `styleSwatch` already gets from caching its in-flight promise. - Add a test asserting no preview url can carry a configured credential. Previews fire on scroll rather than on an explicit pick, so reaching a keyed endpoint would spend the user's quota. The control keeps the catalog's raw templates and substitutes only when a basemap is applied, so keyed entries are already rejected — the test fails if that stops being true. --- .../plugins/src/plugins/basemap-thumbnails.ts | 13 +++++++++ tests/basemap-thumbnails.test.ts | 27 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts index 4a3d4a6f2..ecd29e2c9 100644 --- a/packages/plugins/src/plugins/basemap-thumbnails.ts +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -124,6 +124,13 @@ function createStyleCamera(): { let gate: Promise = Promise.resolve(); let openGate: (() => void) | null = null; let resumeTimer = 0; + /** + * Snapshots already queued, keyed by url. `snapCache` only fills in once a + * job has resolved, so without this a second request for the same style + * before the first settles would queue a redundant full style load — the same + * reuse `styleSwatch` gets from caching its in-flight promise. + */ + const inFlight = new Map>(); /** Settles the job that is already waiting on the hidden map, if any. */ let cancelInFlight: (() => void) | null = null; @@ -172,6 +179,8 @@ function createStyleCamera(): { snapshot(url) { const cached = snapCache.get(url); if (cached) return Promise.resolve(cached); + const queued = inFlight.get(url); + if (queued) return queued; const job = async (): Promise => { await gate; if (disposed) return null; @@ -233,6 +242,10 @@ function createStyleCamera(): { () => undefined, () => undefined, ); + inFlight.set(url, next); + void next.then(() => { + if (inFlight.get(url) === next) inFlight.delete(url); + }); return next; }, pause() { diff --git a/tests/basemap-thumbnails.test.ts b/tests/basemap-thumbnails.test.ts index f32534cd9..1220064ad 100644 --- a/tests/basemap-thumbnails.test.ts +++ b/tests/basemap-thumbnails.test.ts @@ -90,6 +90,33 @@ describe("basemap preview urls", () => { assert.equal(styleUrlOf(style("https://{s}.example.com/style.json")), null); }); + it("never previews a url carrying a configured credential", () => { + // Previews fire on scroll, not on an explicit pick, so they must never + // reach a keyed endpoint and spend the user's quota. The control keeps the + // catalog's raw `{api-key}`/`{aws-region}` templates and only substitutes + // when a basemap is actually applied, so `hasUnresolvedPlaceholder` rejects + // every keyed entry. This fails if that ever stops being true. + const secrets = { + mapboxAccessToken: "SECRET-MAPBOX", + maptilerApiKey: "SECRET-MAPTILER", + googleMapsApiKey: "SECRET-GOOGLE", + tomtomApiKey: "SECRET-TOMTOM", + hereApiKey: "SECRET-HERE", + stadiaApiKey: "SECRET-STADIA", + tiandituApiKey: "SECRET-TIANDITU", + amazonApiKey: "SECRET-AMAZON", + protomapsApiKey: "SECRET-PROTOMAPS", + }; + const control = new BasemapControl({ ...secrets, amazonRegion: "us-east-1" } as never); + for (const basemap of control.getBasemaps()) { + const url = rasterPreviewUrl(basemap) ?? styleUrlOf(basemap); + if (!url) continue; + for (const secret of Object.values(secrets)) { + assert.ok(!url.includes(secret), `${basemap.id} would preview with a credential: ${url}`); + } + } + }); + it("ignores the other source kind", () => { assert.equal(rasterPreviewUrl(style("https://tiles.openfreemap.org/styles/positron")), null); assert.equal(styleUrlOf(raster(["https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"])), null); From 539f3692e2aef6cf9c20c91a0558e90169d2270b Mon Sep 17 00:00:00 2001 From: giswqs Date: Mon, 24 Aug 2026 14:16:49 -0400 Subject: [PATCH 11/11] Address review feedback - Extract the never-move-a-row-backwards rule into an exported `advances`, and cover it directly. It was the trickiest invariant in the preview code and was only reachable through a race that no test could stage. - Add a test for the style path: a row whose swatch and snapshot both fail must fall back to the name-only layout without rejecting. Verified it fails if the snapshot's try/catch is removed, so it pins the WebGL-failure fix too. --- .../plugins/src/plugins/basemap-thumbnails.ts | 20 +++-- tests/basemap-thumbnails.test.ts | 78 +++++++++++++++++++ 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/packages/plugins/src/plugins/basemap-thumbnails.ts b/packages/plugins/src/plugins/basemap-thumbnails.ts index ecd29e2c9..2b18d2ae4 100644 --- a/packages/plugins/src/plugins/basemap-thumbnails.ts +++ b/packages/plugins/src/plugins/basemap-thumbnails.ts @@ -31,8 +31,20 @@ const PAUSE_MS = 1500; * and the fetch needs it for the same reason. */ const PREVIEW_TIMEOUT_MS = 6000; -/** A row only ever moves forward — see `paint`. */ const STATE_RANK: Record = { skip: 0, pending: 1, ready: 2, loaded: 3 }; + +/** + * Whether a row currently showing `current` may be repainted as `next`. + * + * The flat-colour swatch and the real render race independently, and + * `snapCache` outlives dispose(), so a reopened panel can paint "loaded" before + * a slow swatch fetch settles. A row only ever moves forward. An unrecognized + * state is treated as advancing, so a future state cannot silently freeze a row. + */ +export function advances(current: string | null, next: string): boolean { + if (!current) return true; + return !(STATE_RANK[current] >= STATE_RANK[next]); +} const swatchCache = new Map>(); const snapCache = new Map(); @@ -295,11 +307,7 @@ function rowSelector(id: string): string { } function apply(row: HTMLElement, src: string, state: string): void { - // The flat-colour swatch and the real render race independently, and - // `snapCache` outlives dispose(), so a reopened panel can paint "loaded" - // before a slow swatch fetch settles. Never let a row move backwards. - const current = row.getAttribute(ATTR); - if (current && STATE_RANK[current] >= STATE_RANK[state]) return; + if (!advances(row.getAttribute(ATTR), state)) return; row.setAttribute(ATTR, state); stamp(row, src); } diff --git a/tests/basemap-thumbnails.test.ts b/tests/basemap-thumbnails.test.ts index 1220064ad..37a942daa 100644 --- a/tests/basemap-thumbnails.test.ts +++ b/tests/basemap-thumbnails.test.ts @@ -6,6 +6,7 @@ import { BASEMAP_PANEL_SELECTOR, BASEMAP_ROW_ID_ATTR, BASEMAP_ROW_SELECTOR, + advances, installBasemapThumbnails, rasterPreviewUrl, styleUrlOf, @@ -25,6 +26,16 @@ function tmsRaster(tiles: string[]): BasemapDefinition { return { ...raster(tiles), source: { type: "raster", tiles, scheme: "tms" } }; } +function styleBasemap(url: string): BasemapDefinition { + return { + id: "positron", + name: "Positron", + provider: "openfreemap", + type: "style", + source: { type: "style", url }, + }; +} + function style(url: string): BasemapDefinition { return { id: "positron", @@ -131,6 +142,26 @@ describe("basemap preview urls", () => { * restate the strings, this builds a real `BasemapControl` and asks it for its * rendered panel. */ +describe("the preview state rank", () => { + it("only ever moves a row forward", () => { + // The swatch and the snapshot resolve independently, so a slow flat-colour + // swatch must not overwrite a row already showing the real render. + assert.equal(advances("ready", "loaded"), true); + assert.equal(advances("loaded", "ready"), false); + assert.equal(advances("pending", "ready"), true); + assert.equal(advances("ready", "pending"), false); + assert.equal(advances("loaded", "loaded"), false, "a repaint at the same state is a no-op"); + }); + + it("paints a row that has no state yet, and can revive a skipped one", () => { + assert.equal(advances(null, "pending"), true); + // markSkipped drops a row that produced nothing; a snapshot arriving later + // still gets to fill it in. + assert.equal(advances("skip", "loaded"), true); + assert.equal(advances("skip", "ready"), true); + }); +}); + describe("the maplibre-gl-basemap-control DOM mirror", () => { let restoreGlobals: () => void; @@ -142,6 +173,7 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { MutationObserver: globalThis.MutationObserver, IntersectionObserver: globalThis.IntersectionObserver, requestAnimationFrame: globalThis.requestAnimationFrame, + CSS: (globalThis as { CSS?: unknown }).CSS, }; // The control assigns `select.value`, which linkedom exposes as a getter // only. Give it a setter so its filter row renders. @@ -162,6 +194,10 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { MutationObserver: window.MutationObserver, // linkedom ships no rAF; the control only uses it to position the panel. requestAnimationFrame: () => 0, + // Nor `CSS.escape`, which `rowSelector` uses to build its attribute + // selector. Basemap ids are kebab-case, so escaping anything else is + // enough for these tests. + CSS: { escape: (value: string) => value.replace(/[^\w-]/g, (ch) => `\\${ch}`) }, }); restoreGlobals = () => Object.assign(globalThis, previous); }); @@ -227,6 +263,48 @@ describe("the maplibre-gl-basemap-control DOM mirror", () => { thumbnails.dispose(); }); + it("drops a style row to name-only when neither preview can be produced", async () => { + // Neither preview path can succeed here: there is no canvas 2d context for + // the swatch and no WebGL for the snapshot, which is exactly how a style + // host that is unreachable behaves. Both have to degrade quietly to the + // name-only row rather than reject or strand the placeholder. + const rejections: unknown[] = []; + const onRejection = (reason: unknown) => rejections.push(reason); + process.on("unhandledRejection", onRejection); + const fetched: string[] = []; + Object.assign(globalThis, { + fetch: (url: string) => { + fetched.push(String(url)); + return Promise.resolve({ ok: true, json: () => Promise.resolve({ layers: [] }) }); + }, + }); + + const container = document.createElement("div"); + document.body.append(container); + const control = new BasemapControl({ + collapsed: false, + includeDefaultBasemaps: false, + basemaps: [styleBasemap("https://tiles.example/style.json")], + } as never); + control.onAdd(fakeMap(container)); + const thumbnails = installBasemapThumbnails(control); + + const row = container.querySelector(BASEMAP_ROW_SELECTOR); + assert.ok(row, "no row was rendered"); + assert.equal(fetched.length, 1, "the style json was not fetched"); + + await new Promise((resolve) => setTimeout(resolve, 50)); + assert.equal( + row.getAttribute("data-geolibre-basemap-preview"), + "skip", + "the row kept its placeholder instead of falling back to name-only", + ); + assert.equal(container.querySelectorAll(".geolibre-basemap-thumbnail").length, 0); + assert.deepEqual(rejections, [], "a failed preview rejected instead of resolving null"); + process.off("unhandledRejection", onRejection); + thumbnails.dispose(); + }); + it("defers every preview request until the row is on screen", () => { // Opening the panel must not contact every provider in the catalog at once: // the raster tile and the style JSON are deferred the same way the full