Merge pull request #7243 from TerriaJS/wms-diff-fixes

Fix legend and feature info for wms difference output.

CHANGES.md

@@ -6,6 +6,8 @@
   - getFeaturesFromPickResult now async to handle I3SNode.loadFields()
   - extract common style logic to new Cesium3dTilesStyleMixin.ts
 - Set default value for date and datetime WPS fields only when the field is marked as required.
+- Fix legend shown for WMS difference output item
+- Add `diffItemProperties` trait to override properties of WSM difference output item. Useful for customizing feature info template strings etc.
 - Add Proj4 definition for EPSG:8059
 - Add support for ArcGis ImageServer - this includes
   - Support for "dynamic" `exportImage` endpoint (using `102100` wkid)

lib/ModelMixins/DiffableMixin.ts

@@ -1,17 +1,16 @@
 import { computed, makeObservable, override } from "mobx";
 import JulianDate from "terriajs-cesium/Source/Core/JulianDate";
 import AbstractConstructor from "../Core/AbstractConstructor";
-import createStratumInstance from "../Models/Definition/createStratumInstance";
 import LoadableStratum from "../Models/Definition/LoadableStratum";
 import Model, { BaseModel } from "../Models/Definition/Model";
-import StratumOrder from "../Models/Definition/StratumOrder";
+import createStratumInstance from "../Models/Definition/createStratumInstance";
 import { SelectableDimensionEnum } from "../Models/SelectableDimensions/SelectableDimensions";
 import DiffableTraits from "../Traits/TraitsClasses/DiffableTraits";
 import LegendTraits from "../Traits/TraitsClasses/LegendTraits";
 import MappableMixin from "./MappableMixin";
 import TimeFilterMixin from "./TimeFilterMixin";
 
