[Maps] show vector tile labels on top #69444

nreese · 2020-06-17T19:54:19Z

This PR adds labelsOnTop setting to LayerDescriptor. When true, label mapbox layers will be moved to the top of the map.

elasticmachine · 2020-06-17T19:54:21Z

Pinging @elastic/kibana-gis (Team:Geo)

jsanz · 2020-06-18T13:48:12Z

The thing with labels on top is that once you see them, you can't unsee them and you compare any other map by this feature.

Still, in my experience, there are users that still want to have the option to have them below their business data, or even not having labels at all. I can imagine the second being outside the scope of this PR, but I wonder if the first can be doable as a basemap layer setting.

nreese · 2020-06-18T14:05:23Z

Still, in my experience, there are users that still want to have the option to have them below their business data, or even not having labels at all. I can imagine the second being outside the scope of this PR, but I wonder if the first can be doable as a basemap layer setting.

This option can be made configurable if that is desired.

nreese · 2020-06-26T14:21:10Z

@elasticmachine merge upstream

nreese · 2020-06-29T03:37:19Z

@jsanz @thomasneirynck I have updated this PR and its ready for review.

Changes:

Fixed sort logic so now sorting works with labels on top
The setting is now configurable in layer settings panel
The setting is turned off by default to ensure maps do not change when users upgrade.

nreese · 2020-06-29T13:14:33Z

@elasticmachine merge upstream

nreese · 2020-06-29T15:38:02Z

@elasticmachine merge upstream

nreese · 2020-07-01T14:57:54Z

@elasticmachine merge upstream

nyurik · 2020-07-02T16:50:18Z

There are two types of labels: "symbol labels" (a horizontal label attached to a point), and "linestring labels" (a label that follows a geometry, e.g. the name of a road). This only becomes apparent at high zooms. In my previous experience, it may make sense to bring only the symbol labels to the top, while leaving the road/river labels under the choropleth selection. On the other hand, in some cases lake labels could also be done as linestrings, and those may need to be looked at. One possible solution -- offer a multi-option choice rather than a checkbox -- all above / all below / only point labels above
We may want to use the same label placing system for both the base map labels as well as the geojson labels, giving the geojson labels higher priority. This way basemap labels would disappear (or possibly be auto-moved?) instead of overlapping.

nreese · 2020-07-02T17:28:37Z

There are two types of labels: "symbol labels" (a horizontal label attached to a point), and "linestring labels" (a label that follows a geometry, e.g. the name of a road)

Is there a way to tell symbol labels and linestring labels apart just by looking at the layer style? When the labels are sorted, there is not any context about which mapbox layers do what. We can isolate labeling layers but I am not sure we can isolate the difference between symbol labels and linestring labels.

nreese · 2020-07-02T17:37:59Z

The linestring labels have the property "symbol-placement": "line" inside the layout section. Does it help?

That does help. Is this a distinction we want to make?

jsanz · 2020-07-02T17:44:50Z

Not really sure. If I understand correctly, @nyurik comment was more in line of having just the place names on the top, so it's not only the linestrings but also other layers like points of interest, waterway names, etc. that would be below.

Looking at the layer list, I understand we would want to push up just the symbol layers starting with place-

I've checked the three styles and they all have this group of lists on the top, so the pattern is consistent for EMS basemaps.

thomasneirynck · 2020-07-02T18:22:28Z

That does help. Is this a distinction we want to make?

that could be an additional improvement, but I would keep this PR to the basics: pull labels to top from the EMS-baselayer.

This feature ("pulling labels to the top") can always grow when we start getting real-world feedback.

There's a lot of outstanding improvements wrt. "pulling labels to top":

