Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[maps] Lens choropleth chart #126819

Merged
merged 52 commits into from
Mar 17, 2022
Merged

[maps] Lens choropleth chart #126819

merged 52 commits into from
Mar 17, 2022

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Mar 3, 2022 •
edited
Loading

This PR adds MVP "Choropleth" visualization to lens. To limit scope, the following features have been omitted

  • labels
  • adding filters from map
  • opening choropleth visualization in Maps
  • color configuration

Screen Shot 2022-03-09 at 1 44 54 PM

Screen Shot 2022-03-09 at 1 44 43 PM

nreese added 18 commits February 22, 2022 10:22
@nreese nreese added Team:Visualizations Visualization editors, elastic-charts and infrastructure backport:skip This commit does not require backporting and removed reason:enhancement labels Mar 3, 2022
@nreese
Copy link
Contributor Author

nreese commented Mar 3, 2022

@MichaelMarcialis thanks for the lens icon. We are using it in this PR.

We need another icon for suggestions. In the screen shot below notice how the provided icon is too small and does not have the correct colors. Other visualizations render a mini chart but this will not work for maps since the area is too small. We were hoping to put a static icon here.

Screen Shot 2022-03-03 at 12 58 13 PM

@nreese nreese mentioned this pull request Mar 3, 2022
Closed
@flash1293
Copy link
Contributor

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@flash1293
Copy link
Contributor

@elasticmachine merge upstream

@jsanz
Copy link
Member

jsanz commented Mar 15, 2022

Loving the automatic fit to bounds behavior of the new region maps 👏 👏

Peek 2022-03-15 12-16

@flash1293
Copy link
Contributor

lens app lens smokescreen tests should transition from a multi-layer stacked bar to donut chart using suggestions

Seems like this is caused by adding activeData to the dependency list of suggestion calculation. Let's roll back this change and fix it separately, I have to think about this.

@nreese
Copy link
Contributor Author

nreese commented Mar 15, 2022

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Mar 15, 2022

@elasticmachine merge upstream

@flash1293
Copy link
Contributor

@elasticmachine merge upstream

flash1293
flash1293 approved these changes Mar 17, 2022
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

LGTM except for one small thing

x-pack/plugins/maps/public/lens/choropleth_chart/expression_function.ts
},
inputTypes: ['lens_multitable'],
fn(data, args) {
return {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a very recent change but we moved the responsibility of logging the data table to the visualization function. Could you add the same integration as xy chart here ?

kibana/x-pack/plugins/lens/common/expressions/xy_chart/xy_chart.ts

Line 161 in 11451ad

if (handlers?.inspectorAdapters?.tables) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to revert these changes. logDataTable is not exposed to plugins. I created #127995 to track.

@flash1293 and I chatted offline and @flash1293 is ok with merging this PR without logging data table in the visualization function. This can be addressed in #127995

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
maps 759 773 +14

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
lens 345 353 +8
maps 215 219 +4
total +12

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.1MB 1.1MB +17.0B
maps 2.5MB 2.5MB +23.7KB
total +23.8KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
lens 43 42 -1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
maps 66.2KB 68.2KB +2.0KB
Unknown metric groups

API count

id before after diff
lens 397 426 +29
maps 216 220 +4
total +33

async chunk count

id before after diff
maps 6 10 +4

History

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

@nreese
Copy link
Contributor Author

nreese commented Mar 17, 2022

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Mar 17, 2022

@flash1293 Accessing logDataTable has required some changes to lens kibana.json, specifically exposing the folder containing logDataTable as a extraPublicDirs. Let me know if this is correct.

I had to make this change to resolve
Screen Shot 2022-03-17 at 12 05 59 PM

@nreese nreese mentioned this pull request Mar 17, 2022
Closed
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
maps 759 773 +14

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
lens 345 353 +8
maps 215 219 +4
total +12

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.1MB 1.1MB +17.0B
maps 2.5MB 2.5MB +23.7KB
total +23.8KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
lens 43 42 -1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
maps 66.2KB 68.2KB +2.0KB
Unknown metric groups

API count

id before after diff
lens 397 426 +29
maps 216 220 +4
total +33

async chunk count

id before after diff
maps 6 10 +4

History

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

@nreese nreese merged commit e45209b into elastic:main Mar 17, 2022
@nreese nreese mentioned this pull request Mar 31, 2022
Closed
@nreese nreese mentioned this pull request May 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gchaps gchaps gchaps approved these changes

@nickpeihl nickpeihl nickpeihl approved these changes

@flash1293 flash1293 flash1293 approved these changes

@teresaalvarezsoler teresaalvarezsoler Awaiting requested review from teresaalvarezsoler

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.