-class DiffStratum extends LoadableStratum(DiffableTraits) {
+export class DiffStratum extends LoadableStratum(DiffableTraits) {
   static stratumName = "diffStratum";
   constructor(readonly catalogItem: DiffableMixin.Instance) {
     super();
@@ -56,6 +55,22 @@ class DiffStratum extends LoadableStratum(DiffableTraits) {
   get disableDateTimeSelector() {
     return this.catalogItem.isShowingDiff;
   }
+
+  @computed
+  get disableExport() {
+    // disable export if showing diff
+    // currently there is no way to generate export for the difference layer as
+    // it requires 2 time parameters which is not supported in standard WCS
+    return this.catalogItem.isShowingDiff;
+  }
+
+  @computed
+  get disableSplitter() {
+    // disable splitter if showing diff
+    // currently there is no use splitting the difference layer because
+    // most comparable features like style, datetime etc are disabled.
+    return this.catalogItem.isShowingDiff;
+  }
 }
 
 type BaseType = Model<DiffableTraits> & MappableMixin.Instance;
@@ -113,8 +128,6 @@ namespace DiffableMixin {
   export function isMixedInto(model: any): model is Instance {
     return model?.hasDiffableMixin;
   }
-
-  StratumOrder.addLoadStratum(DiffStratum.stratumName);
 }
 
 export default DiffableMixin;

lib/Models/Catalog/Ows/WebMapServiceCapabilitiesStratum.ts

@@ -825,7 +825,7 @@ export default class WebMapServiceCapabilitiesStratum extends LoadableStratum(
   @computed get currentTime() {
     // Get default times for all layers
     const defaultTimes = filterOutUndefined(
-      Array.from(this.capabilitiesLayers).map(([layerName, layer]) => {
+      Array.from(this.capabilitiesLayers).map(([_layerName, layer]) => {
         if (!layer) return;
         const dimensions = this.capabilities.getInheritedValues(
           layer,

lib/Models/Catalog/Ows/WebMapServiceCatalogItem.ts

@@ -16,14 +16,15 @@ import combine from "terriajs-cesium/Source/Core/combine";
 import GetFeatureInfoFormat from "terriajs-cesium/Source/Scene/GetFeatureInfoFormat";
 import WebMapServiceImageryProvider from "terriajs-cesium/Source/Scene/WebMapServiceImageryProvider";
 import URI from "urijs";
+import { JsonObject } from "../../../Core/Json";
 import TerriaError from "../../../Core/TerriaError";
 import createTransformerAllowUndefined from "../../../Core/createTransformerAllowUndefined";
 import filterOutUndefined from "../../../Core/filterOutUndefined";
 import isDefined from "../../../Core/isDefined";
 import CatalogMemberMixin, {
   getName
 } from "../../../ModelMixins/CatalogMemberMixin";
-import DiffableMixin from "../../../ModelMixins/DiffableMixin";
+import DiffableMixin, { DiffStratum } from "../../../ModelMixins/DiffableMixin";
 import ExportWebCoverageServiceMixin from "../../../ModelMixins/ExportWebCoverageServiceMixin";
 import GetCapabilitiesMixin from "../../../ModelMixins/GetCapabilitiesMixin";
 import MappableMixin, {
@@ -99,7 +100,9 @@ export class WebMapServiceUrlStratum extends LoadableStratum(
   }
 }
 
+// Order is important so that the traits are overridden correctly
 StratumOrder.addLoadStratum(WebMapServiceUrlStratum.stratumName);
+StratumOrder.addLoadStratum(DiffStratum.stratumName);
 
 class WebMapServiceCatalogItem
   extends TileErrorHandlerMixin(
@@ -497,8 +500,13 @@ class WebMapServiceCatalogItem
   }
 
   @computed
-  get diffModeParameters() {
-    return { styles: this.diffStyleId };
+  get diffModeParameters(): JsonObject {
+    return this.isShowingDiff ? { styles: this.diffStyleId } : {};
+  }
+
+  @computed
+  get diffModeGetFeatureInfoParameters(): JsonObject {
+    return this.isShowingDiff ? { styles: this.diffStyleId } : {};
   }
 
   getTagForTime(date: JulianDate): string | undefined {
@@ -550,10 +558,6 @@ class WebMapServiceCatalogItem
         ...this.getFeatureInfoParameters
       };
 
-      const diffModeParameters = this.isShowingDiff
-        ? this.diffModeParameters
-        : {};
-
       if (this.supportsColorScaleRange) {
         parameters.COLORSCALERANGE = this.colorScaleRange;
       }
@@ -562,7 +566,11 @@ class WebMapServiceCatalogItem
       parameters.styles = this.styles ?? "";
       getFeatureInfoParameters.styles = this.styles ?? "";
 
-      Object.assign(parameters, diffModeParameters);
+      Object.assign(parameters, this.diffModeParameters);
+      Object.assign(
+        getFeatureInfoParameters,
+        this.diffModeGetFeatureInfoParameters
+      );
 
       // Remove problematic query parameters from URL - these are handled by the parameters objects
 

lib/ReactViews/Tools/DiffTool/DiffTool.tsx

@@ -52,6 +52,7 @@ import Loader from "../../Loader";
 import DatePicker from "./DatePicker";
 import LocationPicker from "./LocationPicker";
 import { CLOSE_TOOL_ID } from "../../Map/MapNavigation/registerMapNavigations";
+import updateModelFromJson from "../../../Models/Definition/updateModelFromJson";
 
 const dateFormat = require("dateformat");
 
@@ -355,7 +356,7 @@ class Main extends React.Component<MainPropsType> {
   }
 
   @action.bound
-  onUserPickingLocation(pickingLocation: LatLonHeight) {
+  onUserPickingLocation(_pickingLocation: LatLonHeight) {
     this._isPickingNewLocation = true;
   }
 
@@ -364,7 +365,7 @@ class Main extends React.Component<MainPropsType> {
     pickedFeatures: PickedFeatures,
     pickedLocation: LatLonHeight
   ) {
-    const { leftItem, rightItem, t } = this.props;
+    const { leftItem, rightItem } = this.props;
     const feature = pickedFeatures.features.find(
       (f) =>
         doesFeatureBelongToItem(f, leftItem) ||
@@ -404,10 +405,17 @@ class Main extends React.Component<MainPropsType> {
     const terria = this.props.terria;
     terria.overlays.remove(this.props.leftItem);
     terria.overlays.remove(this.props.rightItem);
+
     terria.workbench.add(this.diffItem);
 
     this.diffItem.setTrait(CommonStrata.user, "name", this.diffItemName);
     this.diffItem.showDiffImage(this.leftDate, this.rightDate, this.diffStyle);
+
+    // If given, appply additional properties for the diff item
+    const diffItemProperties = this.diffItem.diffItemProperties;
+    if (diffItemProperties) {
+      updateModelFromJson(this.diffItem, CommonStrata.user, diffItemProperties);
+    }
     terria.showSplitter = false;
   }
 
@@ -959,10 +967,8 @@ const LegendImage = function (props: any) {
       {...props}
       // Show the legend only if it loads successfully, so we start out hidden
       style={{ display: "none", marginTop: "4px" }}
-      // @ts-expect-error
-      onLoad={(e) => (e.target.style.display = "block")}
-      // @ts-expect-error
-      onError={(e) => (e.target.style.display = "none")}
+      onLoad={(e) => (e.currentTarget.style.display = "block")}
+      onError={(e) => (e.currentTarget.style.display = "none")}
     />
   );
 };
@@ -993,6 +999,13 @@ async function createSplitItem(
       newItem.setTrait(CommonStrata.user, "opacity", 0);
     }
 
+    // Override feature info template as the parent featureInfoTemplate might
+    // not be relevant for the difference item. This has to be done in the user
+    // stratum to override template set in definition stratum.
+    updateModelFromJson(newItem, CommonStrata.user, {
+      featureInfoTemplate: { template: "" }
+    });
+
     setDefaultDiffStyle(newItem);
 
     // Set the default style to true color style if it exists

lib/Traits/TraitsClasses/DiffableTraits.ts

@@ -1,3 +1,5 @@
+import { JsonObject } from "../../Core/Json";
+import anyTrait from "../Decorators/anyTrait";
 import primitiveArrayTrait from "../Decorators/primitiveArrayTrait";
 import primitiveTrait from "../Decorators/primitiveTrait";
 import mixTraits from "../mixTraits";
@@ -39,4 +41,10 @@ export default class DiffableTraits extends mixTraits(TimeFilterTraits) {
     description: "The ID of the style used to compute the difference image"
   })
   diffStyleId?: string;
+
+  @anyTrait({
+    name: "Difference item properties",
+    description: "Additional properties to set on the difference output item."
+  })
+  diffItemProperties?: JsonObject;
 }

test/ModelMixins/DiffableMixinSpec.ts

@@ -62,17 +62,17 @@ class TestDiffableItem extends DiffableMixin(
   }
 
   showDiffImage(
-    firstDate: JulianDate,
-    secondDate: JulianDate,
-    diffStyleId: string
+    _firstDate: JulianDate,
+    _secondDate: JulianDate,
+    _diffStyleId: string
   ) {}
 
   clearDiffImage() {}
 
   getLegendUrlForStyle(
-    diffStyleId: string,
-    firstDate: JulianDate,
-    secondDate: JulianDate
+    _diffStyleId: string,
+    _firstDate: JulianDate,
+    _secondDate: JulianDate
   ) {
     return "test-legend-url";
   }