make the zoom attribute value of mapFeature as a native value, fix the ordering of g elements for zooming in/out and attribute changes

yhy0217 · yhy0217 · commit 2a077ddfc297 · 2023-09-18T14:52:37.000-04:00
diff --git a/src/map-feature.js b/src/map-feature.js
@@ -70,25 +70,25 @@ export class MapFeature extends HTMLElement {
   attributeChangedCallback(name, oldValue, newValue) {
     switch (name) {
       case 'zoom': {
-        this.whenReady().then(() => {
-          if (oldValue !== newValue) {
-            let layer = this._layer,
-              layerEl = layer._layerEl,
-              mapmlvectors = layer._mapmlvectors;
-            // if the vector layer only has static features, should update zoom bounds when zoom attribute is changed
-            if (mapmlvectors?._staticFeature) {
-              this._removeInFeatureList(oldValue);
-              let native = this._getNativeZoomAndCS(layer._content);
-              mapmlvectors.zoomBounds = M.getZoomBounds(
-                layer._content,
-                native.zoom
-              );
+        if (oldValue !== newValue && this._layer) {
+          let layer = this._layer,
+            zoom = newValue,
+            mapmlvectors = layer._mapmlvectors;
+          // if the vector layer only has static features, should update zoom bounds when zoom attribute is changed
+          if (mapmlvectors._staticFeature) {
+            this._removeInFeatureList(oldValue);
+            if (zoom in mapmlvectors._features) {
+              mapmlvectors._features[zoom].push(this._featureGroup);
+            } else {
+              mapmlvectors._features[zoom] = [this._featureGroup];
             }
-            //this._removeFeature();
-            //this._updateFeature();
-            this._reRender();
+            let native = this._getNativeZoomAndCS(layer._content);
+            mapmlvectors.zoomBounds = M.getZoomBounds(
+              layer._content,
+              native.zoom
+            );
           }
-        });
+        }
         break;
       }
     }
@@ -123,8 +123,7 @@ export class MapFeature extends HTMLElement {
           return;
         }
         // re-render feature if there is any observed change
-        this._removeFeature();
-        this._updateFeature();
+        this._reRender();
       }
     });
     this._observer.observe(this, {
@@ -147,6 +146,7 @@ export class MapFeature extends HTMLElement {
     if (this._groupEl.isConnected) {
       let native = this._getNativeZoomAndCS(this._layer._content);
       let placeholder = document.createElement('span');
+      let mapmlvectors = this._layer._mapmlvectors;
       this._groupEl.insertAdjacentElement('beforebegin', placeholder);
 
       this._featureGroup._map.removeLayer(this._featureGroup);
@@ -155,6 +155,9 @@ export class MapFeature extends HTMLElement {
         .addData(this, native.cs, native.zoom)
         .addTo(this._map);
       placeholder.replaceWith(this._featureGroup.options.group);
+      // TODO: getBounds() should dynamically update the layerBounds
+      this._layer._setLayerElExtent();
+      delete this._getFeatureExtent;
     }
   }
 
@@ -199,32 +202,43 @@ export class MapFeature extends HTMLElement {
         ? this.parentNode
         : this.parentNode.host;
 
-    // arrow function is not hoisted, define before use
-    let _attachedToMap = (e) => {
-      this._parentEl.whenReady().then(() => {
-        let parentLayer =
-          this._parentEl.nodeName.toUpperCase() === 'LAYER-'
-            ? this._parentEl
-            : this._parentEl.parentElement || this._parentEl.parentNode.host;
-        this._layer = parentLayer._layer;
-        this._map = this._layer._map;
-        // "synchronize" the event handlers between map-feature and <g>
-        if (!this.querySelector('map-geometry')) return;
-        if (!this._parentEl._layer._mapmlvectors) {
-          // if vector layer has not yet created (i.e. the layer- is not yet rendered on the map / layer is empty)
-          this._layer.once('add', this._setUpEvents, this);
-          return;
-        } else if (!this._featureGroup) {
-          // if the map-feature el or its subtree is updated
-          // this._featureGroup has been free in this._removeFeature()
-          this._updateFeature();
-        } else {
-          this._setUpEvents();
+    this._parentEl.whenReady().then(() => {
+      let parentLayer =
+        this._parentEl.nodeName.toUpperCase() === 'LAYER-'
+          ? this._parentEl
+          : this._parentEl.parentElement || this._parentEl.parentNode.host;
+      this._layer = parentLayer._layer;
+      this._map = this._layer._map;
+      let mapmlvectors = this._layer._mapmlvectors;
+      // "synchronize" the event handlers between map-feature and <g>
+      if (!this.querySelector('map-geometry')) return;
+      if (!this._extentEl) {
+        let native = this._getNativeZoomAndCS(this._layer._content);
+        this._featureGroup = mapmlvectors.addData(this, native.cs, native.zoom);
+        if (parentLayer.checked) {
+          this._featureGroup.addTo(this._map);
         }
-      });
-    };
+        mapmlvectors._layers[this._featureGroup._leaflet_id] =
+          this._featureGroup;
+        if (mapmlvectors._staticFeature && !this._extentEl) {
+          // update zoom bounds of vector layer
+          mapmlvectors.zoomBounds = M.getZoomBounds(
+            this._layer._content,
+            this._getNativeZoomAndCS(this._layer._content).zoom
+          );
+          // todo: dynamically update layer bounds of vector layer
+          mapmlvectors.layerBounds = M.getBounds(this._layer._content);
+          // update map's zoom limit
+          this._map._addZoomLimit(mapmlvectors);
+          L.extend(mapmlvectors.options, mapmlvectors.zoomBounds);
+        }
+      }
 
-    _attachedToMap();
+      if (Object.keys(mapmlvectors._layers).length === 1) {
+        this._layer._setLayerElExtent();
+      }
+      this._setUpEvents();
+    });
   }
 
   _updateFeature() {
@@ -246,7 +260,7 @@ export class MapFeature extends HTMLElement {
       // update layer bounds of vector layer
       mapmlvectors.layerBounds = M.getBounds(this._layer._content);
       // add feature layers to map
-      mapmlvectors._resetFeatures();
+      // mapmlvectors._resetFeatures();
       this._layer._setLayerElExtent();
       // update map's zoom limit
       this._map._addZoomLimit(mapmlvectors);
diff --git a/src/mapml/layers/FeatureLayer.js b/src/mapml/layers/FeatureLayer.js
@@ -77,6 +77,11 @@ export var FeatureLayer = L.FeatureGroup.extend({
     this._map.featureIndex.cleanIndex();
   },
 
+  removeLayer: function (featureGroupLayer) {
+    L.FeatureGroup.prototype.removeLayer.call(this, featureGroupLayer);
+    delete this._layers[featureGroupLayer._leaflet_id];
+  },
+
   getEvents: function () {
     if (this._staticFeature) {
       return {
@@ -164,21 +169,35 @@ export var FeatureLayer = L.FeatureGroup.extend({
   },
 
   _resetFeatures: function () {
-    this.clearLayers();
     // since features are removed and re-added by zoom level, need to clean the feature index before re-adding
     if (this._map) this._map.featureIndex.cleanIndex();
     let map = this._map || this.options._leafletLayer._map;
     if (this._features) {
       for (let zoom in this._features) {
         for (let k = 0; k < this._features[zoom].length; k++) {
-          let feature = this._features[zoom][k],
-            checkRender = feature._checkRender(
+          let featureGroupLayer = this._features[zoom][k],
+            checkRender = featureGroupLayer._checkRender(
               map.getZoom(),
               this.zoomBounds.minZoom,
               this.zoomBounds.maxZoom
             );
-          if (checkRender) {
-            this.addLayer(feature);
+          if (!checkRender) {
+            let placeholder = document.createElement('span');
+            placeholder.id = featureGroupLayer._leaflet_id;
+            featureGroupLayer.defaultOptions.group.insertAdjacentElement(
+              'beforebegin',
+              placeholder
+            );
+            // removing the rendering without removing the feature from the feature list
+            this.removeLayer(featureGroupLayer);
+          } else if (!map.hasLayer(featureGroupLayer)) {
+            this.addLayer(featureGroupLayer);
+            // update the layerbounds
+            let placeholder =
+              featureGroupLayer.defaultOptions.group.parentNode.querySelector(
+                `span[id="${featureGroupLayer._leaflet_id}"]`
+              );
+            placeholder.replaceWith(featureGroupLayer.defaultOptions.group);
           }
         }
       }
diff --git a/test/e2e/core/mapFeature.test.js b/test/e2e/core/mapFeature.test.js
@@ -45,35 +45,33 @@ test.describe('Playwright MapFeature Custom Element Tests', () => {
       let layer = map.querySelector('layer-'),
         mapFeature = layer.querySelector('map-feature');
       mapFeature.setAttribute('zoom', '4');
+      mapFeature.zoomTo();
     });
     await page.waitForTimeout(200);
-    label = await page.$eval(
-      'body > map > div > div > div.leaflet-pane.leaflet-map-pane > div.leaflet-pane.leaflet-overlay-pane > div > div.mapml-vector-container > svg > g > g:nth-child(1)',
-      (g) => g.getAttribute('aria-label')
+    let mapZoom = await page.$eval('body > map', (map) =>
+      map.getAttribute('zoom')
     );
     // expect the associated <g> el to re-render and re-attach to the map
-    expect(label).toEqual('feature with table properties');
+    expect(mapZoom).toEqual('4');
 
     // change <map-coordinates>
     await page.reload();
     await page.waitForTimeout(500);
-    label = await page.$eval(
-      'body > map > div > div > div.leaflet-pane.leaflet-map-pane > div.leaflet-pane.leaflet-overlay-pane > div > div.mapml-vector-container > svg > g > g:nth-child(1)',
-      (g) => g.getAttribute('aria-label')
-    );
-    expect(label).toEqual("feature, role='button'");
-    await page.$eval('body > map', (map) => {
+    let prevExtentBR = await page.$eval('body > map', (map) => {
+      let layer = map.querySelector('layer-'),
+        mapFeature = layer.querySelector('map-feature');
+      return mapFeature.extent.bottomRight.pcrs;
+    });
+
+    let newExtentBR = await page.$eval('body > map', (map) => {
       let layer = map.querySelector('layer-'),
         mapFeature = layer.querySelector('map-feature'),
         mapCoord = mapFeature.querySelector('map-coordinates');
-      mapCoord.innerHTML = '12 11 12 11 12 12 11 12';
+      mapCoord.innerHTML = '12 11 12 11 12 12 11 13';
+      return mapFeature.extent.bottomRight.pcrs;
     });
-    label = await page.$eval(
-      'body > map > div > div > div.leaflet-pane.leaflet-map-pane > div.leaflet-pane.leaflet-overlay-pane > div > div.mapml-vector-container > svg > g > g:nth-child(1)',
-      (g) => g.getAttribute('aria-label')
-    );
     // expect the associated <g> el to re-render and re-attach to the map
-    expect(label).toEqual('feature with table properties');
+    expect(newExtentBR === prevExtentBR).toBe(false);
 
     // remove <map-properties>
     await page.$eval('body > map', (map) => {
@@ -175,8 +173,6 @@ test.describe('Playwright MapFeature Custom Element Tests', () => {
   });
 
   test('Default click method test', async () => {
-    await page.pause();
-
     // click method test
     // <map-feature> with role="button" should have popup opened after click
     const popup = await page.$eval('body > map', (map) => {
