[Maps] Use mapbox feature-state for dynamic properties and upgrade mapbox-gl to 0.54 #36466
Conversation
thomasneirynck
added
WIP
|
Pinging @elastic/kibana-gis |
💔 Build Failed |
💔 Build Failed |
|
jenkins, test this |
thomasneirynck
changed the title
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
| feature.properties[FEATURE_ID_PROPERTY_NAME] = (typeof feature.id === 'string' || typeof feature.id === 'number') ? feature.id : i; | ||
| const id = generateNumericalId(); | ||
| feature.properties[FEATURE_ID_PROPERTY_NAME] = id; | ||
| feature.id = id; |
There was a problem hiding this comment.
This could be a breaking change. What if the vector source populates id field and users expect to see their id value in a tooltip and now they see our implementation value. Maybe not a big deal
There was a problem hiding this comment.
Feature#id is not accessible as a tooltip-value. Only index-pattern properties are accessible, and they are put as-is under Feature#properties
|
jenkins, test this |
1 similar comment
|
jenkins, test this |
|
jenkins, test this |
💔 Build Failed |
|
jenkins, test this |
💔 Build Failed |
|
Looks like test failure is caused by changes. The functional test may need to be reworked because the current test looks for the scaled property in the feature collection and now with feature state this may have changed. I can take a look tomorrow |
💚 Build Succeeded |
|
7.2 got branched before 7.x backport was merged. Backported to 7.2 with #36945 |
Closes #34396
Switching dynamic styling no longer requires registering the data-source with mapbox. This is a great performance improvement, especially for larger data since the geometries no longer need to be processed and moved to the GL-context by mapbox (they are already on the map after all) for each change in the style-expression. Because of this, we can also simplify the style-syncing logic since the maps-app no longer needs to keep track whether it needs to reset the data.
By naively introducing this, the map got a "wash"-effect as new data from ES gets loaded when panning/zooming/querying. That's likely because mapbox is still having the feature-state of the old feature-set registered, when the new style is applied. See comment in code explaining the issue in more detail, and the proposed work-around to resolve this wash effect.
This also required an upgrade of mapbox-gl, as the current dependency did not include the
removeFeatureStatemethod yet.Perhaps #36059 could resolve that issue. That PR will introduce assign a unique ID to features from ES (the ES-docId), so no feature-id will be reused across resultsets.This does not resolve the issue that property selection in dynamic styling causes a full data round-trip if the dynamic property has not been retrieved yet.