GEOLYTIX / xyz

An open source javascript framework for spatial data and application interfaces.
MIT License
88 stars 26 forks source link

Fix feature properties #1612

Closed RobAndrewHurst closed 1 month ago

RobAndrewHurst commented 1 month ago

Description

The properties are now spread into the OL element and we need to correct the feature properties references.

GitHub Issue

1610

Type of Change

dbauszus-glx commented 1 month ago

I have done a review of all OpenLayers feature getProperties() use cases.

https://github.com/GEOLYTIX/xyz/pull/1612/commits/afe5a35733023bb538608e538ec33607aa0b4194

These should always be stored as const featureProperties with the OL feature as F.

Please double check the cluster label assignment.

This is now working for me but was bugged.

image

RobAndrewHurst commented 1 month ago

@dbauszus-glx We noticed another issues with wkt layers and assigning properties from the featureFormats module.

The params fields were assigning in a weird way with a 2 step shift in the array, which would result in undefined being assigned into the properties, and then spreading them in will make other properties undefined as well.

So if we make 'id' undefined in this part

// Populate featureFields values array with feature property values.
    layer.params.fields?.forEach((field, i) => {

      layer.featureFields[field].values.push(feature[i + 2]);

      properties[field] = feature[i + 2]
    })

It would then spread id:undefined into the ol.feature

// Return feature from geometry with properties.
    return new ol.Feature({
      id: feature.shift(),
      geometry: formatWKT.readGeometry(feature.shift(), {
        dataProjection: 'EPSG:' + layer.srid,
        featureProjection: 'EPSG:' + layer.mapview.srid,
      }),
      ...properties
    })

I have corrected it in this commit : https://github.com/GEOLYTIX/xyz/pull/1612/commits/a86974e81b24d841f38e3fbdb687fac17d24e993

dbauszus-glx commented 1 month ago

@RobAndrewHurst I don't understand the shift change.

I have two fields retailer, and store.

image

This means they are indexed 2 and 3. With 0 being the id and 1 being the geometry.

image

Removing the +2 in shift breaks WKT featureFormats method for me.

image

RobAndrewHurst commented 1 month ago

@RobAndrewHurst I don't understand the shift change.

I have two fields retailer, and store.

image

This means they are indexed 2 and 3. With 0 being the id and 1 being the geometry.

image

Removing the +2 in shift breaks WKT featureFormats method for me.

image

Yeah we noticed that now! We have come up with a fix!

Will document the fix now!

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud