mapbox-gl-draw Pass userProperties to midpoints and vertices

trafficstars

This PR brings @drewbo PR for this issue (#847) up to speed with master. The aim here is to pass userProperties to midpoints and vertices, and in turn allow them to be styled.

Resolves #846

Feb 12 '20 10:02 JamesLMilner

@asheemmamoowala thanks for the review, much appreciated! What would be the next steps to get this merged? I can see #944 is also in a similar state. Would be amazing to get these two approved PRs merged if possible as they'd be very beneficial for us and the community at large :smile:

Feb 14 '20 12:02 JamesLMilner

@kkaefer thanks for merging the other PR. I saw you merged master into this PR, so thanks for keeping this up to date too. Any update on the steps that need to happen to get this merged? :)

Feb 24 '20 12:02 JamesLMilner

+1

Mar 19 '20 11:03 usb248

Has this PR been put on hold? There has been pull requests for this since 2018 and they always get stuck. Are you waiting for something specific before it can be merged and released? Or is this a feature Mapbox don't want to support?

Apr 09 '20 09:04 awulkan

+1

May 21 '20 15:05 caldwellc

Hi @adrianababakanian, thanks for your follow up, really happy to hear you had a chance to test it locally.

Performance isn't something I had considered as I'd mostly taken @drewbo code and brought it in line with main. I can see why that might be problematic however as it grows.

I am pondering about your suggestion about looking up the parent properties using the ID, and the one thing I'm not sure about is if that would be possible using the Mapbox GL style specification? Maybe an alternative solution is to keep the parent as a reference to the parent object then you could do a match lookup on the parent properties? That might have added knock on effects and added complexities however. I guess I am trying to determine how it would be possible to style the vertices based on the parent properties using only the parent ID, if that makes sense?

Jun 28 '20 16:06 JamesLMilner

@adrianababakanian I experimented with using a parentProperties property on the vertex/midpoints which should allow for you to write a style like this:

{
    id: 'gl-draw-polygon-and-line-midpoint-halo-parent-prop',
    type: 'circle',
    filter: [
        'all',
        ['match', ['geometry-type'], 'Point', true, false],
        ['match', ['get', 'meta'], 'midpoint', true, false],
        ['match', ['get', 'parentProperties', ['get', 'someParentProp'], true, false]
    ],
    paint: {
        'circle-radius': 5,
        'circle-color': '#FFF'
    }
}

You can check it here: https://github.com/mapbox/mapbox-gl-draw/compare/main...JamesLMilner:846-midpoint-user-props-parent-props

Perhaps this would be a more performant approach as we don't copy properties in this instance?

Jun 28 '20 18:06 JamesLMilner

@adrianababakanian I experimented with using a parentProperties property on the vertex/midpoints which should allow for you to write a style like this:
{
    id: 'gl-draw-polygon-and-line-midpoint-halo-parent-prop',
    type: 'circle',
    filter: [
        'all',
        ['match', ['geometry-type'], 'Point', true, false],
        ['match', ['get', 'meta'], 'midpoint', true, false],
        ['match', ['get', 'parentProperties', ['get', 'someParentProp'], true, false]
    ],
    paint: {
        'circle-radius': 5,
        'circle-color': '#FFF'
    }
}
You can check it here: main...JamesLMilner:846-midpoint-user-props-parent-props

Perhaps this would be a more performant approach as we don't copy properties in this instance?

This approach doesn't allow us to do dynamic style. Mapbox GL style specification doesn't allow to get properties in another geojson. It means that you can't get user's properties in a vertex while they are defined in (a parent) feature (linestring) for example.

Copy properties seems the only way

Jul 07 '20 10:07 usb248

Would it be feasible to use Object.create or similar to leverage inheritance, instead of copying?

Jul 07 '20 21:07 Zirak

Thanks for looking into this further @JamesLMilner . I agree that trying to make this work with only a reference to the parent ID adds more complexities than just copying the parent's properties. @usb248 brings up a good point that copying properties would be necessary to support fully dynamic styling.

In my opinion, introducing the Feature State API to GL Draw will be beneficial in the long term. Starting to use feature state will involve a larger refactor of GL Draw as a whole, but will bring many additional benefits beyond just the feature in this PR. I opened https://github.com/mapbox/mapbox-gl-draw/issues/994 to start a discussion around a few of these benefits. Using feature state would enable giving child features (i.e. midpoints and vertices) the same id as the parent feature for styling purposes.

Jul 08 '20 06:07 adrianababakanian

Is it correct to assume this same issue exists with 'Points' also? I'm trying to use: layout: { ... "text-field": ['get', 'name'], ... }, or alternatively: layout: { ... "text-field": "{name}", ... }, But it appears properties.name is not available to the style. Is this a generic issue with Draw, that properties are not available for styling?

Jul 11 '20 22:07 jaybo

One year later... Can we merge this and add a warning in the md that adding a larger number of properties could introduce performance problems, especially with complicated shapes that render many midpoints and vertices? Not being able to style midpoints and vertices is holding us back.