Skip to content

Commit d97f330

Browse files
author
Peter Rushforth
committed
Get rid of MapLayer.pgrocessFeatures / _mapmlvectors approach
Fix tile clean up in MapTileLayer esp for CBMTILE <map-tile>s Add calculatePosition function to be shared by map-extent, -tile, -feature to determine zIndex positioning in parent based on how Leaflet layers are created and shared among sibling elements in a sequence Add red/green/blue-tile.png Add setZIndex function to MapFeatureLayer, MapTileLayer, copied from MapExtentLayer. Update MapFeatureLayer, MapExtentLayer, MapTileLayer initialize constructors to set the zIndex if there's an option present for it Replace map-extent way of setting zIndex to use map-extent.position Adapt calculatePosition to working within either shadow root or light DOM add map-tile tests make map-tile attributes row,col,zoom immutable; add tests for that Fix a few small issues with map-tile attributes Clean up map-feature connectedCallback (remove unused bind) Add timeouts to decrease flakiness Replace page screenshots with viewer screenshots When you remove the src attribute from a layer, it's now empty, and should NOT be disabled. Remove test dependency on external services If this is the last feature or tile to be removed from a static layer context, i.e. as child of a <map-layer>, the container for the MapFeatureLayer or the MapTileLayer should be removed from the layer rendered dom; includes basic tests of this for both types of static content. Add code+test to ensure that TemplatedFeaturesOrTilesLayer.js does not allow dynamic content loaded by a <map-link rel=features> to contain a map-extent Update map-feature.disconnectedCallback to clean up when it's the last feature in a MapFeatureLayer being disconnected, make it remove the <svg> (_container) element created by the renderer. Because Renderer inherits from Layer, we can run its remove() method to do this. Because `disabled` is an observed attribute on <map-linK>, purge the shadowRoot contents when the link is disabled. Add test for <svg> count when panning and zooming the map to prevent regressions hopefully. Keep count of logged messages by map-layer.getProjection(), limit to 1 Change playwright config which was causing problems locally, will hopefully not affect CI Make nodeName dep on returning all CAPS equal by map-tile, map-feature Remove unused method parameter
1 parent df49def commit d97f330

File tree

66 files changed

+1791
-869
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

66 files changed

+1791
-869
lines changed

