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.

Author: Peter Rushforth

src/layer.js

@@ -3,6 +3,7 @@ import { setOptions, DomUtil, bounds, point } from 'leaflet';
 import { Util } from './mapml/utils/Util.js';
 import { MapLayer, mapLayer } from './mapml/layers/MapLayer.js';
 import { MapTileLayer } from './mapml/layers/MapTileLayer.js';
+import { MapFeatureLayer } from './mapml/layers/MapFeatureLayer.js';
 import { createLayerControlHTML } from './mapml/elementSupport/layers/createLayerControlForLayer.js';
 
 export class BaseLayerElement extends HTMLElement {
@@ -465,22 +466,11 @@ export class BaseLayerElement extends HTMLElement {
     return projection;
   }
   /*
-   * Runs the effects of the mutation observer, which is to add map-features' and
-   * map-extents' leaflet layer implementations to the appropriate container in
-   * the map-layer._layer: either as a sub-layer directly in the LayerGroup
-   * (MapLayer._layer) or as a sub-layer in the MapLayer._mapmlvectors
-   * FeatureGroup
+   * Runs the effects of the mutation observer for child elements of map-layer.
+   * This method primarily handles extent recalculation and other
+   * child element processing.
    */
   _runMutationObserver(elementsGroup) {
-    const _addFeatureToMapMLVectors = (feature) => {
-      this.whenReady().then(() => {
-        // the layer extent must change as features are added, this.extent
-        // property only recalculates the bounds and zoomBounds when .bounds
-        // doesn't exist, so delete it to ensure that the extent is reset
-        delete this._layer.bounds;
-        feature.addFeature(this._layer._mapmlvectors);
-      });
-    };
     const _addStylesheetLink = (mapLink) => {
       this.whenReady().then(() => {
         this._layer.renderStyles(mapLink);
@@ -493,8 +483,6 @@ export class BaseLayerElement extends HTMLElement {
     };
     const _addExtentElement = (mapExtent) => {
       this.whenReady().then(() => {
-        // see comment regarding features / extent. Same thing applies to
-        // map-extent
         delete this._layer.bounds;
         this._validateDisabled();
       });
@@ -512,9 +500,6 @@ export class BaseLayerElement extends HTMLElement {
     for (let i = 0; i < elementsGroup.length; ++i) {
       let element = elementsGroup[i];
       switch (element.nodeName) {
-        case 'MAP-FEATURE':
-          _addFeatureToMapMLVectors(element);
-          break;
         case 'MAP-LINK':
           if (element.link && !element.link.isConnected)
             _addStylesheetLink(element);
@@ -596,17 +581,15 @@ export class BaseLayerElement extends HTMLElement {
       mapprojection: proj,
       opacity: window.getComputedStyle(this).opacity
     });
-    // make sure the Leaflet layer has a reference to the map
-    this._layer._map = this.parentNode._map;
 
     if (this.checked) {
-      this._layer.addTo(this._layer._map);
+      this._layer.addTo(this.parentNode._map);
+      // toggle the this.disabled attribute depending on whether the layer
+      // is: same prj as map, within view/zoom of map
     }
+    this.parentNode._map.on('moveend layeradd', this._validateDisabled, this);
 
     this._layer.on('add remove', this._validateDisabled, this);
-    // toggle the this.disabled attribute depending on whether the layer
-    // is: same prj as map, within view/zoom of map
-    this._layer._map.on('moveend layeradd', this._validateDisabled, this);
 
     if (this.parentNode._layerControl)
       this._layerControl = this.parentNode._layerControl;
@@ -647,6 +630,19 @@ export class BaseLayerElement extends HTMLElement {
 
       return { totalCount, disabledCount };
     };
+    const countFeatureLayers = () => {
+      let totalCount = 0;
+      let disabledCount = 0;
+
+      this._layer.eachLayer((layer) => {
+        if (layer instanceof MapFeatureLayer) {
+          totalCount++;
+          if (!layer.isVisible()) disabledCount++;
+        }
+      });
+
+      return { totalCount, disabledCount };
+    };
     // setTimeout is necessary to make the validateDisabled happen later than the moveend operations etc.,
     // to ensure that the validated result is correct
     setTimeout(() => {
@@ -687,8 +683,9 @@ export class BaseLayerElement extends HTMLElement {
                   }
                 } else if (type === '_mapmlvectors') {
                   // inline / static features
-                  totalExtentCount++;
-                  if (!layer[type].isVisible()) disabledExtentCount++;
+                  const featureLayerCounts = countFeatureLayers();
+                  totalExtentCount += featureLayerCounts.totalCount;
+                  disabledExtentCount += featureLayerCounts.disabledCount;
                 } else {
                   // inline tiles
                   const tileLayerCounts = countTileLayers();

src/map-extent.js

@@ -3,6 +3,7 @@ import { bounds as Lbounds, point as Lpoint } from 'leaflet';
 import { Util } from './mapml/utils/Util.js';
 import { mapExtentLayer } from './mapml/layers/MapExtentLayer.js';
 import { createLayerControlExtentHTML } from './mapml/elementSupport/extents/createLayerControlForExtent.js';
+import { calculatePosition } from './mapml/elementSupport/layers/calculatePosition.js';
 
 /* global M */
 export class HTMLExtentElement extends HTMLElement {
@@ -77,6 +78,9 @@ export class HTMLExtentElement extends HTMLElement {
       ? getExtent(this)
       : getCalculatedExtent(this);
   }
+  get position() {
+    return calculatePosition(this);
+  }
 
   getOuterHTML() {
     let tempElement = this.cloneNode(true);
@@ -266,11 +270,7 @@ export class HTMLExtentElement extends HTMLElement {
     this._extentLayer = mapExtentLayer({
       opacity: this.opacity,
       crs: M[this.units],
-      extentZIndex: Array.from(
-        this.parentLayer.src
-          ? this.parentLayer.shadowRoot.querySelectorAll(':host > map-extent')
-          : this.parentLayer.querySelectorAll(':scope > map-extent')
-      ).indexOf(this),
+      zIndex: this.position,
       extentEl: this
     });
     // this._layerControlHTML is the fieldset for the extent in the LayerControl
@@ -434,13 +434,7 @@ export class HTMLExtentElement extends HTMLElement {
     if (this.checked && !this.disabled && this.parentLayer._layer) {
       // can be added to MapLayer LayerGroup no matter map-layer is checked or not
       this._extentLayer.addTo(this.parentLayer._layer);
-      this._extentLayer.setZIndex(
-        Array.from(
-          this.parentLayer.src
-            ? this.parentLayer.shadowRoot.querySelectorAll(':host > map-extent')
-            : this.parentLayer.querySelectorAll(':scope > map-extent')
-        ).indexOf(this)
-      );
+      this._extentLayer.setZIndex(this.position);
     } else {
       this.parentLayer._layer?.removeLayer(this._extentLayer);
     }

src/map-feature.js

@@ -1,17 +1,18 @@
 import { bounds, point, extend } from 'leaflet';
 
-import { featureLayer } from './mapml/layers/FeatureLayer.js';
+import { MapFeatureLayer } from './mapml/layers/MapFeatureLayer.js';
 import { featureRenderer } from './mapml/features/featureRenderer.js';
 import { Util } from './mapml/utils/Util.js';
 import proj4 from 'proj4';
+import { calculatePosition } from './mapml/elementSupport/layers/calculatePosition.js';
 
 export class HTMLFeatureElement extends HTMLElement {
   static get observedAttributes() {
     return ['zoom', 'min', 'max'];
   }
 
   /* jshint ignore:start */
-  #hasConnected;
+  #hasConnected; // prevents attributeChangedCallback before connectedCallback
   /* jshint ignore:end */
   get zoom() {
     // for templated or queried features ** native zoom is only used for zoomTo() **
@@ -138,6 +139,9 @@ export class HTMLFeatureElement extends HTMLElement {
       return this._getFeatureExtent();
     }
   }
+  get position() {
+    return calculatePosition(this);
+  }
   getMapEl() {
     return Util.getClosest(this, 'mapml-viewer,map[is=web-map]');
   }
@@ -172,17 +176,21 @@ export class HTMLFeatureElement extends HTMLElement {
     // used for fallback zoom getter for static features
     this._initialZoom = this.getMapEl().zoom;
     this._parentEl =
-      this.parentNode.nodeName.toUpperCase() === 'MAP-LAYER' ||
-      this.parentNode.nodeName.toUpperCase() === 'LAYER-' ||
-      this.parentNode.nodeName.toUpperCase() === 'MAP-LINK'
+      this.parentNode.nodeName === 'MAP-LAYER' ||
+      this.parentNode.nodeName === 'LAYER-' ||
+      this.parentNode.nodeName === 'MAP-LINK'
         ? this.parentNode
         : this.parentNode.host;
     if (
       this.getLayerEl().hasAttribute('data-moving') ||
       this._parentEl.parentElement?.hasAttribute('data-moving')
     )
       return;
-    if (this._parentEl.nodeName === 'MAP-LINK') {
+    if (
+      this._parentEl.nodeName === 'MAP-LAYER' ||
+      this._parentEl.nodeName === 'LAYER-' ||
+      this._parentEl.nodeName === 'MAP-LINK'
+    ) {
       this._createOrGetFeatureLayer();
     }
     // use observer to monitor the changes in mapFeature's subtree
@@ -215,6 +223,17 @@ export class HTMLFeatureElement extends HTMLElement {
     this._observer.disconnect();
     if (this._featureLayer) {
       this.removeFeature(this._featureLayer);
+      // If this was the last feature in the layer, clean up the layer
+      if (this._featureLayer.getLayers().length === 0) {
+        if (this._featureLayer.options.renderer) {
+          // since this is the last reference to the MapFeatureLayer, we need to also
+          // manually remove the shared renderer
+          this._featureLayer.options.renderer.remove();
+        }
+        this._featureLayer.remove();
+        this._featureLayer = null;
+        delete this._featureLayer;
+      }
     }
   }
 
@@ -292,65 +311,80 @@ export class HTMLFeatureElement extends HTMLElement {
     return this.previousElementSibling;
   }
   _createOrGetFeatureLayer() {
-    if (this.isFirst() && this._parentEl._templatedLayer) {
-      const parentElement = this._parentEl;
-
-      let map = parentElement.getMapEl()._map;
-
-      // Create a new FeatureLayer
-      this._featureLayer = featureLayer(null, {
-        // pass the vector layer a renderer of its own, otherwise leaflet
-        // puts everything into the overlayPane
-        renderer: featureRenderer(),
-        // pass the vector layer the container for the parent into which
-        // it will append its own container for rendering into
-        pane: parentElement._templatedLayer.getContainer(),
-        // the bounds will be static, fixed, constant for the lifetime of the layer
-        layerBounds: parentElement.getBounds(),
-        zoomBounds: this._getZoomBounds(),
-        projection: map.options.projection,
-        mapEl: parentElement.getMapEl(),
-        onEachFeature: function (properties, geometry) {
-          if (properties) {
-            const popupOptions = {
-              autoClose: false,
-              autoPan: true,
-              maxHeight: map.getSize().y * 0.5 - 50,
-              maxWidth: map.getSize().x * 0.7,
-              minWidth: 165
-            };
-            var c = document.createElement('div');
-            c.classList.add('mapml-popup-content');
-            c.insertAdjacentHTML('afterbegin', properties.innerHTML);
-            geometry.bindPopup(c, popupOptions);
+    // Wait for parent layer to be ready before proceeding
+    this._parentEl
+      .whenReady()
+      .then(() => {
+        // Detect parent context and get the appropriate layer container
+        const isMapLink = this._parentEl.nodeName === 'MAP-LINK';
+        const parentLayer = isMapLink
+          ? this._parentEl._templatedLayer
+          : this._parentEl._layer;
+
+        if (this.isFirst() && parentLayer) {
+          const parentElement = this._parentEl;
+
+          let map = parentElement.getMapEl()._map;
+
+          this._featureLayer = new MapFeatureLayer(null, {
+            // pass the vector layer a renderer of its own, otherwise leaflet
+            // puts everything into the overlayPane
+            // with this feature creating its own MapFeatureLayer for each
+            // sub-sequence of features, it means that there may be > 1 <svg>
+            // container (one per renderer) in the pane...
+            renderer: featureRenderer(),
+            // pass the vector layer the container for the parent into which
+            // it will append its own container for rendering into
+            pane: parentLayer.getContainer(),
+            // the bounds will be static, fixed, constant for the lifetime of a (templated) layer
+            ...(isMapLink && parentElement.getBounds()
+              ? { layerBounds: parentElement.getBounds() }
+              : {}),
+            ...(isMapLink ? { zoomBounds: this._getZoomBounds() } : {}),
+            ...(isMapLink ? {} : { _leafletLayer: parentElement._layer }),
+            zIndex: this.position,
+            projection: map.options.projection,
+            mapEl: parentElement.getMapEl(),
+            onEachFeature: function (properties, geometry) {
+              if (properties) {
+                const popupOptions = {
+                  autoClose: false,
+                  autoPan: true,
+                  maxHeight: map.getSize().y * 0.5 - 50,
+                  maxWidth: map.getSize().x * 0.7,
+                  minWidth: 165
+                };
+                var c = document.createElement('div');
+                c.classList.add('mapml-popup-content');
+                c.insertAdjacentHTML('afterbegin', properties.innerHTML);
+                geometry.bindPopup(c, popupOptions);
+              }
+            }
+          });
+          // this is used by DebugOverlay testing "multipleExtents.test.js
+          // but do we really need or want each feature to have the bounds of the
+          // map link?  tbd
+          extend(this._featureLayer.options, {
+            _leafletLayer: Object.assign(this._featureLayer, {
+              _layerEl: this.getLayerEl()
+            })
+          });
+
+          this.addFeature(this._featureLayer);
+
+          // add MapFeatureLayer to appropriate parent layer
+          parentLayer.addLayer(this._featureLayer);
+        } else {
+          // get the previous feature's layer
+          this._featureLayer = this.getPrevious()?._featureLayer;
+          if (this._featureLayer) {
+            this.addFeature(this._featureLayer);
           }
         }
+      })
+      .catch((error) => {
+        console.log('Error waiting for parent layer to be ready:', error);
       });
-      // this is used by DebugOverlay testing "multipleExtents.test.js
-      // but do we really need or want each feature to have the bounds of the
-      // map link?  tbd
-      extend(this._featureLayer.options, {
-        _leafletLayer: Object.assign(this._featureLayer, {
-          _layerEl: this.getLayerEl()
-        })
-      });
-
-      this.addFeature(this._featureLayer);
-
-      // add featureLayer to TemplatedFeaturesOrTilesLayer of the parentElement
-      if (
-        parentElement._templatedLayer &&
-        parentElement._templatedLayer.addLayer
-      ) {
-        parentElement._templatedLayer.addLayer(this._featureLayer);
-      }
-    } else {
-      // get the previous feature's layer
-      this._featureLayer = this.getPrevious()?._featureLayer;
-      if (this._featureLayer) {
-        this.addFeature(this._featureLayer);
-      }
-    }
   }
   _setUpEvents() {
     ['click', 'focus', 'blur', 'keyup', 'keydown'].forEach((name) => {

src/map-link.js

@@ -343,6 +343,7 @@ export class HTMLLinkElement extends HTMLElement {
         ) {
           this.parentExtent._extentLayer.removeLayer(this._templatedLayer);
           delete this._templatedLayer;
+          this.shadowRoot.innerHTML = '';
           this.getLayerEl()._validateDisabled();
         }
         break;
@@ -559,8 +560,7 @@ export class HTMLLinkElement extends HTMLElement {
         zIndex: this.zIndex,
         pane: this.parentExtent._extentLayer.getContainer(),
         linkEl: this,
-        projection: this.mapEl._map.options.projection,
-        renderer: this.mapEl._map.options.renderer
+        projection: this.mapEl._map.options.projection
       }).addTo(this.parentExtent._extentLayer);
     } else if (this.rel === 'query') {
       if (!this.shadowRoot) {