Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature id should not be num but dynamic #14

Closed
felix-ht opened this issue Oct 25, 2021 · 6 comments · Fixed by #16
Closed

Feature id should not be num but dynamic #14

felix-ht opened this issue Oct 25, 2021 · 6 comments · Fixed by #16

Comments

@felix-ht
Copy link
Contributor

Feature ids are currently of type num

num get id => jsObject.id;

It would be great if we could change that to be dynamic. As this aligns it with how id is defined for IOS and android.

This is how it is defined in iOS and android
https://docs.mapbox.com/ios/maps/api/6.3.0/Protocols/MGLFeature.html#/c:objc(pl)MGLFeature(py)identifier

geojson-types also specifies it asid?: number | string

https://github.com/mapbox/geojson-types/blob/master/index.js

@andrea689
Copy link
Owner

Hi @felix-ht, I tried to use dynamic, but it seems mapbox-gl.js only accepts numbers.

If I print the id on click, a feature with numeric id is printed, the one with id string is printed null.

@andrea689
Copy link
Owner

you can try this branch: https://github.com/andrea689/mapbox-gl-dart/tree/feature-id-as-dynamic

@felix-ht
Copy link
Contributor Author

felix-ht commented Nov 8, 2021

i checked and it seems that this is an upstream issue:
mapbox/mapbox-gl-js#2716

however adding promote id to the creation of the layer allows promoting an id that is part of the feature.properties - this works with strings as well.

Not strings
https://jsbin.com/gubadidaze/edit?html,console,output

Working with promotion
https://jsbin.com/jurocewunu/edit?html,console,output

So i would suggest to change id to dynamic anyhow. As this would allow to the use of promoteId to have string ids.

@felix-ht
Copy link
Contributor Author

felix-ht commented Nov 8, 2021

promoteId seems work fine with dart as well - https://github.com/Ocell-io/mapbox-gl-dart/tree/test-promoteId - (GeoJsonSource doesn’t contain the promoteId api)

@andrea689
Copy link
Owner

@felix-ht I also added promoteId, thank you!

@felix-ht
Copy link
Contributor Author

felix-ht commented Nov 9, 2021

Thanks for adding this 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants