mapsense / mapsense.js

Mapsense.js
Other
88 stars 22 forks source link

GeoJson.classify #12

Closed sttawm closed 9 years ago

sttawm commented 9 years ago

I met with Erez and we seemed to agree on the following:

  1. Should be in GeoJson.js, not TopoJson.js
  2. Should return an array of string "classes"
  3. The renderer should assign these as classes on the path (dom) element
  4. Each group should be a feature with a single property, "classes", the array, and a GeometryCollection as the geometry.

For example

Given Three Features...

{
    "type": "FeatureCollection",
    "features": [
        {
            "type": "Feature",
            "properties": {
                "layer": "roads",
                "sub_layer": "tertiary"
            },
            "geometry": {}
        },
        {
            "type": "Feature",
            "properties": {
                "layer": "roads",
                "sub_layer": "primary"
            },
            "geometry": {}
        },
        {
            "type": "Feature",
            "properties": {
                "layer": "roads",
                "sub_layer": "tertiary"
            },
            "geometry": {}
        }
    ]
}

And Classify Function...

geojson.classify(function(feature) {
     return [feature.properties.layer, feature.properties.sub_layer];
});

We'd get two groups (in geojson)

{
    "type": "FeatureCollection",
    "features": [
        {
            "type": "Feature",
            "properties": {
                "classes": [
                    "roads",
                    "tertiary"
                ]
            },
            "geometry": {
                "type": "GeometryCollection",
                "geometries": [
                    {},
                    {}
                ]
            }
        },
        {
            "type": "Feature",
            "properties": {
                "classes": [
                    "roads",
                    "primary"
                ]
            },
            "geometry": {
                "type": "GeometryCollection",
                "geometries": [
                    {}
                ]
            }
        }
    ]
}

... and two svg's

<path class="roads primary" d="..." />
<path class="roads tertiary" d="..." />
sttawm commented 9 years ago

Actually, I believe we decided (previously) to return a string instead of an array. I go back and forth. What do y'all htink?

Either way, it seems there's a bug with the current implementation (or perhaps some downstream rendering logic). @erex78 , can you provide the relevant html+js?