From 25090230f2b2fbb784cf38930e9d0cbde9bdc3eb Mon Sep 17 00:00:00 2001 From: efiten Date: Wed, 2 Sep 2026 15:12:55 +0200 Subject: [PATCH] fix(map): render the Esri labels overlay it was already named for (rebase of #1917) (#1935) Continues #1917 by @nullrouten0. The commit is theirs, authorship unchanged; I only rebased it onto master. It went CONFLICTING because #1891 (the OpenTopoMap and USGS layers) landed in the same `BASE_STYLES` block, and both PRs also add cases to `test-issue-1420-tile-providers.js`. Resolution: kept #1891's two `usgs-*` entries and took this PR's `esri-darkgray-labels` line, which is the one that adds `refUrl`. Both test suites kept in full. Nothing else touched. Verified: `test-issue-1420-tile-providers.js` 47 passed, 0 failed, which includes this PR's four Esri cases and the Carto key cases that landed since. My review stands: approve. The id `esri-darkgray-labels` promised labels the layer control never stacked, and the test asserting that single-layer providers stay bare tile layers is the part that makes this safe to merge. Co-authored-by: nullrouten Co-authored-by: Claude Opus 5 (1M context) --- public/map-tile-providers.js | 14 +++++- test-issue-1420-tile-providers.js | 76 +++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/public/map-tile-providers.js b/public/map-tile-providers.js index aadff608..74fe94e1 100644 --- a/public/map-tile-providers.js +++ b/public/map-tile-providers.js @@ -62,7 +62,7 @@ 'stamen-toner-dark': { provider: 'stamen', label: 'Stamen Toner Lite', url: _getStamenUrl, invertFilter: INVERT_CSS, type: 'dark', attribution: '© Stadia Maps © Stamen Design © OpenStreetMap', maxZoom: 20 }, 'usgs-topo': { provider: 'usgs', label: 'USGS Topographic', url: function() { return 'https://basemap.nationalmap.gov/arcgis/rest/services/USGSTopo/MapServer/tile/{z}/{y}/{x}'; }, invertFilter: null, type: 'light', attribution: 'U.S. Geological Survey, National Geospatial Program', maxZoom: 16 }, 'usgs-imagery': { provider: 'usgs', label: 'USGS Imagery', url: function() { return 'https://basemap.nationalmap.gov/arcgis/rest/services/USGSImageryTopo/MapServer/tile/{z}/{y}/{x}'; }, invertFilter: null, type: 'light', attribution: 'U.S. Geological Survey, National Geospatial Program', maxZoom: 16 }, - 'esri-darkgray-labels': { provider: 'esri', label: 'Esri Dark Gray Canvas', url: function() { return 'https://server.arcgisonline.com/ArcGIS/rest/services/Canvas/World_Dark_Gray_Base/MapServer/tile/{z}/{y}/{x}'; }, invertFilter: null, type: 'dark', attribution: 'Tiles © Esri', maxZoom: 19 } + 'esri-darkgray-labels': { provider: 'esri', label: 'Esri Dark Gray Canvas', url: function() { return 'https://server.arcgisonline.com/ArcGIS/rest/services/Canvas/World_Dark_Gray_Base/MapServer/tile/{z}/{y}/{x}'; }, refUrl: 'https://server.arcgisonline.com/ArcGIS/rest/services/Canvas/World_Dark_Gray_Reference/MapServer/tile/{z}/{y}/{x}', invertFilter: null, type: 'dark', attribution: 'Tiles © Esri', maxZoom: 19 } }; var REGISTRY = {}; @@ -278,7 +278,17 @@ function _makeLayer(id) { var p = REGISTRY[id]; var url = typeof p.url === 'function' ? p.url() : p.url; - var layer = L.tileLayer(url, { attribution: p.attribution || '', maxZoom: p.maxZoom || 19 }); + var opts = { attribution: p.attribution || '', maxZoom: p.maxZoom || 19 }; + var layer = L.tileLayer(url, opts); + + // Two-layer providers (Esri Dark Gray Canvas) ship their place labels as + // a separate transparent reference tileset that has to be stacked on the + // base. The theme-synced path in map.js/live.js already does this; the + // layer control did not, so picking such a provider explicitly produced + // a map with no place names at all. + if (p.refUrl && typeof L.layerGroup === 'function') { + layer = L.layerGroup([layer, L.tileLayer(p.refUrl, opts)]); + } // Every explicit layer enforces its own filter and locks the pane layer.on('add', function () { diff --git a/test-issue-1420-tile-providers.js b/test-issue-1420-tile-providers.js index 15c3b3cc..5c9c4854 100644 --- a/test-issue-1420-tile-providers.js +++ b/test-issue-1420-tile-providers.js @@ -457,6 +457,11 @@ test('MC_createLayerControl handles Auto mode and explicit layers correctly', () createdLayers.push(layer); return layer; }, + layerGroup: (layers) => { + const group = { _isGroup: true, _layers: layers, _events: {} }; + group.on = (ev, cb) => { group._events[ev] = cb; }; + return group; + }, control: { layers: (maps) => { ctx._capturedBaseMaps = maps; return mockControl; } } @@ -599,6 +604,77 @@ test('MC_tileUrlById returns the fallback for a disabled or unknown provider', ( }); +// ─── Esri two-layer provider (base + labels reference) ────────────────────── + +test('Esri Dark Gray Canvas declares a labels reference layer', () => { + const ctx = makeSandbox(); + loadProviders(ctx, {}); + const p = ctx.window.MC_TILE_PROVIDERS['esri-darkgray-labels']; + assert.ok(p, 'esri provider should be registered'); + assert.ok(p.refUrl, 'must declare refUrl — the id promises labels'); + assert.ok(p.refUrl.indexOf('World_Dark_Gray_Reference') >= 0, 'refUrl points at the Reference tileset: ' + p.refUrl); +}); + +test('Esri refUrl is a string, matching how map.js/live.js/nodes.js consume it', () => { + const ctx = makeSandbox(); + loadProviders(ctx, {}); + const p = ctx.window.MC_TILE_PROVIDERS['esri-darkgray-labels']; + assert.strictEqual(typeof p.refUrl, 'string', 'consumers pass refUrl straight to L.tileLayer'); + assert.ok(/\{z\}\/\{y\}\/\{x\}/.test(p.refUrl), 'Esri uses {z}/{y}/{x} order: ' + p.refUrl); +}); + +test('Single-layer providers declare no refUrl', () => { + const ctx = makeSandbox(); + loadProviders(ctx, { tiles: { providers: { carto: { enabled: true } } } }); + for (const id of ALL_CARTO_IDS) { + assert.ok(!ctx.window.MC_TILE_PROVIDERS[id].refUrl, id + ' should not declare refUrl'); + } +}); + +test('Layer control stacks base + labels for a two-layer provider', () => { + const ctx = makeSandbox(); + const created = []; + const groups = []; + ctx.L = ctx.window.L = { + tileLayer: (url, opts) => { const l = { url, _events: {} }; l.on = (e, c) => { l._events[e] = c; }; created.push(l); return l; }, + layerGroup: (layers) => { const g = { _isGroup: true, _layers: layers, _events: {} }; g.on = (e, c) => { g._events[e] = c; }; groups.push(g); return g; }, + control: { layers: (maps) => { ctx._capturedBaseMaps = maps; return { addTo: function () { return this; } }; } } + }; + const mockMap = { + hasLayer: () => false, addLayer: () => {}, removeLayer: () => {}, + on: () => {}, off: () => {}, getPane: () => ctx.tilePane + }; + loadProviders(ctx, {}); + ctx.window.MC_initTileRegistry(false); + ctx.window.MC_createLayerControl(mockMap, { _isAutoGroup: true }); + + const entry = ctx._capturedBaseMaps['esri-darkgray-labels']; + assert.ok(entry, 'esri should appear in the control'); + assert.ok(entry._isGroup, 'esri entry must be a layer group, not a bare tile layer'); + assert.strictEqual(entry._layers.length, 2, 'group holds base + reference'); + assert.ok(entry._layers[1].url.indexOf('World_Dark_Gray_Reference') >= 0, 'second layer is the labels overlay'); +}); + +test('Layer control still builds a bare tile layer for single-layer providers', () => { + const ctx = makeSandbox(); + ctx.L = ctx.window.L = { + tileLayer: (url) => { const l = { url, _events: {} }; l.on = (e, c) => { l._events[e] = c; }; return l; }, + layerGroup: (layers) => ({ _isGroup: true, _layers: layers, on: () => {} }), + control: { layers: (maps) => { ctx._capturedBaseMaps = maps; return { addTo: function () { return this; } }; } } + }; + const mockMap = { + hasLayer: () => false, addLayer: () => {}, removeLayer: () => {}, + on: () => {}, off: () => {}, getPane: () => ctx.tilePane + }; + loadProviders(ctx, { tiles: { providers: { carto: { enabled: true } } } }); + ctx.window.MC_initTileRegistry(false); + ctx.window.MC_createLayerControl(mockMap, { _isAutoGroup: true }); + + const entry = ctx._capturedBaseMaps['carto-dark']; + assert.ok(entry, 'carto-dark should appear in the control'); + assert.ok(!entry._isGroup, 'single-layer provider must stay a bare tile layer'); +}); + process.on('beforeExit', () => { console.log(''); console.log(' ' + passed + ' passed, ' + failed + ' failed');