[Maps] add text halo color and width style properties by nreese · Pull Request #53827 · elastic/kibana

nreese · 2019-12-27T18:21:59Z

This PR adds labelBorderColor and labelBorderWidth style properties allowing users to style text halo color and text halo width.

elasticmachine · 2019-12-27T18:22:01Z

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

jsanz · 2019-12-27T19:39:28Z

Works nicely to me, tested in Chromium and Firefox.

I found a small issue but probably not related to this change but with #52957: if you have a text field set up and then you add or remove a tooltip field the label field setting is removed. I'll create a new issue for this.

kibanamachine · 2019-12-27T19:42:48Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: c9de8dd

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

thomasneirynck

Fun to see that adding new features basically requires no changes to existing code.

Main comment is around how we should define halo-width. I'm not sure if 0-100 pixels works for that, since it's range is dependent on the label-size. Consider re-interpreting that as 0-100%. (this has some additional issues, that you can't have fixed halo-widths when label-size is dynamic).

x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js

x-pack/legacy/plugins/maps/public/layers/styles/vector/components/vector_style_editor.js

x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style_defaults.js

nreese · 2020-01-07T16:33:26Z

@thomasneirynck Nice suggestion to simplify label border width to just small/medium/large and then derive the value from label size. That really simplifies the UI.

nreese · 2020-01-07T16:35:39Z

@thomasneirynck @jsanz I need a second set of eyes on a map box expression. The expression below from

kibana/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/label_border_size_property.js

Line 39 in 0cb97cc

mbMap.setPaintProperty(mbLayerId, 'text-halo-width', [

is not working as intended. The expression is supposed to pull the label size from geojson properties but instead, the coalesce function is just returning the 0. Any ideas why? It looks right to me.

const labelSizeKey = this._labelSizeProperty.getComputedFieldName();
mbMap.setPaintProperty(mbLayerId, 'text-halo-width', [
        'max',
        ['*', ['coalesce', ['get', labelSizeKey], 0], widthRatio],
        1,
      ]);

nreese · 2020-01-10T17:06:12Z

@jsanz thanks for the help with the map box expression. Everything is working now and this PR is ready for re-review

nreese · 2020-01-10T17:06:30Z

@elasticmachine merge upstream

nreese · 2020-01-13T01:14:45Z

@elasticmachine merge upstream

thomasneirynck

I really like the none-small-medium-large. It's a nice simplification.

jsanz

Works as expected on my setup, tested in both Chromium 78/79 and Firefox 71 (Linux). 👍

nreese · 2020-01-13T17:36:47Z

retest

kibanamachine · 2020-01-13T21:10:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4074895

History

💔 Build #19845 failed ed30450
💔 Build #19623 failed 5efd862
💔 Build #19348 failed 66a5571
💔 Build #19293 failed 04ca5bf
💔 Build #18441 failed 0cb97cc

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

* [Maps] add text halo color and width style properties * fix jest test * update for new editor UI * add removed styling * get halo size from label size * fix label border size with dynamic label size * clean up * fix jest test * fix jest test Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* [Maps] add text halo color and width style properties * fix jest test * update for new editor UI * add removed styling * get halo size from label size * fix label border size with dynamic label size * clean up * fix jest test * fix jest test Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* upstream/master: (26 commits) Take page offset into account too (elastic#54567) [APM] Support error.{log,exception}.stacktrace.classname (elastic#54577) Np migration tsvb route validation (elastic#51850) [ML] MML calculator enhancements for multi-metric job wizard (elastic#54573) [SIEM] Fix Inspect query 'request timestamp' value changes when curso… (elastic#54223) Fix chromeless NP apps not using full page width (elastic#54550) Remove extraneous public import to prevent failing Kibana startup (elastic#54676) [Uptime] Temporarily skip flakey tests (elastic#54675) Skip failing uptime tests Create UI for alerting and actions plugin (elastic#48959) [dev/build/sass] build stylesheets for disabled plugins too (elastic#54654) [SIEM] Use bulk actions API when updating or deleting rules (elastic#54521) Support "Deprecated" label in advanced settings (elastic#54539) [Maps] add text halo color and width style properties (elastic#53827) Service Map Data API at Runtime (elastic#54027) [SIEM] Detection Engine Create Rule Design Review #1 (elastic#54442) Skip flaky test [Canvas] Enable Embeddable maps (elastic#53971) [SIEM][Detection Engine] Increases the number or rules you can view on a single page (elastic#54628) uiSettings - use validation field for image field maxSize (elastic#54522) ...

* [Maps] add text halo color and width style properties * fix jest test * update for new editor UI * add removed styling * get halo size from label size * fix label border size with dynamic label size * clean up * fix jest test * fix jest test Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

[Maps] add text halo color and width style properties

c9de8dd

nreese added release_note:enhancement Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.6.0 labels Dec 27, 2019

nreese requested review from jsanz and thomasneirynck December 27, 2019 18:21

nreese requested a review from a team as a code owner December 27, 2019 18:21

nreese added 3 commits December 27, 2019 13:50

fix jest test

948de75

Merge branch 'master' of github.com:elastic/kibana into text-halo

4287d30

update for new editor UI

98f2fd9

thomasneirynck reviewed Jan 6, 2020

View reviewed changes

nreese added 2 commits January 6, 2020 16:36

add removed styling

6119f23

get halo size from label size

0cb97cc

nreese requested a review from thomasneirynck January 7, 2020 16:32

nreese added 2 commits January 10, 2020 09:42

fix label border size with dynamic label size

04ca5bf

clean up

3ea01da

elasticmachine and others added 2 commits January 10, 2020 11:06

Merge branch 'master' into text-halo

66a5571

fix jest test

d98b20f

Merge branch 'master' into text-halo

5efd862

thomasneirynck approved these changes Jan 13, 2020

View reviewed changes

jsanz approved these changes Jan 13, 2020

View reviewed changes

Merge branch 'text-halo' of github.com:nreese/kibana into text-halo

ed30450

nreese added 2 commits January 13, 2020 14:41

Merge branch 'master' of github.com:elastic/kibana into text-halo

412e912

fix jest test

4074895

nreese merged commit e9e44ec into elastic:master Jan 13, 2020

nreese mentioned this pull request Jan 13, 2020

[7.x] [Maps] add text halo color and width style properties (#53827) #54657

Merged

Conversation

nreese commented Dec 27, 2019

Uh oh!

elasticmachine commented Dec 27, 2019

Uh oh!

jsanz commented Dec 27, 2019

Uh oh!

kibanamachine commented Dec 27, 2019

💔 Build Failed

Uh oh!

thomasneirynck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nreese commented Jan 7, 2020

Uh oh!

nreese commented Jan 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nreese commented Jan 10, 2020

Uh oh!

nreese commented Jan 10, 2020

Uh oh!

nreese commented Jan 13, 2020

Uh oh!

thomasneirynck left a comment

Choose a reason for hiding this comment

Uh oh!

jsanz left a comment

Choose a reason for hiding this comment

Uh oh!

nreese commented Jan 13, 2020

Uh oh!

kibanamachine commented Jan 13, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nreese commented Jan 7, 2020 •

edited

Loading