Skip to content

[Maps] style icons by category#55747

Merged
nreese merged 24 commits intoelastic:masterfrom
nreese:dynamic_icons
Jan 29, 2020
Merged

[Maps] style icons by category#55747
nreese merged 24 commits intoelastic:masterfrom
nreese:dynamic_icons

Conversation

@nreese
Copy link
Contributor

@nreese nreese commented Jan 23, 2020

Fixes #55204

This PR updates icon styling to allow by value styling that will assign different icons to each category

Screen Shot 2020-01-22 at 4 58 12 PM

To support data driven styling, this PR migrates old style descriptor

 symbol: {
  options: {
    symbolizeAs: 'icon',
    symbolId: 'square',
  },
}

to

symbolizeAs: {
  options: { value: 'icon' },
},
icon: {
  type: STYLE_TYPE.STATIC,
  options: { value: 'square' },
}

This PR also ensures that icon-anchor is properly set for dynamic icons. In the image below, there are 2 layers. The circles are just showing where the point is located. The icons show icons for the points. Notice that the marker icons are above the point while the star icons are centered on the point.

Screen Shot 2020-01-24 at 8 51 31 AM

This PR highlights the problem that each style is displayed independently in legend. For example, in the screen shot below users see both colors and icons in the legend. Fixing this issue is out of scope of this PR

Screen Shot 2020-01-24 at 8 54 54 AM

@nreese nreese added release_note:enhancement Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.7.0 labels Jan 23, 2020
@nreese nreese requested a review from a team as a code owner January 23, 2020 20:25
@elasticmachine
Copy link
Contributor

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

@nreese
Copy link
Contributor Author

nreese commented Jan 23, 2020

@nickpeihl @jsanz

I am having some mapbox problems with this PR. View the screen shots below to view the problem. At certain zoom levels, the icons are displayed all fuzzy with a map box warning: Style sheet warning: Cannot mix SDF and non-SDF icons in one buffer. Change the zoom level appears to fixes everything. Any ideas what is going on or causing this? Solved, passing in icon name with pixel suffix so the icon was not found.

Screen Shot 2020-01-23 at 3 16 51 PM

Screen Shot 2020-01-23 at 3 17 07 PM

@nreese
Copy link
Contributor Author

nreese commented Jan 23, 2020

@nickpeihl @jsanz

I figured out the problem. I was not setting icon pixels for the fallback icon so the icon was missing.

@nreese nreese requested review from nickpeihl and removed request for thomasneirynck January 24, 2020 15:18
@nreese nreese requested a review from spong January 26, 2020 13:35
@nreese nreese requested a review from a team as a code owner January 27, 2020 16:46
Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked out, tested locally and all is good with the SIEM Network Map. LGTM from the SIEM side of things. Thanks for these changes @nreese! 🙂

Copy link
Contributor

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is awesome! lgtm!

I have just one nit. Is it possible to add left padding for the icon in the select box? It runs right up to the edge of the box. Compare how it looks relative to the color selectors below.

Screen Shot 2020-01-28 at 10 54 08 AM

@nreese
Copy link
Contributor Author

nreese commented Jan 28, 2020

Is it possible to add left padding for the icon in the select box? It runs right up to the edge of the box.

That will be fixed once Kibana upgrades to the next EUI version. There was a bug that got fixed yesterday preventing the passing of a className to the component.

elastic/eui#2796

Copy link
Contributor

@jsanz jsanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beautiful, tested in Chromium and Firefox 🆗

image

@jsanz
Copy link
Contributor

jsanz commented Jan 29, 2020

One thing @nreese, should the custom icon set be prepopulated as with #54964?

@elizabetdev elizabetdev self-requested a review January 29, 2020 11:22
@nreese
Copy link
Contributor Author

nreese commented Jan 29, 2020

One thing @nreese, should the custom icon set be prepopulated as with #54964?

For this PR, pre-populating the custom icon set is out of scope. In the long term, yes, we want to provide a way to pre-populate the custom styling maps for the current field. This will effect color ramps and icon ramps. We need to work with design to get a good UI for this work flow.

@nreese
Copy link
Contributor Author

nreese commented Jan 29, 2020

@elasticmachine merge upstream

@nreese nreese merged commit 31e413c into elastic:master Jan 29, 2020
nreese added a commit to nreese/kibana that referenced this pull request Jan 29, 2020
* dynamic icons

* split symbols UI into 2 parts

* static dynamic icon editor UI

* rename style property symbolMarker to icon

* add field select to dynamic icon form

* icon map select component

* create property classes for icon style property

* dynamic icons from palette

* changes

* fix image problem

* implement legend details

* fix image-anchor setting for dynamic images

* update functional test expect because of migration

* fix jest tests

* migrate SIEM style descriptors

* modify IconSelect to show icon in input

* fix jest test

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
nreese added a commit that referenced this pull request Jan 29, 2020
* dynamic icons

* split symbols UI into 2 parts

* static dynamic icon editor UI

* rename style property symbolMarker to icon

* add field select to dynamic icon form

* icon map select component

* create property classes for icon style property

* dynamic icons from palette

* changes

* fix image problem

* implement legend details

* fix image-anchor setting for dynamic images

* update functional test expect because of migration

* fix jest tests

* migrate SIEM style descriptors

* modify IconSelect to show icon in input

* fix jest test

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💔 Build Failed

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:enhancement Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v7.7.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Maps] Dynamic Icons for categorical maps

6 participants

Comments