Fix FeatureLayer.refresh for moved point layers #1304

patrickarlt · 2021-12-18T02:10:27Z

This fixes FeatureLayer.refresh() for layers where simplifyFactor is not set. Originally this was intended to update the geometry when a higher resolution geometry came in from the service but it also means that layers with edited geometries were missed by FeatureLayer.refresh()

Can be reproudced with the following HTML in the debug folder (originally from @gavinr):

<!-- https://esri.github.io/esri-leaflet/examples/simple-editing.html -->
<!-- Forked from https://codepen.io/jwasilgeo/pen/KKXVjvy?editors=1000 -->
<!DOCTYPE html>
<html>

<head>
  <meta charset="utf-8" />
  <title>Editing feature layers</title>
  <meta name="viewport" content="initial-scale=1,maximum-scale=1,user-scalable=no" />

  <!-- Load Leaflet from CDN -->
  <link rel="stylesheet" href="https://unpkg.com/[email protected]/dist/leaflet.css" />
  <script src="https://unpkg.com/[email protected]/dist/leaflet.js"></script>

  <!-- Load Esri Leaflet from source-->
  <script src="../dist/esri-leaflet-debug.js"></script>

  <style>
    body {
      margin: 0;
      padding: 0;
    }

    #map {
      position: absolute;
      top: 150px;
      bottom: 0;
      right: 0;
      left: 0;
    }
  </style>
</head>

<body>

  <!-- Leaflet Draw -->
  <style>
    #info-pane {
      z-index: 400;
      padding: 1em;
      background: white;
      text-align: right;
    }

    #form {
      display: none;
    }
  </style>

  <div id="map"></div>
  <div id="info-pane" class="leaflet-bar" style="font-size: 10px">
    These do not use the Leaflet or Esri Leaflet add/edit funcitonality. They call the REST endpoints using FETCH
    directly:<br />
    <button id="addFeatureButton">ADD a feature within the current extent</button><br />
    <button id="moveFeatureButton">MOVE one of the features within the current extent</button>
    <br /><br />
    This DOES call the leaflet layer.refresh() function:<br />
    <button id="refreshLayerButton">Refresh the layer</button>
  </div>

  <script>
    // create the map
    var map = L.map('map').setView([38.631, -90.199], 8);
    // basemap
    var OpenStreetMap_Mapnik = L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
      maxZoom: 19,
      attribution: '&copy; <a href="https://www.openstreetmap.org/copyright">OpenStreetMap</a> contributors'
    });
    OpenStreetMap_Mapnik.addTo(map);
    // add our feature layer to the map
    const militaryOps = L.esri.featureLayer({
      url: 'https://sampleserver6.arcgisonline.com/arcgis/rest/services/Military/FeatureServer/3'
    }).addTo(map);

    const addFeatureButton = document.querySelector("#addFeatureButton");
    const moveFeatureButton = document.querySelector("#moveFeatureButton");
    const refreshLayerButton = document.querySelector("#refreshLayerButton");
    addFeatureButton.addEventListener("click", () => {
      const latlng = getRandomPointWithinCurrentExtent(map);
      const feat = L.marker(latlng).toGeoJSON();
      // set the attribute value that controls the feature service symbology
      feat.properties.symbolname = "Decision Point";
      fetch(`${militaryOps.options.url}/addFeatures`, {
        "headers": {
          "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8",
        },
        "body": `features=${JSON.stringify([L.esri.Util.geojsonToArcGIS(feat)])}&f=json`,
        "method": "POST",
        "mode": "cors"
      })
        .then(res => res.json())
        .then(res => {
          console.log(res);
          // militaryOps.refresh();
        });
    });

    moveFeatureButton.addEventListener("click", () => {
      console.log('move', map.getPanes()[militaryOps.options.pane]);
      const latlng = getRandomPointWithinCurrentExtent(map);
      console.log("latlng", latlng);

      let feat = {};
      militaryOps.eachActiveFeature((feature) => {
        feat.attributes = {};
        feat.attributes.OBJECTID = feature.feature.id
        feat.geometry = { "spatialReference": { "wkid": 4326 } };

        feat.geometry.x = latlng.lng;
        feat.geometry.y = latlng.lat;
      });

      console.log('sending:', feat);
      // const latlng = getRandomPointWithinCurrentExtent(map);
      // const feat = L.marker(latlng).toGeoJSON();
      // // set the attribute value that controls the feature service symbology
      // feat.properties.symbolname = "Decision Point";
      fetch(`${militaryOps.options.url}/updateFeatures`, {
        "headers": {
          "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8",
        },
        "body": `features=${JSON.stringify([feat])}&f=json`,
        "method": "POST",
        "mode": "cors"
      })
        .then(res => res.json())
        .then(res => {
          console.log(res);
          // militaryOps.refresh();
        });
    });

    refreshLayerButton.addEventListener("click", () => {
      militaryOps.refresh();
    });

    function randomIntFromInterval(min, max) { // min and max included 
      return Math.random() * (max - min) + min;
    }
    const getRandomPointWithinCurrentExtent = (map) => {
      const bounds = map.getBounds();
      var lng = randomIntFromInterval(bounds.getEast(), bounds.getWest()).toFixed(5);
      var lat = randomIntFromInterval(bounds.getSouth(), bounds.getNorth()).toFixed(5);
      return L.latLng(lat, lng);
    }

    // when a marker is clicked, remove the feature from the service, using its id
    militaryOps.on('click', function (e) {
      // militaryOps.deleteFeature(e.layer.feature.id, function(err, response) {
      //   if (err) {
      //     return;
      //   }
      //   console.log(response);
      // });
      fetch(`${militaryOps.options.url}/deleteFeatures`, {
        "headers": {
          "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8",
        },
        "body": `objectIds=${e.layer.feature.id}&f=json`,
        "method": "POST",
        "mode": "cors"
      })
        .then(res => res.json())
        .then(res => {
          console.log(res);
          militaryOps.refresh();
        });
      // make sure map click event isn't fired.
      L.DomEvent.stopPropagation(e);
    });
  </script>

</body>

</html>

PedroVenancio · 2021-12-19T00:21:57Z

Dear @patrickarlt and @jwasilgeo

Thank you very much for this fix! I was testing it and these are my findings after apply the fix:

it works updating geometry position;
it also works updating attributes, both in popup and tooltip;
but it does not update the symbology. In my test case (https://codepen.io/PedroNGV/pen/rNzoQeE) I'm defining the icons symbol

    var testIcons = {
      'em curso': L.icon({
        iconUrl: 'https://esri.github.io/esri-leaflet/img/red-triangle.png',
        iconSize: [20, 20],
        iconAnchor: [6, 6],
        popupAnchor: [-3, -5],
      }),
      'em resolução': L.icon({
        iconUrl: 'https://esri.github.io/esri-leaflet/img/orange-triangle.png',
        iconSize: [20, 20],
        iconAnchor: [6, 6],
        popupAnchor: [-3, -5],
      }),
      'em conclusão': L.icon({
        iconUrl: 'https://esri.github.io/esri-leaflet/img/yellow-triangle.png',
        iconSize: [20, 20],
        iconAnchor: [6, 6],
        popupAnchor: [-3, -5],
      }),
    };

and then the icon is applied based on an attribute value:

function(geojson, latlng) {
        return L.marker(latlng, {
          icon: testIcons[geojson.properties.EstadoAgrupado.toLowerCase()]
        });
      }

The attribute is updated after this fix, but the symbol only changes if I refresh the browser. Sometimes I also see that the symbology changes randomly after some FeatureLayer.refresh() requests, just like it happened with attributes and geometries before the fix. So it seems to me that the refresh to symbology is somehow missing.

Can you check it in your test case?

Thank you very much! Best regards,
Pedro

jwasilgeo · 2021-12-19T19:50:48Z

Related to https://community.esri.com/t5/esri-leaflet-questions/esri-leaflet-refresh-method-not-working/m-p/1117315

jwasilgeo · 2021-12-19T20:14:20Z

I can reproduce the invalid symbology after an update and refresh. Below is a slightly modified debug case (starting from the debug code at the top here) that switches between 2 different attributes and symbols during the update operation.

debug code

  <!-- https://esri.github.io/esri-leaflet/examples/simple-editing.html -->
  <!-- Forked from https://codepen.io/jwasilgeo/pen/KKXVjvy?editors=1000 -->
  <!DOCTYPE html>
  <html>

  <head>
    <meta charset="utf-8" />
    <title>Editing feature layers</title>
    <meta name="viewport" content="initial-scale=1,maximum-scale=1,user-scalable=no" />

    <!-- Load Leaflet from CDN -->
    <link rel="stylesheet" href="https://unpkg.com/[email protected]/dist/leaflet.css" />
    <script src="https://unpkg.com/[email protected]/dist/leaflet.js"></script>

    <!-- Load Esri Leaflet from source-->
    <script src="../dist/esri-leaflet-debug.js"></script>

    <style>
      body {
        margin: 0;
        padding: 0;
      }

      #map {
        position: absolute;
        top: 150px;
        bottom: 0;
        right: 0;
        left: 0;
      }
    </style>
  </head>

  <body>

    <!-- Leaflet Draw -->
    <style>
      #info-pane {
        z-index: 400;
        padding: 1em;
        background: white;
        text-align: right;
      }

      #form {
        display: none;
      }
    </style>

    <div id="map"></div>
    <div id="info-pane" class="leaflet-bar" style="font-size: 10px">
      These do not use the Leaflet or Esri Leaflet add/edit funcitonality. They call the REST endpoints using FETCH
      directly:<br />
      <button id="addFeatureButton">ADD a feature within the current extent</button><br />
      <button id="moveFeatureButton">MOVE one of the features within the current extent</button>
      <br /><br />
      This DOES call the leaflet layer.refresh() function:<br />
      <button id="refreshLayerButton">Refresh the layer</button>
    </div>

    <script>
      var testIcons = {
        'decision point': L.icon({
          iconUrl: 'https://esri.github.io/esri-leaflet/img/red-triangle.png',
          iconSize: [20, 20],
          iconAnchor: [6, 6],
          popupAnchor: [-3, -5],
        }),
        'air control point': L.icon({
          iconUrl: 'https://esri.github.io/esri-leaflet/img/orange-triangle.png',
          iconSize: [20, 20],
          iconAnchor: [6, 6],
          popupAnchor: [-3, -5],
        }),
        'unmanned aerial system (uas ua)': L.icon({
          iconUrl: 'https://esri.github.io/esri-leaflet/img/yellow-triangle.png',
          iconSize: [20, 20],
          iconAnchor: [6, 6],
          popupAnchor: [-3, -5],
        }),
      };

      // create the map
      var map = L.map('map').setView([38.631, -90.199], 8);
      // basemap
      var OpenStreetMap_Mapnik = L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
        maxZoom: 19,
        attribution: '&copy; <a href="https://www.openstreetmap.org/copyright">OpenStreetMap</a> contributors'
      });
      OpenStreetMap_Mapnik.addTo(map);
      // add our feature layer to the map
      const militaryOps = L.esri.featureLayer({
        url: 'https://sampleserver6.arcgisonline.com/arcgis/rest/services/Military/FeatureServer/3',
        pointToLayer: function (geojson, latlng) {
          return L.marker(latlng, {
            icon: testIcons[geojson.properties.symbolname.toLowerCase()]
          });
        }
      }).addTo(map);

      const addFeatureButton = document.querySelector("#addFeatureButton");
      const moveFeatureButton = document.querySelector("#moveFeatureButton");
      const refreshLayerButton = document.querySelector("#refreshLayerButton");
      addFeatureButton.addEventListener("click", () => {
        const latlng = getRandomPointWithinCurrentExtent(map);
        const feat = L.marker(latlng).toGeoJSON();
        // set the attribute value that controls the feature service symbology
        feat.properties.symbolname = "Decision Point";
        fetch(`${militaryOps.options.url}/addFeatures`, {
          "headers": {
            "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8",
          },
          "body": `features=${JSON.stringify([L.esri.Util.geojsonToArcGIS(feat)])}&f=json`,
          "method": "POST",
          "mode": "cors"
        })
          .then(res => res.json())
          .then(res => {
            console.log(res);
            // militaryOps.refresh();
          });
      });

      moveFeatureButton.addEventListener("click", () => {
        console.log('move', map.getPanes()[militaryOps.options.pane]);
        const latlng = getRandomPointWithinCurrentExtent(map);
        console.log("latlng", latlng);

        let feat = {};
        militaryOps.eachActiveFeature((feature) => {
          feat.attributes = {};
          feat.attributes.OBJECTID = feature.feature.id;
          feat.attributes.symbolname = feature.feature.properties.symbolname === "decision point" ? "air control point" : "decision point"
          feat.geometry = { "spatialReference": { "wkid": 4326 } };

          feat.geometry.x = latlng.lng;
          feat.geometry.y = latlng.lat;
        });

        console.log('sending:', feat);
        // const latlng = getRandomPointWithinCurrentExtent(map);
        // const feat = L.marker(latlng).toGeoJSON();
        // // set the attribute value that controls the feature service symbology
        // feat.properties.symbolname = "Decision Point";
        fetch(`${militaryOps.options.url}/updateFeatures`, {
          "headers": {
            "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8",
          },
          "body": `features=${JSON.stringify([feat])}&f=json`,
          "method": "POST",
          "mode": "cors"
        })
          .then(res => res.json())
          .then(res => {
            console.log(res);
            // militaryOps.refresh();
          });
      });

      refreshLayerButton.addEventListener("click", () => {
        militaryOps.refresh();
      });

      function randomIntFromInterval(min, max) { // min and max included 
        return Math.random() * (max - min) + min;
      }
      const getRandomPointWithinCurrentExtent = (map) => {
        const bounds = map.getBounds();
        var lng = randomIntFromInterval(bounds.getEast(), bounds.getWest()).toFixed(5);
        var lat = randomIntFromInterval(bounds.getSouth(), bounds.getNorth()).toFixed(5);
        return L.latLng(lat, lng);
      }

      // when a marker is clicked, remove the feature from the service, using its id
      militaryOps.on('click', function (e) {
        // militaryOps.deleteFeature(e.layer.feature.id, function(err, response) {
        //   if (err) {
        //     return;
        //   }
        //   console.log(response);
        // });
        fetch(`${militaryOps.options.url}/deleteFeatures`, {
          "headers": {
            "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8",
          },
          "body": `objectIds=${e.layer.feature.id}&f=json`,
          "method": "POST",
          "mode": "cors"
        })
          .then(res => res.json())
          .then(res => {
            console.log(res);
            militaryOps.refresh();
          });
        // make sure map click event isn't fired.
        L.DomEvent.stopPropagation(e);
      });
    </script>

  </body>

  </html>

…dated

jwasilgeo · 2021-12-19T20:24:00Z

@patrickarlt, your thoughts on commit ef8579b, if that's the right spot--timing wise--to add a call to redraw()?

PedroVenancio · 2021-12-20T00:20:06Z

@jwasilgeo It seems to work also in my debug case! Thank you very much!

jwasilgeo · 2021-12-20T19:35:02Z

@PedroVenancio I'm glad to hear it is working for you in your case.
We decided for now not to merge this PR as we need more time to consider the consequences of these changes (that in some cases this would lead to inefficient code and re-rendering on the map, especially with the added call to redraw()), and then plan how we will improve upon it when using a FeatureServer layer that has editing enabled.

For your situation I recommend that your options are either:

Use the full set of proposed changes in this PR yourself, or
Use the original proposed changes in commit 836caa2 and also manually call yourLayer.redraw(yourFeatureUniqueID) after the refresh, if you know the ID of the feature that gets updated.
```
yourLayer.refresh();
yourLayer.redraw(90210);
```

PedroVenancio · 2021-12-23T10:08:07Z

Hi @jwasilgeo

I've implemented your first proposal for now. If I see it causes some impact, I will change for the second proposal, but I hope you can check the impact and merge this (or a polished) PR in the meanwhile.

Thank you very much!

