Measure behavior breaks modify interaction under certain circumstances

[mstenta]

A strange bug was encountered with the edit behavior's modify interaction, specifically after the edit layer's features are changed, for example by pasting WKT text into farmOS's data field under the map, which is a new feature provided by this PR: farmOS/farmOS#670

After pasting the WKT, if you click the "Modify" button, then click on a feature in the map, then attempt to drag one of its vertices, the vertex point itself moves, but the shape itself doesn't change. If you release the click, and then try to move the same vertex again, it works as expected.

In testing, @symbioquine @paul121 and myself narrowed it down to the measure behavior that is added by farmOS's WKT behavior here: https://github.com/farmOS/farmOS/blob/2b3bb21683396fc0b5ec0cd2f5107dc4e4e4d16a/modules/core/map/js/farmOS.map.behaviors.wkt.js#L44

Commenting out that line (omitting the measure behavior) fixes the issue. So it seems that something within the measure behavior's logic is causing this.

[mstenta]

Copying some of @symbioquine's thoughts from chat:

I suspect one of the issues is that this code uses farmOS-map "events" instead of watching the vector layer for changes more directly...
I think that means that you change the features without causing a "drawstart", "drawend", "modifystart"; it will be somewhat broken.

https://irc.farmos.org/bot/log/farmOS/2023-05-09#T89925

[symbioquine]

We can reproduce this problem by in farmOS-map directly with the following change to the examples;

--- a/examples/simple-html-consumer/static/index.html
+++ b/examples/simple-html-consumer/static/index.html
@@ -32,6 +32,21 @@
         myMap.addBehavior("measure", { layer: myMap.edit.layer });
         myMap.edit.wktOn("featurechange", console.log);
         myMap.edit.geoJSONOn("featurechange", console.log);
+
+        myMap.edit.addInteractionListener('drawend', () => {
+          console.log("event: ", evt, wkt);
+
+          setTimeout(() => {
+            // Clear features from the layer.
+            myMap.edit.layer.getSource().clear();
+
+            // Read features from WKT and add them to the layer.
+            const features = myMap.readFeatures('wkt', 'POLYGON((-36.93981145757795 58.73126735211832,17.882523567802764 75.9487847403179,15.010877447425676 57.76974206386987,-36.93981145757795 58.73126735211832))');
+            myMap.edit.layer.getSource().addFeatures(features);
+          }, 1000);
+
+        });

2023_05_11_farmOS-map_measure_behaviour_issue.webm

[symbioquine]

There's actually two related issues here;

1. The modify geometry functionality breaks
2. The old measure overlays are not cleaned up when the layer is cleared

2023_05_11_farmOS-map_measure_behaviour_issue_further.webm

I was originally proposing a rewrite of the measure behaviour to fix it more broadly, but I think a more restrained approach makes sense first...

The smallest change we need to do to fix the measure behaviour and the modify geometry bug is;

--- a/src/behavior/measure.js
+++ b/src/behavior/measure.js
@@ -134,6 +134,14 @@ export default {
       createMeasure(feature);
     });
 
+    layer.getSource().on('clear', () => {
+      stopMeasure();
+    });
+
+    layer.getSource().on('addfeature', (evt) => {
+      startMeasure(evt.feature);
+    });
+
     // If the instance has an Edit control, attach listeners to the map
     // interactions so that we can apply measurements to the features.
     if (instance.edit) {

2023_05_11_farmOS-map_measure_behaviour_issue_with_fix.webm

We'll still need to do a bit more serious change if we want measure to work correctly for multiple map instances - involving moving the global state onto the map instance somehow. See #84 and #94