playwright.config.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ export default defineConfig({
55
testDir: './test/e2e',
66
webServer: {
77
command: 'node test/server.js',
8-
port: 30001,
9-
timeout: 10000
8+
// port: 30001, // this causes a 2 min pause on my machine as of Sept 2025
9+
// url: 'http://localhost:30001/', //also causes a 2 min pause
10+
timeout: 10000,
11+
reuseExistingServer: true
1012
},
1113
use: {
1214
headless: true,

src/layer.js

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { setOptions, DomUtil, bounds, point } from 'leaflet';
33
import { Util } from './mapml/utils/Util.js';
44
import { MapLayer, mapLayer } from './mapml/layers/MapLayer.js';
55
import { MapTileLayer } from './mapml/layers/MapTileLayer.js';
6+
import { MapFeatureLayer } from './mapml/layers/MapFeatureLayer.js';
67
import { createLayerControlHTML } from './mapml/elementSupport/layers/createLayerControlForLayer.js';
78

89
export class BaseLayerElement extends HTMLElement {
@@ -176,6 +177,8 @@ export class BaseLayerElement extends HTMLElement {
176177
// the initial value of this._opacity should be set as opacity attribute value, if exists, or the default value 1.0
177178
this._opacity = this.opacity || 1.0;
178179
this.attachShadow({ mode: 'open' });
180+
// by keeping track of console.log, we can avoid overwhelming the console
181+
this.loggedMessages = new Set();
179182
}
180183
disconnectedCallback() {
181184
// if the map-layer node is removed from the dom, the layer should be
@@ -458,29 +461,20 @@ export class BaseLayerElement extends HTMLElement {
458461
Array.from(mapml.querySelectorAll('map-extent[units]'))
459462
) || projection;
460463
} else {
461-
console.log(
462-
`A projection was not assigned to the '${mapml.label}' Layer. Please specify a projection for that layer using a map-meta element. See more here - https://maps4html.org/web-map-doc/docs/elements/meta/`
463-
);
464+
const message = `A projection was not assigned to the '${mapml.label}' Layer. \nPlease specify a projection for that layer using a map-meta element. \nSee more here - https://maps4html.org/web-map-doc/docs/elements/meta/`;
465+
if (!this.loggedMessages.has(message)) {
466+
console.log(message);
467+
this.loggedMessages.add(message);
468+
}
464469
}
465470
return projection;
466471
}
467472
/*
468-
* Runs the effects of the mutation observer, which is to add map-features' and
469-
* map-extents' leaflet layer implementations to the appropriate container in
470-
* the map-layer._layer: either as a sub-layer directly in the LayerGroup
471-
* (MapLayer._layer) or as a sub-layer in the MapLayer._mapmlvectors
472-
* FeatureGroup
473+
* Runs the effects of the mutation observer for child elements of map-layer.
474+
* This method primarily handles extent recalculation and other
475+
* child element processing.
473476
*/
474477
_runMutationObserver(elementsGroup) {
475-
const _addFeatureToMapMLVectors = (feature) => {
476-
this.whenReady().then(() => {
477-
// the layer extent must change as features are added, this.extent
478-
// property only recalculates the bounds and zoomBounds when .bounds
479-
// doesn't exist, so delete it to ensure that the extent is reset
480-
delete this._layer.bounds;
481-
feature.addFeature(this._layer._mapmlvectors);
482-
});
483-
};
484478
const _addStylesheetLink = (mapLink) => {
485479
this.whenReady().then(() => {
486480
this._layer.renderStyles(mapLink);
@@ -493,8 +487,6 @@ export class BaseLayerElement extends HTMLElement {
493487
};
494488
const _addExtentElement = (mapExtent) => {
495489
this.whenReady().then(() => {
496-
// see comment regarding features / extent. Same thing applies to
497-
// map-extent
498490
delete this._layer.bounds;
499491
this._validateDisabled();
500492
});
@@ -512,9 +504,6 @@ export class BaseLayerElement extends HTMLElement {
512504
for (let i = 0; i < elementsGroup.length; ++i) {
513505
let element = elementsGroup[i];
514506
switch (element.nodeName) {
515-
case 'MAP-FEATURE':
516-
_addFeatureToMapMLVectors(element);
517-
break;
518507
case 'MAP-LINK':
519508
if (element.link && !element.link.isConnected)
520509
_addStylesheetLink(element);
@@ -596,17 +585,15 @@ export class BaseLayerElement extends HTMLElement {
596585
mapprojection: proj,
597586
opacity: window.getComputedStyle(this).opacity
598587
});
599-
// make sure the Leaflet layer has a reference to the map
600-
this._layer._map = this.parentNode._map;
601588

602589
if (this.checked) {
603-
this._layer.addTo(this._layer._map);
590+
this._layer.addTo(this.parentNode._map);
591+
// toggle the this.disabled attribute depending on whether the layer
592+
// is: same prj as map, within view/zoom of map
604593
}
594+
this.parentNode._map.on('moveend layeradd', this._validateDisabled, this);
605595

606596
this._layer.on('add remove', this._validateDisabled, this);
607-
// toggle the this.disabled attribute depending on whether the layer
608-
// is: same prj as map, within view/zoom of map
609-
this._layer._map.on('moveend layeradd', this._validateDisabled, this);
610597

611598
if (this.parentNode._layerControl)
612599
this._layerControl = this.parentNode._layerControl;
@@ -647,6 +634,19 @@ export class BaseLayerElement extends HTMLElement {
647634

648635
return { totalCount, disabledCount };
649636
};
637+
const countFeatureLayers = () => {
638+
let totalCount = 0;
639+
let disabledCount = 0;
640+
641+
this._layer.eachLayer((layer) => {
642+
if (layer instanceof MapFeatureLayer) {
643+
totalCount++;
644+
if (!layer.isVisible()) disabledCount++;
645+
}
646+
});
647+
648+
return { totalCount, disabledCount };
649+
};
650650
// setTimeout is necessary to make the validateDisabled happen later than the moveend operations etc.,
651651
// to ensure that the validated result is correct
652652
setTimeout(() => {
@@ -687,8 +687,9 @@ export class BaseLayerElement extends HTMLElement {
687687
}
688688
} else if (type === '_mapmlvectors') {
689689
// inline / static features
690-
totalExtentCount++;
691-
if (!layer[type].isVisible()) disabledExtentCount++;
690+
const featureLayerCounts = countFeatureLayers();
691+
totalExtentCount += featureLayerCounts.totalCount;
692+
disabledExtentCount += featureLayerCounts.disabledCount;
692693
} else {
693694
// inline tiles
694695
const tileLayerCounts = countTileLayers();

src/map-extent.js

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { bounds as Lbounds, point as Lpoint } from 'leaflet';
33
import { Util } from './mapml/utils/Util.js';
44
import { mapExtentLayer } from './mapml/layers/MapExtentLayer.js';
55
import { createLayerControlExtentHTML } from './mapml/elementSupport/extents/createLayerControlForExtent.js';
6+
import { calculatePosition } from './mapml/elementSupport/layers/calculatePosition.js';
67

78
/* global M */
89
export class HTMLExtentElement extends HTMLElement {
@@ -77,6 +78,9 @@ export class HTMLExtentElement extends HTMLElement {
7778
? getExtent(this)
7879
: getCalculatedExtent(this);
7980
}
81+
get position() {
82+
return calculatePosition(this);
83+
}
8084

8185
getOuterHTML() {
8286
let tempElement = this.cloneNode(true);
@@ -266,11 +270,7 @@ export class HTMLExtentElement extends HTMLElement {
266270
this._extentLayer = mapExtentLayer({
267271
opacity: this.opacity,
268272
crs: M[this.units],
269-
extentZIndex: Array.from(
270-
this.parentLayer.src
271-
? this.parentLayer.shadowRoot.querySelectorAll(':host > map-extent')
272-
: this.parentLayer.querySelectorAll(':scope > map-extent')
273-
).indexOf(this),
273+
zIndex: this.position,
274274
extentEl: this
275275
});
276276
// this._layerControlHTML is the fieldset for the extent in the LayerControl
@@ -434,13 +434,7 @@ export class HTMLExtentElement extends HTMLElement {
434434
if (this.checked && !this.disabled && this.parentLayer._layer) {
435435
// can be added to MapLayer LayerGroup no matter map-layer is checked or not
436436
this._extentLayer.addTo(this.parentLayer._layer);
437-
this._extentLayer.setZIndex(
438-
Array.from(
439-
this.parentLayer.src
440-
? this.parentLayer.shadowRoot.querySelectorAll(':host > map-extent')
441-
: this.parentLayer.querySelectorAll(':scope > map-extent')
442-
).indexOf(this)
443-
);
437+
this._extentLayer.setZIndex(this.position);
444438
} else {
445439
this.parentLayer._layer?.removeLayer(this._extentLayer);
446440
}

src/map-feature.js

Lines changed: 96 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
import { bounds, point, extend } from 'leaflet';
22

3-
import { featureLayer } from './mapml/layers/FeatureLayer.js';
3+
import { MapFeatureLayer } from './mapml/layers/MapFeatureLayer.js';
44
import { featureRenderer } from './mapml/features/featureRenderer.js';
55
import { Util } from './mapml/utils/Util.js';
66
import proj4 from 'proj4';
7+
import { calculatePosition } from './mapml/elementSupport/layers/calculatePosition.js';
78

89
export class HTMLFeatureElement extends HTMLElement {
910
static get observedAttributes() {
1011
return ['zoom', 'min', 'max'];
1112
}
1213

1314
/* jshint ignore:start */
14-
#hasConnected;
15+
#hasConnected; // prevents attributeChangedCallback before connectedCallback
1516
/* jshint ignore:end */
1617
get zoom() {
1718
// for templated or queried features ** native zoom is only used for zoomTo() **
@@ -138,6 +139,9 @@ export class HTMLFeatureElement extends HTMLElement {
138139
return this._getFeatureExtent();
139140
}
140141
}
142+
get position() {
143+
return calculatePosition(this);
144+
}
141145
getMapEl() {
142146
return Util.getClosest(this, 'mapml-viewer,map[is=web-map]');
143147
}
@@ -172,17 +176,21 @@ export class HTMLFeatureElement extends HTMLElement {
172176
// used for fallback zoom getter for static features
173177
this._initialZoom = this.getMapEl().zoom;
174178
this._parentEl =
175-
this.parentNode.nodeName.toUpperCase() === 'MAP-LAYER' ||
176-
this.parentNode.nodeName.toUpperCase() === 'LAYER-' ||
177-
this.parentNode.nodeName.toUpperCase() === 'MAP-LINK'
179+
this.parentNode.nodeName === 'MAP-LAYER' ||
180+
this.parentNode.nodeName === 'LAYER-' ||
181+
this.parentNode.nodeName === 'MAP-LINK'
178182
? this.parentNode
179183
: this.parentNode.host;
180184
if (
181185
this.getLayerEl().hasAttribute('data-moving') ||
182186
this._parentEl.parentElement?.hasAttribute('data-moving')
183187
)
184188
return;
185-
if (this._parentEl.nodeName === 'MAP-LINK') {
189+
if (
190+
this._parentEl.nodeName === 'MAP-LAYER' ||
191+
this._parentEl.nodeName === 'LAYER-' ||
192+
this._parentEl.nodeName === 'MAP-LINK'
193+
) {
186194
this._createOrGetFeatureLayer();
187195
}
188196
// use observer to monitor the changes in mapFeature's subtree
@@ -215,6 +223,17 @@ export class HTMLFeatureElement extends HTMLElement {
215223
this._observer.disconnect();
216224
if (this._featureLayer) {
217225
this.removeFeature(this._featureLayer);
226+
// If this was the last feature in the layer, clean up the layer
227+
if (this._featureLayer.getLayers().length === 0) {
228+
if (this._featureLayer.options.renderer) {
229+
// since this is the last reference to the MapFeatureLayer, we need to also
230+
// manually remove the shared renderer
231+
this._featureLayer.options.renderer.remove();
232+
}
233+
this._featureLayer.remove();
234+
this._featureLayer = null;
235+
delete this._featureLayer;
236+
}
218237
}
219238
}
220239

@@ -292,65 +311,80 @@ export class HTMLFeatureElement extends HTMLElement {
292311
return this.previousElementSibling;
293312
}
294313
_createOrGetFeatureLayer() {
295-
if (this.isFirst() && this._parentEl._templatedLayer) {
296-
const parentElement = this._parentEl;
297-
298-
let map = parentElement.getMapEl()._map;
299-
300-
// Create a new FeatureLayer
301-
this._featureLayer = featureLayer(null, {
302-
// pass the vector layer a renderer of its own, otherwise leaflet
303-
// puts everything into the overlayPane
304-
renderer: featureRenderer(),
305-
// pass the vector layer the container for the parent into which
306-
// it will append its own container for rendering into
307-
pane: parentElement._templatedLayer.getContainer(),
308-
// the bounds will be static, fixed, constant for the lifetime of the layer
309-
layerBounds: parentElement.getBounds(),
310-
zoomBounds: this._getZoomBounds(),
311-
projection: map.options.projection,
312-
mapEl: parentElement.getMapEl(),
313-
onEachFeature: function (properties, geometry) {
314-
if (properties) {
315-
const popupOptions = {
316-
autoClose: false,
317-
autoPan: true,
318-
maxHeight: map.getSize().y * 0.5 - 50,
319-
maxWidth: map.getSize().x * 0.7,
320-
minWidth: 165
321-
};
322-
var c = document.createElement('div');
323-
c.classList.add('mapml-popup-content');
324-
c.insertAdjacentHTML('afterbegin', properties.innerHTML);
325-
geometry.bindPopup(c, popupOptions);
314+
// Wait for parent layer to be ready before proceeding
315+
this._parentEl
316+
.whenReady()
317+
.then(() => {
318+
// Detect parent context and get the appropriate layer container
319+
const isMapLink = this._parentEl.nodeName === 'MAP-LINK';
320+
const parentLayer = isMapLink
321+
? this._parentEl._templatedLayer
322+
: this._parentEl._layer;
323+
324+
if (this.isFirst() && parentLayer) {
325+
const parentElement = this._parentEl;
326+
327+
let map = parentElement.getMapEl()._map;
328+
329+
this._featureLayer = new MapFeatureLayer(null, {
330+
// pass the vector layer a renderer of its own, otherwise leaflet
331+
// puts everything into the overlayPane
332+
// with this feature creating its own MapFeatureLayer for each
333+
// sub-sequence of features, it means that there may be > 1 <svg>
334+
// container (one per renderer) in the pane...
335+
renderer: featureRenderer(),
336+
// pass the vector layer the container for the parent into which
337+
// it will append its own container for rendering into
338+
pane: parentLayer.getContainer(),
339+
// the bounds will be static, fixed, constant for the lifetime of a (templated) layer
340+
...(isMapLink && parentElement.getBounds()
341+
? { layerBounds: parentElement.getBounds() }
342+
: {}),
343+
...(isMapLink ? { zoomBounds: this._getZoomBounds() } : {}),
344+
...(isMapLink ? {} : { _leafletLayer: parentElement._layer }),
345+
zIndex: this.position,
346+
projection: map.options.projection,
347+
mapEl: parentElement.getMapEl(),
348+
onEachFeature: function (properties, geometry) {
349+
if (properties) {
350+
const popupOptions = {
351+
autoClose: false,
352+
autoPan: true,
353+
maxHeight: map.getSize().y * 0.5 - 50,
354+
maxWidth: map.getSize().x * 0.7,
355+
minWidth: 165
356+
};
357+
var c = document.createElement('div');
358+
c.classList.add('mapml-popup-content');
359+
c.insertAdjacentHTML('afterbegin', properties.innerHTML);
360+
geometry.bindPopup(c, popupOptions);
361+
}
362+
}
363+
});
364+
// this is used by DebugOverlay testing "multipleExtents.test.js
365+
// but do we really need or want each feature to have the bounds of the
366+
// map link? tbd
367+
extend(this._featureLayer.options, {
368+
_leafletLayer: Object.assign(this._featureLayer, {
369+
_layerEl: this.getLayerEl()
370+
})
371+
});
372+
373+
this.addFeature(this._featureLayer);
374+
375+
// add MapFeatureLayer to appropriate parent layer
376+
parentLayer.addLayer(this._featureLayer);
377+
} else {
378+
// get the previous feature's layer
379+
this._featureLayer = this.getPrevious()?._featureLayer;
380+
if (this._featureLayer) {
381+
this.addFeature(this._featureLayer);
326382
}
327383
}
384+
})
385+
.catch((error) => {
386+
console.log('Error waiting for parent layer to be ready:', error);
328387
});
329-
// this is used by DebugOverlay testing "multipleExtents.test.js
330-
// but do we really need or want each feature to have the bounds of the
331-
// map link? tbd
332-
extend(this._featureLayer.options, {
333-
_leafletLayer: Object.assign(this._featureLayer, {
334-
_layerEl: this.getLayerEl()
335-
})
336-
});
337-
338-
this.addFeature(this._featureLayer);
339-
340-
// add featureLayer to TemplatedFeaturesOrTilesLayer of the parentElement
341-
if (
342-
parentElement._templatedLayer &&
343-
parentElement._templatedLayer.addLayer
344-
) {
345-
parentElement._templatedLayer.addLayer(this._featureLayer);
346-
}
347-
} else {
348-
// get the previous feature's layer
349-
this._featureLayer = this.getPrevious()?._featureLayer;
350-
if (this._featureLayer) {
351-
this.addFeature(this._featureLayer);
352-
}
353-
}
354388
}
355389
_setUpEvents() {
356390
['click', 'focus', 'blur', 'keyup', 'keydown'].forEach((name) => {

0 commit comments

Comments
 (0)