Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refresh use setWhere #1224

Merged
merged 1 commit into from
Aug 6, 2020
Merged

refresh use setWhere #1224

merged 1 commit into from
Aug 6, 2020

Conversation

gavinr
Copy link
Contributor

@gavinr gavinr commented Jul 28, 2020

trying to address #665, taking advantage of the code in setWhere() - there may be a catch here that I'm missing though, would love your input @patrickarlt @jgravois

sample.html
<!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"
    integrity="sha512-xwE/Az9zrjBIphAcBb3F6JVqxf46+CDLwfLMHloNu6KEQCAWi6HcDUbeOfBIptF7tcCzusKFjFw2yuvEpDL9wQ=="
    crossorigin="" />
  <script src="https://unpkg.com/[email protected]/dist/leaflet.js"
    integrity="sha512-gZwIG9x3wUXg2hdXF6+rVkLF/0Vi9U8D2Ntg4Ga5I5BZpVkVxlJWbSQtXPSiUTtC0TjtGOmxa1AJPuV0CPthew=="
    crossorigin=""></script>

  <script src="../dist/esri-leaflet-debug.js"></script>

  <!-- Load Esri Leaflet Renderers plugin to use feature service symbology -->
  <script src="https://unpkg.com/[email protected]"></script>

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

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

<body>

  <!-- Leaflet Draw -->
  <style>
    #info-pane {
      position: absolute;
      top: 10px;
      right: 10px;
      z-index: 400;
      padding: 1em;
      background: white;
      text-align: right;
      max-width: 250px;
    }

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

  <div id="map"></div>
  <div id="info-pane" class="leaflet-bar">
    <label>
      Click on the map to post an edit.<br>
      Click on a marker to remove it.
      <select id="symbols">
        <option value="Air Control Point">Air Control Point</option>
        <option value="Decision Point">Decision Point</option>
        <option value="Unmanned Aerial System (UAS UA)">Unmanned Aerial System (UAS UA)</option>
      </select>
    </label>

    <button id="refreshLayer">refreshLayer</button><br />
    <button id="deleteAllAndCreate">delete all</button>
  </div>

  <script>
    // create the map
    var map = L.map('map').setView([65, -18.5], 14);
    L.esri.basemapLayer('Physical').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);

    var symbols = document.querySelector('#symbols');

    // when the map is clicked, add a new feature
    map.on('click', function (e) {
      // convert to GeoJSON
      const feat = L.marker(e.latlng).toGeoJSON();
      // set the attribute value that controls the feature service symbology
      feat.properties.symbolname = symbols.value;
      // make a request to add the new feature to the feature service.
      militaryOps.addFeature(feat, function (err, response) {
        if (err) {
          return;
        }

        console.log(response);
      });
    });

    // 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);
      });
      // make sure map click event isn't fired.
      L.DomEvent.stopPropagation(e);
    });

    document.getElementById('refreshLayer').addEventListener('click', (evt) => {
      console.log('REFRESH BUTTON CLICK',);
      militaryOps.refresh();
    });

    document.getElementById('deleteAllAndCreate').addEventListener('click', (evt) => {
      L.esri.post('https://sampleserver6.arcgisonline.com/arcgis/rest/services/Military/FeatureServer/3/deleteFeatures', {
        where: '1=1'
      }, function (error, response) {
        if (error) {
          console.log(error);
        } else {
          console.log(response.name);
        }
      });
    })
  </script>

</body>

</html>

@gavinr gavinr requested a review from patrickarlt July 28, 2020 19:45
@jgravois
Copy link
Contributor

if a popup is opened it will close when the setWhere() is triggered.

is ☝️ still an issue? (ref)

@gavinr
Copy link
Contributor Author

gavinr commented Jul 29, 2020

Looks like it does not close the popup:
http://somup.com/cYiUDP6okM

... which is preferable according to the original issue.

In the example in the recording (where the feature was removed), however, it would probably be preferable to close it. The preference on what you'd want it to do depends on the specific edits that have happened between the original layer load and the refresh I guess.

@jgravois
Copy link
Contributor

awesome. great job tackling an old chestnut. 🔩

depends on the specific edits that have happened between the original layer load and the refresh I guess.

agreed. i wouldn't expect the library to be clever enough to close the popup because the feature was removed upstream.

@gavinr gavinr requested a review from jwasilgeo July 31, 2020 14:49
@jwasilgeo
Copy link
Contributor

I like the way these changes are headed in relying on setWhere, but I was able to still get invalid features to show up on the map. The exact repro steps aren't clear yet, but while using your sample code above (thanks for that, btw) with some combo of panning away from the initial extent and/or zooming out, then adding new features here and there like a salt shaker, and finally pressing 'delete' followed by 'refresh' one can occasionally get incorrect fossil features hanging out on the map.

@gavinr
Copy link
Contributor Author

gavinr commented Aug 3, 2020

I think I discovered the issue: the refresh() call does not delete points that have been added since the page load (no panning needed). Replication steps:

  1. Load up my sample.html from above
  2. Add some points by clicking on the map
  3. Click "delete all"
  4. Click "RefreshLayer"
    • Expected: all points disappear from map
    • Actual: Points that where there when the page loaded disappear, but points that have been added since page load do not disappear.

@gavinr
Copy link
Contributor Author

gavinr commented Aug 3, 2020

@jwasilgeo I think we have discovered a separate issue specifically around editing in esri-leaflet and the setWhere functionality.

If you take the editing out of the picture by using this app to do all editing:
https://arcgis.com/home/webmap/viewer.html?webmap=21e58350f6884ca49505a94ec538ccec
(do all editing there and then just "refreshLayer" button in the esri-leaflet debug app to see if the changes are applied)
.... it seems to work correctly for me.

@jwasilgeo
Copy link
Contributor

Thanks for the additional debugging and clear info, @gavinr. I agree with and can verify what you've uncovered.

@gavinr gavinr merged commit 2e32238 into master Aug 6, 2020
@gavinr gavinr deleted the refresh-use-setwhere branch August 6, 2020 13:29
jgravois pushed a commit to jgravois/esri-leaflet that referenced this pull request Apr 23, 2022
jgravois pushed a commit to jgravois/esri-leaflet that referenced this pull request Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants