JamesLMilner / terra-draw

A library for drawing on maps that supports Mapbox, MapLibre, Google Maps, OpenLayers and Leaflet out the box
https://terradraw.io
MIT License
447 stars 50 forks source link

Use promoteId and properties.id in mapbox/maplibre adapter #266

Closed ScottEAdams closed 1 week ago

ScottEAdams commented 2 months ago

Is your feature request related to a problem? Please describe.

Maplibre does not support uuid as a featureId out of the box (possibly other libs suffer the same). This causes features to have undefined id which prevents a bunch of things such as feature-state, lookups etc.

Old discussion can be seen here - https://github.com/maplibre/maplibre-gl-js/issues/1043

Describe your proposed idea for the solution to this problem

Add the following to mapbox-gl.adapter.ts render:

            for (let i = 0; i < features.length; i++) {
                const feature = features[i];

                Object.keys(styling).forEach((mode) => {
                    const { properties } = feature;
                    properties.id = feature.id;

Together with the following in _addGeoJSONSource:

        this._map.addSource(id, {
            type: "geojson",
            data: {
                type: "FeatureCollection",
                features: features,
            },
            tolerance: 0,
            promoteId: 'id',
        });

And you will get a valid feature Id:

image
JamesLMilner commented 2 months ago

Thank you for raising this, I will do a bit of reading about this and get back to you shortly 👍🏻

JamesLMilner commented 2 months ago

I've looked into this a little, and wanted to get a better understanding of what your end goal is and what you are trying to achieve? Perhaps I am misunderstanding, but as a general rule it is advisable not to to interact with the layers/sources that Terra Draw creates for you as these are meant to be managed directly by the library (obviously we have no method of preventing this, hence why it is just advice!).

ScottEAdams commented 1 month ago

Sorry for the late response! We are using a custom version of the maplibre adapter so that we can do a bit more with the styling and as such want to use feature-state which requires a valid ID. For example:

                'circle-color': [
                    'case',
                    ['boolean', ['feature-state', 'hover'], false],
                    '#1a639d',
                    '#3f97e0',
                ],
JamesLMilner commented 3 weeks ago

Hey @ScottEAdams sorry about my late response also!

I understand - for people using the out the box MapLibre adapter I don't think they should need the promoted id, but can totally understand your specific use case. This is one of the USPs about Terra Draw, is you can clone the built in adapters and tweak them do do specific things if you need to, without having to fork the whole repository etc!

I will close this soon, as I understand your custom requirements and it sounds like they're being satisfied via your own adapter. I will give a bit of time for anyone else to chime in if they have any thoughts.

What are you working on out of interest?

JamesLMilner commented 1 week ago

Just closing this out as no feedback from anyone else. Still keen to hear what you're work on however!

ScottEAdams commented 13 hours ago

We have a very old leaflet running in prod at https://kartor.eniro.se/ which we have moved to maplibre at https://www.eniro.se/kartor with our own tiles. Eniro are basically the equivalent of yellow pages but in Sweden. You can check out how I have implemented terra-draw by clicking the little layers icon on the right.

image