Feature/1526 measurer select feature

[OlofSvahnVbg]

closes #1526

[jacobwod]

The code looks fine and I was about to approve. However, a quick test (compared with Sketch) reveals that the SelectFeaturesDialog does not show the highlight, as Sketch does, making it difficult to see which feature actually gets selected.

Please see the attached video.

Skarminspelning.2024-06-26.kl.10.57.50.mov

[OlofSvahnVbg]

Thanks for the video, will check this

[OlofSvahnVbg]

So a few things: SketchModel has this function:

createHighlightFeature = (feature) => {
    // If no feature (or a feature with no get-geometry) is supplied, we abort.
    if (feature && feature.getGeometry()) {
      // Otherwise we create a new feature...
      const highlightFeature = new Feature({
        geometry: feature.getGeometry().clone(),
      });
      // ...set an id and a highlight-style...
      highlightFeature.setId(this.generateRandomString());
      highlightFeature.setStyle(this.#createHighlightStyle());
      // Finally we return the feature so that we can add it to the map etc.
      return highlightFeature;
    }
  };

This is being used via a model passed down to SelectFeaturesDialog in Sketch.

The problem is that Measurer doesnt have a model. Should we use Sketchmodel in measurer as model to acces this function or what is best practice here?

[Hallbergs]

So a few things: SketchModel has this function:

createHighlightFeature = (feature) => {
    // If no feature (or a feature with no get-geometry) is supplied, we abort.
    if (feature && feature.getGeometry()) {
      // Otherwise we create a new feature...
      const highlightFeature = new Feature({
        geometry: feature.getGeometry().clone(),
      });
      // ...set an id and a highlight-style...
      highlightFeature.setId(this.generateRandomString());
      highlightFeature.setStyle(this.#createHighlightStyle());
      // Finally we return the feature so that we can add it to the map etc.
      return highlightFeature;
    }
  };

This is being used via a model passed down to SelectFeaturesDialog in Sketch.

The problem is that Measurer doesnt have a model. Should we use Sketchmodel in measurer as model to acces this function or what is best practice here?

Maybe the function can be moved to a core model such as the DrawModel? We shouldn't share (plugin-)models between plugins (since measurer can be activated while sketch is not).

[OlofSvahnVbg]

So a few things: SketchModel has this function:

createHighlightFeature = (feature) => {
    // If no feature (or a feature with no get-geometry) is supplied, we abort.
    if (feature && feature.getGeometry()) {
      // Otherwise we create a new feature...
      const highlightFeature = new Feature({
        geometry: feature.getGeometry().clone(),
      });
      // ...set an id and a highlight-style...
      highlightFeature.setId(this.generateRandomString());
      highlightFeature.setStyle(this.#createHighlightStyle());
      // Finally we return the feature so that we can add it to the map etc.
      return highlightFeature;
    }
  };

This is being used via a model passed down to SelectFeaturesDialog in Sketch.
The problem is that Measurer doesnt have a model. Should we use Sketchmodel in measurer as model to acces this function or what is best practice here?

Maybe the function can be moved to a core model such as the DrawModel? We shouldn't share (plugin-)models between plugins (since measurer can be activated while sketch is not).

Sure. That makes sense

[OlofSvahnVbg]

So in DrawModel, we have this prop:

  #publishInformation = ({ subject, payLoad }) => {
    // We utilize the fact that this method runs on important changes, such
    // as feature add/remove. We check if there are any features in the draw
    // layer and save that information in the Public API. This is later
    // read in a handler for "onbeforeunload". See #1403.
    if (this.getAllDrawnFeatures().length > 0) {
      window.hajkPublicApi.dirtyLayers[this.#layerName] = true;
    } else {
      delete window.hajkPublicApi.dirtyLayers[this.#layerName];
    }

    // If no observer has been set-up, or if the subject is missing, we abort
    if (!this.#observer || !subject) {
      return;
    }
    // Otherwise we create the prefixed-subject to send. (The drawModel might have
    // been initiated with a prefix that should be added on all subjects).
    const prefixedSubject = this.#observerPrefix
      ? `${this.#observerPrefix}.${subject}`
      : subject;
    // Then we publish the event!
    this.#observer.publish(prefixedSubject, payLoad);
  };

Why do we need the prefixedSubject?

[Hallbergs]

So in DrawModel, we have this prop:

  #publishInformation = ({ subject, payLoad }) => {
    // We utilize the fact that this method runs on important changes, such
    // as feature add/remove. We check if there are any features in the draw
    // layer and save that information in the Public API. This is later
    // read in a handler for "onbeforeunload". See #1403.
    if (this.getAllDrawnFeatures().length > 0) {
      window.hajkPublicApi.dirtyLayers[this.#layerName] = true;
    } else {
      delete window.hajkPublicApi.dirtyLayers[this.#layerName];
    }

    // If no observer has been set-up, or if the subject is missing, we abort
    if (!this.#observer || !subject) {
      return;
    }
    // Otherwise we create the prefixed-subject to send. (The drawModel might have
    // been initiated with a prefix that should be added on all subjects).
    const prefixedSubject = this.#observerPrefix
      ? `${this.#observerPrefix}.${subject}`
      : subject;
    // Then we publish the event!
    this.#observer.publish(prefixedSubject, payLoad);
  };

Why do we need the prefixedSubject?

If i remember correctly, we use the prefix so that plugins etc. that uses the DrawModel can catch "their" events. For example: the measure plugin might initiate a DrawModel with "measure-draw-model".

[jacobwod]

Seems to work rather nice now. You have one tiny warning to fix, then it's ready to merge. Feel free to add a CHANGELOG row about it too!

[eslint] 
src/plugins/Sketch/models/SketchModel.js
  Line 3:32:  'Style' is defined but never used  no-unused-vars

[OlofSvahnVbg]

Fixed warning and added to changelog