fix: use singleclick instead of click to avoid bugs

Author: oterral

cypress/integration/control/modify.spec.js

@@ -48,33 +48,35 @@ describe('ModifyControl', () => {
         .click(102, 152)
         .then(() => {
           // singleclick event needs a timeout period.
-          // cy.wait(400).then(() => {
-          selectedFeaturesArray = win.modify.selectModify
-            .getFeatures()
-            .getArray();
-          // Verify one polygon node was deleted on click (3 nodes, 4 coordinates)
-          expect(
-            selectedFeaturesArray[0].getGeometry().getCoordinates()[0].length,
-          ).to.equal(4);
+          cy.wait(400).then(() => {
+            selectedFeaturesArray = win.modify.selectModify
+              .getFeatures()
+              .getArray();
+            // Verify one polygon node was deleted on click (3 nodes, 4 coordinates)
+            expect(
+              selectedFeaturesArray[0].getGeometry().getCoordinates()[0].length,
+            ).to.equal(4);
+          });
         });
 
       // Click another node (click on map canvas at node pixel)
       cy.get('.ol-viewport')
         .click(100, 100, FORCE)
         .then(() => {
           // singleclick event needs a timeout period.
-          // cy.wait(400).then(() => {
-          // Verify no further node was deleted on click (because polygon minimum number nodes is 3)
-          expect(
-            selectedFeaturesArray[0].getGeometry().getCoordinates()[0].length,
-          ).to.equal(4);
-          // Check that no features from the overlay are mistakenly selected
-          const toTest = omitFeatureSelectSpy.withArgs(
-            omitFeatureSelectSpy.args[0][0],
-            null,
-          );
-          // eslint-disable-next-line no-unused-expressions
-          expect(toTest).to.not.be.called;
+          cy.wait(400).then(() => {
+            // Verify no further node was deleted on click (because polygon minimum number nodes is 3)
+            expect(
+              selectedFeaturesArray[0].getGeometry().getCoordinates()[0].length,
+            ).to.equal(4);
+            // Check that no features from the overlay are mistakenly selected
+            const toTest = omitFeatureSelectSpy.withArgs(
+              omitFeatureSelectSpy.args[0][0],
+              null,
+            );
+            // eslint-disable-next-line no-unused-expressions
+            expect(toTest).to.not.be.called;
+          });
         });
 
       // Select line (double click line in map canvas container to start modifying)
@@ -97,11 +99,12 @@ describe('ModifyControl', () => {
         .click(270, 344, FORCE)
         .then(() => {
           // singleclick event needs a timeout period.
-          // cy.wait(400).then(() => {
-          // Verify one line node was deleted on click (2 nodes, 2 coordinates)
-          expect(
-            selectedFeaturesArray[0].getGeometry().getCoordinates().length,
-          ).to.equal(2);
+          cy.wait(400).then(() => {
+            // Verify one line node was deleted on click (2 nodes, 2 coordinates)
+            expect(
+              selectedFeaturesArray[0].getGeometry().getCoordinates().length,
+            ).to.equal(2);
+          });
         });
 
       // Click another node (click on map canvas at node pixel)

index.html

@@ -14,7 +14,7 @@
     <script src="https://cdn.jsdelivr.net/gh/openlayers/openlayers.github.io/en/v6.4.3/build/ol.js"></script>
     <script src="https://unpkg.com/[email protected]/dist/mapbox-gl.js"></script>
     <script src="https://unpkg.com/[email protected]/dist/jsts.min.js"></script>
-    <script src="build/index.js"></script>
+    <script src="index.js"></script>
     <div id="app">
       <div id="header">
         <nav>

src/control/modify.js

@@ -1,5 +1,5 @@
-import { Modify } from 'ol/interaction';
-import { click } from 'ol/events/condition';
+import { Modify, Interaction } from 'ol/interaction';
+import { singleClick } from 'ol/events/condition';
 import Control from './control';
 import image from '../../img/modify_geometry2.svg';
 import SelectMove from '../interaction/selectmove';
@@ -23,7 +23,7 @@ class ModifyControl extends Control {
    * @param {Object} [options.moveInteractionOptions] Options for the move interaction.
    * @param {Object} [options.modifyInteractionOptions] Options for the modify interaction.
    * @param {Object} [options.deleteInteractionOptions] Options for the delete interaction.
-   *
+   * @param {Object} [options.deselectInteractionOptions] Options for the deselect interaction. Default: features are deselected on click on map.
    */
   constructor(options) {
     super({
@@ -70,15 +70,13 @@ class ModifyControl extends Control {
     this.cursorFilter = options.cursorFilter || (() => true);
     this.cursorHandler = this.cursorHandler.bind(this);
 
-    /* onClickOutsideFeatures management */
-    this.onClickOutsideFeatures = this.onClickOutsideFeatures.bind(this);
-
     /* Interactions */
     this.createSelectMoveInteraction(options.selectMoveOptions);
     this.createSelectModifyInteraction(options.selectModifyOptions);
     this.createModifyInteraction(options.modifyInteractionOptions);
     this.createMoveInteraction(options.moveInteractionOptions);
     this.createDeleteInteraction(options.deleteInteractionOptions);
+    this.createDeselectInteraction(options.deselectInteractionOptions);
   }
 
   /**
@@ -156,6 +154,7 @@ class ModifyControl extends Control {
   /**
    * Create the interaction used to move feature.
    * @param {*} options
+   * @private
    */
   createMoveInteraction(options = {}) {
     /**
@@ -182,6 +181,7 @@ class ModifyControl extends Control {
   /**
    * Create the interaction used to modify vertexes of features.
    * @param {*} options
+   * @private
    */
   createModifyInteraction(options = {}) {
     /**
@@ -190,7 +190,7 @@ class ModifyControl extends Control {
      */
     this.modifyInteraction = new Modify({
       features: this.selectModify.getFeatures(),
-      deleteCondition: click,
+      deleteCondition: singleClick,
       ...options,
     });
 
@@ -209,6 +209,7 @@ class ModifyControl extends Control {
   /**
    * Create the interaction used to delete selected features.
    * @param {*} options
+   * @private
    */
   createDeleteInteraction(options = {}) {
     /**
@@ -223,6 +224,42 @@ class ModifyControl extends Control {
     this.deleteInteraction.setActive(false);
   }
 
+  /**
+   * Create the interaction used to deselected features when we click on the map.
+   * @param {*} options
+   * @private
+   */
+  createDeselectInteraction(options = {}) {
+    // it's important that this condition was the same as the selectModify's
+    // deleteCondition to avoid the selection of the feature under the node to delete.
+    const condition = options.condition || singleClick;
+
+    /**
+     * @type {ol.interaction.Interaction}
+     * @private
+     */
+    this.deselectInteraction = new Interaction({
+      handleEvent: (mapBrowserEvent) => {
+        if (!condition(mapBrowserEvent)) {
+          return true;
+        }
+        const onFeature = this.getFeatureAtPixel(mapBrowserEvent.pixel);
+        const onVertex = this.isHoverVertexFeatureAtPixel(
+          mapBrowserEvent.pixel,
+        );
+
+        if (!onVertex && !onFeature) {
+          // Default: Clear selection on click outside features.
+          this.selectMove.getFeatures().clear();
+          this.selectModify.getFeatures().clear();
+          return false;
+        }
+        return true;
+      },
+    });
+    this.deselectInteraction.setActive(false);
+  }
+
   /**
    * Get a selectable feature at a pixel.
    * @param {*} pixel
@@ -324,36 +361,19 @@ class ModifyControl extends Control {
     }
   }
 
-  /**
-   * Clear selection on map's singleclick event.
-   * @param {*} evt
-   * @private
-   */
-  onClickOutsideFeatures(evt) {
-    if (!this.getActive()) {
-      return;
-    }
-    const onFeature = this.getFeatureAtPixel(evt.pixel);
-    const onVertex = this.isHoverVertexFeatureAtPixel(evt.pixel);
-
-    if (!onVertex && !onFeature) {
-      // Default: Clear selection on click outside features.
-      this.selectMove.getFeatures().clear();
-      this.selectModify.getFeatures().clear();
-    }
-  }
-
   setMap(map) {
     if (this.map) {
       this.map.removeInteraction(this.modifyInteraction);
       this.map.removeInteraction(this.moveInteraction);
       this.map.removeInteraction(this.selectMove);
       this.map.removeInteraction(this.selectModify);
       this.map.removeInteraction(this.deleteInteraction);
+      this.map.removeInteraction(this.deselectInteraction);
       this.removeListeners();
     }
     super.setMap(map);
     this.addListeners();
+    this.map.addInteraction(this.deselectInteraction);
     this.map.addInteraction(this.deleteInteraction);
     this.map.addInteraction(this.selectModify);
     // For the default behvior it's very important to add selectMove after selectModify.
@@ -363,16 +383,23 @@ class ModifyControl extends Control {
     this.map.addInteraction(this.modifyInteraction);
   }
 
+  /**
+   * Add others listeners on the map than interactions.
+   * @param {*} evt
+   * @private
+   */
   addListeners() {
     this.removeListeners();
-    // To avoid bug, it's important that this event is the same as for modify's deleteCondition.
-    this.map.on('click', this.onClickOutsideFeatures);
     this.map.on('pointermove', this.cursorHandler);
   }
 
+  /**
+   * Remove others listeners on the map than interactions.
+   * @param {*} evt
+   * @private
+   */
   removeListeners() {
     clearTimeout(this.cursorTimeout);
-    this.map.un('click', this.onClickOutsideFeatures);
     this.map.un('pointermove', this.cursorHandler);
   }
 
@@ -381,6 +408,7 @@ class ModifyControl extends Control {
    */
   activate() {
     super.activate();
+    this.deselectInteraction.setActive(true);
     this.deleteInteraction.setActive(true);
     this.selectModify.setActive(true);
     // For the default behavior it's very important to add selectMove after selectModify.
@@ -395,6 +423,7 @@ class ModifyControl extends Control {
     clearTimeout(this.cursorTimeout);
     this.selectMove.getFeatures().clear();
     this.selectModify.getFeatures().clear();
+    this.deselectInteraction.setActive(false);
     this.deleteInteraction.setActive(false);
     this.selectModify.setActive(false);
     this.selectMove.setActive(false);

src/interaction/selectmove.js

@@ -1,6 +1,6 @@
 import Select from 'ol/interaction/Select';
 import { Circle, Style, Fill, Stroke } from 'ol/style';
-import { click } from 'ol/events/condition';
+import { singleClick } from 'ol/events/condition';
 
 // Default style on moving geometries
 const selectMoveStyle = new Style({
@@ -34,7 +34,7 @@ class SelectMove extends Select {
    */
   constructor(options) {
     super({
-      condition: click,
+      condition: singleClick,
       style: selectMoveStyle,
       ...options,
     });