different behavior for different kind of labels ([Maps] show vector tile labels on top #69444 (comment))
pulling labels from all layers (not just EMS labels)
having "global" label-context in Maps so labels from different layers don't clutter eachother
...

^ I don't think this PR needs to address nuances like that until we get real-world use.

thomasneirynck

Only some nits.

Great scope of the functionality for a first pass. It unblocks APM, and puts the core in place to grow this if so desired.

Thank you so much for fixing technical debt wrt layer-ordering. Maps now can reliably order any number of layer-reorderings. e.g. consider a client of the MapsEmbeddable replacing the layer-list with an arbitrary re-ordering. This is now functional.

x-pack/plugins/maps/public/connected_components/layer_panel/layer_settings/layer_settings.tsx

x-pack/plugins/maps/public/connected_components/map/mb/sort_layers.test.ts

x-pack/plugins/maps/public/connected_components/layer_panel/layer_settings/layer_settings.tsx

x-pack/plugins/maps/public/connected_components/map/mb/sort_layers.test.ts

thomasneirynck · 2020-07-02T19:24:33Z

x-pack/plugins/maps/public/connected_components/map/mb/sort_layers.ts

+  layerClass: LAYER_CLASS,
+  beneathMbLayerId?: string
+) {
+  const mbLayers = mbMap.getStyle().layers!; // check ordering against mapbox to account for any upstream moves.


will ! force the creation of an empty mbLayer array?

x-pack/plugins/maps/common/descriptor_types/sources.ts

thomasneirynck

nice!

nreese · 2020-07-02T22:40:03Z

@elasticmachine merge upstream

kibanamachine · 2020-07-03T00:23:33Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 843f424

Build metrics

✅ unchanged

History

💔 Build #58513 failed 6692b19
💔 Build #58462 failed 395d554
💛 Build #58436 was flaky 9f5c902
💔 Build #58388 failed 26d8681
💚 Build #57201 succeeded 265b7d86417ad100866d9a82c617776944876c49

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* [Maps] show vector tile labels on top * experiment with new sort algorithm * clean up * remove old sort method * add unit test for sort layer * tslint * clean up * make labelsOnTop configurable * tslint * more tslint * add another test case for single layer move * clarify should messages * fix assert not null operators * review feedback * update snapshot Co-authored-by: Elastic Machine <[email protected]>

* [Maps] show vector tile labels on top * experiment with new sort algorithm * clean up * remove old sort method * add unit test for sort layer * tslint * clean up * make labelsOnTop configurable * tslint * more tslint * add another test case for single layer move * clarify should messages * fix assert not null operators * review feedback * update snapshot Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

* master: (32 commits) [Ingest Pipelines] Load from json (elastic#70297) [Rum Dashbaord] Rum selected service view (elastic#70579) [Uptime] Prevent duplicate requests on load for index status (elastic#70585) [ML] Changing shared module setup function parameters (elastic#70589) [Ingest Manager] Add ability to sort to agent configs and package configs (elastic#70676) [Alerting] document requirements for developing new action types (elastic#69164) Fixed adding an extra space character on selecting alert variable in action text fields (elastic#70028) [Maps] show vector tile labels on top (elastic#69444) chore(NA): upgrade to lodash@4 (elastic#69868) Add Snapshot Restore README with quick-testing steps. (elastic#70494) [EPM] Use higher priority than default templates (elastic#70640) [Maps] Fix cannot select Solid fill-color when removing fields (elastic#70621) [kbn/optimizer] only build specified themes (elastic#70389) Fix saved query modal overlay (elastic#68826) Update component templates list to render empty prompt inside of content container. Show detail panel when deep-linked, even if there are no component templates. (elastic#70633) [Security Solution] Renames the `Investigate in Resolver` Timeline action (elastic#70634) fix 400 error on initial signals search (elastic#70618) [Maps] fix unable to edit heatmap metric (elastic#70606) Update network idle timeout (elastic#70629) [APM] Disable flaky useFetcher test (elastic#70638) ...

* master: (199 commits) [Telemetry] Add documentation about Application Usage (elastic#70624) [Ingest Manager] Improve agent unenrollment with unenroll action (elastic#70031) Handle timeouts on creating templates (elastic#70635) [Lens] Add ability to set colors for y-axis series (elastic#70311) [Uptime] Use elastic charts donut (elastic#70364) [Ingest Manager] Update registry URL to point to snapshot registry (elastic#70687) [Composable template] Create / Edit wizard (elastic#70220) [APM] Optimize services overview (elastic#69648) [Ingest Pipelines] Load from json (elastic#70297) [Rum Dashbaord] Rum selected service view (elastic#70579) [Uptime] Prevent duplicate requests on load for index status (elastic#70585) [ML] Changing shared module setup function parameters (elastic#70589) [Ingest Manager] Add ability to sort to agent configs and package configs (elastic#70676) [Alerting] document requirements for developing new action types (elastic#69164) Fixed adding an extra space character on selecting alert variable in action text fields (elastic#70028) [Maps] show vector tile labels on top (elastic#69444) chore(NA): upgrade to lodash@4 (elastic#69868) Add Snapshot Restore README with quick-testing steps. (elastic#70494) [EPM] Use higher priority than default templates (elastic#70640) [Maps] Fix cannot select Solid fill-color when removing fields (elastic#70621) ...

nreese added release_note:enhancement WIP Work in progress [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.9.0 labels Jun 17, 2020

nreese requested a review from a team as a code owner June 17, 2020 19:54

nreese removed the WIP Work in progress label Jun 29, 2020

thomasneirynck self-requested a review June 29, 2020 21:42

nreese added 10 commits July 2, 2020 11:31

[Maps] show vector tile labels on top

24582f8

experiment with new sort algorithm

a51539e

clean up

2af9ae3

remove old sort method

b5c59c0

add unit test for sort layer

6283461

tslint

777819d

clean up

a84f67c

make labelsOnTop configurable

0ef45b2

tslint

18ec7a2

more tslint

26d8681

nreese requested review from a team as code owners July 2, 2020 17:33

nreese removed request for a team July 2, 2020 17:36

nreese removed Team:Fleet Team label for Observability Data Collection Fleet team Team:APM All issues that need APM UI Team support labels Jul 2, 2020

nreese added 2 commits July 2, 2020 13:04

add another test case for single layer move

1433b12

clarify should messages

9f5c902

thomasneirynck reviewed Jul 2, 2020

View reviewed changes

nreese added 2 commits July 2, 2020 13:49

fix assert not null operators

f76b329

review feedback

395d554

nreese requested a review from thomasneirynck July 2, 2020 19:57

thomasneirynck approved these changes Jul 2, 2020

View reviewed changes

update snapshot

6692b19

Merge branch 'master' into labels_on_top

843f424

nreese merged commit ebcec3a into elastic:master Jul 3, 2020

nreese mentioned this pull request Jul 3, 2020

[7.x] [Maps] show vector tile labels on top (#69444) #70681

Merged

thomasneirynck mentioned this pull request Jul 6, 2020

[Maps] Distinguish between poi-labels and road-labels #70805

Closed

smith mentioned this pull request Jul 6, 2020

[Maps] [APM] Use Maps in APM instead of custom solution #48538

Closed

nreese mentioned this pull request Aug 13, 2020

[Maps] User shouldn't be able to add roadmap again as the top layer(?) when its the base layer in maps #31757

Closed

This was referenced Aug 26, 2020

[Maps] Visibility issue with WMS layers #75997

Closed

[Maps] fix layer sorting issue with hidden layers at map load #76007

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Maps] show vector tile labels on top #69444

[Maps] show vector tile labels on top #69444

nreese commented Jun 17, 2020 •

edited

Loading

elasticmachine commented Jun 17, 2020

jsanz commented Jun 18, 2020

nreese commented Jun 18, 2020

nreese commented Jun 26, 2020

nreese commented Jun 29, 2020

nreese commented Jun 29, 2020

nreese commented Jun 29, 2020

nreese commented Jul 1, 2020

nyurik commented Jul 2, 2020

nreese commented Jul 2, 2020

nreese commented Jul 2, 2020

jsanz commented Jul 2, 2020

thomasneirynck commented Jul 2, 2020 •

edited

Loading

thomasneirynck left a comment •

edited

Loading

thomasneirynck Jul 2, 2020

thomasneirynck left a comment

nreese commented Jul 2, 2020

kibanamachine commented Jul 3, 2020

[Maps] show vector tile labels on top #69444

[Maps] show vector tile labels on top #69444

Conversation

nreese commented Jun 17, 2020 • edited Loading

elasticmachine commented Jun 17, 2020

jsanz commented Jun 18, 2020

nreese commented Jun 18, 2020

nreese commented Jun 26, 2020

nreese commented Jun 29, 2020

nreese commented Jun 29, 2020

nreese commented Jun 29, 2020

nreese commented Jul 1, 2020

nyurik commented Jul 2, 2020

nreese commented Jul 2, 2020

nreese commented Jul 2, 2020

jsanz commented Jul 2, 2020

thomasneirynck commented Jul 2, 2020 • edited Loading

thomasneirynck left a comment • edited Loading

Choose a reason for hiding this comment

thomasneirynck Jul 2, 2020

Choose a reason for hiding this comment

thomasneirynck left a comment

Choose a reason for hiding this comment

nreese commented Jul 2, 2020

kibanamachine commented Jul 3, 2020

💚 Build Succeeded

Build metrics

History

nreese commented Jun 17, 2020 •

edited

Loading

thomasneirynck commented Jul 2, 2020 •

edited

Loading

thomasneirynck left a comment •

edited

Loading