Skip to content

[Maps] use style-prop methods to determine state - #55607

Merged
thomasneirynck merged 3 commits into
elastic:masterfrom
thomasneirynck:maps/use_methods
Jan 23, 2020
Merged

[Maps] use style-prop methods to determine state#55607
thomasneirynck merged 3 commits into
elastic:masterfrom
thomasneirynck:maps/use_methods

Conversation

@thomasneirynck

@thomasneirynck thomasneirynck commented Jan 22, 2020

Copy link
Copy Markdown
Contributor

Ensures that color-ramp UX shows correctly for fill-color UX when adding a grid-layer (style by points/rectangles).

To reproduce:

  1. add grid agg layer (e.g. from kibana_sample_logs)
  2. open layer settings
  3. note that the layer looks correctly on the map. but the fill-color UX shows a color palette iso. a blue ramp.

Seems to be introduced with #55166.

This does not need to be backported to 7.6, as it is only present on 7.x and master.

@thomasneirynck thomasneirynck added Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Jan 22, 2020
@thomasneirynck
thomasneirynck requested a review from a team as a code owner January 22, 2020 20:21
@elasticmachine

elasticmachine commented Jan 22, 2020

Copy link
Copy Markdown
Contributor

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

@thomasneirynck
thomasneirynck requested a review from nreese January 22, 2020 20:22
@nreese

nreese commented Jan 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Can confirm that the problem was introduced by #55166. That PR expects styleDescriptor options.type to be provided but in the case of older map saved objects that is not the case and options.type is undefined..

@nreese

nreese commented Jan 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

options.type is not set in the style descriptor until field is selected. Should options.type be provided a default value so its not undefined until field is selected?

Either way, we should set type here https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js#L247

nreese
nreese approved these changes Jan 23, 2020
View reviewed changes

@nreese nreese left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm
code review, tested in chrome

@kibanamachine

kibanamachine commented Jan 23, 2020

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

History

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

@thomasneirynck
thomasneirynck merged commit 5a15ad1 into elastic:master Jan 23, 2020
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jan 23, 2020
@thomasneirynck
[Maps] use style-prop methods to determine state (elastic#55607)
1031cf8
@thomasneirynck thomasneirynck mentioned this pull request Jan 23, 2020
Merged
thomasneirynck added a commit that referenced this pull request Jan 24, 2020
@thomasneirynck
[Maps] use style-prop methods to determine state (#55607) (#55749)
6943bca
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
@thomasneirynck
[Maps] use style-prop methods to determine state (elastic#55607)
22ac4f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nreese nreese nreese approved these changes

Assignees

No one assigned

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v7.7.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@thomasneirynck @elasticmachine @nreese @kibanamachine