patrickarlt · 2022-01-13T03:21:17Z

@PedroVenancio @jwasilgeo @gavinr I think we should merge this as is with ef8579b. There might be some performance hit because we will be updating features all the time but I think that is actually the proper behavior.

PedroVenancio · 2022-01-13T09:46:34Z

@patrickarlt @jwasilgeo @gavinr Thank you very much for the fix.

I must say that I've implemented 836caa2 and ef8579b in my applications and I'm not seeing any performance hit. Maybe the impact can be greater when editing features, but I also think this is the proper behaviour.

Thanks again for your help!

gavinr

Looks good!

* fix layer.refresh for moved point layers * added call to `redraw` to update symbol/style after geometries are updated * also redraw features * revert Co-authored-by: jwasilgeo <[email protected]>

fix layer.refresh for moved point layers

836caa2

added call to redraw to update symbol/style after geometries are up…

ef8579b

…dated

patrickarlt added 2 commits January 12, 2022 13:09

also redraw features

b626bc4

revert

d0426cb

gavinr approved these changes Feb 14, 2022

View reviewed changes

gavinr merged commit ead4338 into master Feb 14, 2022

gavinr deleted the feature-layer-refresh-repoduction branch February 14, 2022 20:28

gavinr mentioned this pull request Sep 9, 2022

featureLayer rendering performance #1340

Closed

patrickarlt mentioned this pull request Dec 5, 2022

Fix performance issue with refreshing features #1346

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix FeatureLayer.refresh for moved point layers #1304

Fix FeatureLayer.refresh for moved point layers #1304

patrickarlt commented Dec 18, 2021 •

edited

Loading

PedroVenancio commented Dec 19, 2021

jwasilgeo commented Dec 19, 2021 •

edited

Loading

jwasilgeo commented Dec 19, 2021 •

edited

Loading

jwasilgeo commented Dec 19, 2021

PedroVenancio commented Dec 20, 2021

jwasilgeo commented Dec 20, 2021

PedroVenancio commented Dec 23, 2021

patrickarlt commented Jan 13, 2022

PedroVenancio commented Jan 13, 2022

gavinr left a comment

Fix FeatureLayer.refresh for moved point layers #1304

Fix FeatureLayer.refresh for moved point layers #1304

Conversation

patrickarlt commented Dec 18, 2021 • edited Loading

PedroVenancio commented Dec 19, 2021

jwasilgeo commented Dec 19, 2021 • edited Loading

jwasilgeo commented Dec 19, 2021 • edited Loading

jwasilgeo commented Dec 19, 2021

PedroVenancio commented Dec 20, 2021

jwasilgeo commented Dec 20, 2021

PedroVenancio commented Dec 23, 2021

patrickarlt commented Jan 13, 2022

PedroVenancio commented Jan 13, 2022

gavinr left a comment

Choose a reason for hiding this comment

patrickarlt commented Dec 18, 2021 •

edited

Loading

jwasilgeo commented Dec 19, 2021 •

edited

Loading

jwasilgeo commented Dec 19, 2021 •

edited

Loading