-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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.
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
[Maps] choropleth layer wizard #69699
Conversation
Pinging @elastic/kibana-gis (Team:Geo) |
@thomasneirynck @jsanz Should there be logic to test the first feature in the Elasticsearch index to see if its a line? Lines will require different styling so the line color is styled instead of the fill color. Is this worth the added complexity? |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments on some of the auto-complete for consideration.
x-pack/plugins/maps/public/classes/layers/choropleth_layer_wizard/layer_template.tsx
Show resolved
Hide resolved
x-pack/plugins/maps/public/classes/layers/choropleth_layer_wizard/layer_template.tsx
Show resolved
Hide resolved
...ins/maps/public/classes/layers/choropleth_layer_wizard/create_choropleth_layer_descriptor.ts
Show resolved
Hide resolved
x-pack/plugins/maps/public/classes/layers/choropleth_layer_wizard/layer_template.tsx
Show resolved
Hide resolved
imho it's not necessary. Fwiw, the "easy" fix would maybe just to give the |
This may be a bit confusing to some users, but since the steps on the interface to get back to a fixed color are quite straightforward I think it should be OK. I've tested this with EMS and an index with polygons and works great. My main concern is about the layer name and description, so actually not a big deal 😄
The simplest definition of this feature would be a wizard to configure a join layer that combines data from a geometry source (EMS or elasticsearch documents) and an aggregation from an elasticsearch index in a
Not sure what the best name we could have for this workflow could be, maybe something around these lines? What do you think? cc. @kmartastic |
I do not want to block this PR on copy for the wizard card. I think "choropleth" is good enough until we come up with a better name. "choropleth" may be too specific and does not include lines and points. I prefer "chorpleth" to creating our own term/complex description. I want to avoid language like "join layer that combines data from a geometry source (EMS or elasticsearch documents) and an aggregation from an elasticsearch index in a 1 to many relationship." Phrases like "join" and "1 to many relations" are very technical. |
@nreese What were the alternate names considered for this UI? I don't have a better word at the moment, but agree with Jorge's point. Further, if I know what I'm doing (I know what choropleth means), how do I create a map of my point and line features? Without having enough context. Could we say something like "Common Maps" with subtext "Compare geospatial data using color, size, and symbols." The current text is also quite specific. It's possible I might what a thematic map, not for comparing statistics, but for showing categorical themes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only have a few nits. I love PRs like this...
@kmartastic @jsanz no real preference wrt. wording from my end. Agreed with @nreese that this can always handled in a separate PR. And fwiw, we will need separate PR(s) anyway to update the docs (https://www.elastic.co/guide/en/kibana/current/maps-add-choropleth-layer.html
x-pack/plugins/maps/public/classes/layers/choropleth_layer_wizard/layer_template.tsx
Show resolved
Hide resolved
EMS = 'EMS', | ||
} | ||
|
||
const BOUNDARIES_OPTIONS = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would flip the order and put EMS first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should EMS be the source selected by default?
...ins/maps/public/classes/layers/choropleth_layer_wizard/create_choropleth_layer_descriptor.ts
Show resolved
Hide resolved
There were no other alternative names discussed. The enhancement request just called it choropleth |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
page load asset sizebeta
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! time-to-visualize just got reduced ...
* [Maps] choropleth layer wizard * add boundaries radio group * geo_index_pattern_select * consolidate more logic into geo_index_pattern_select * small clean-up * left geo field and join field * move EuiPanel into render wizard * cleanup * right panel * createEmsChoroplethLayerDescriptor * createEsChoroplethLayerDescriptor * i18n cleanup * tslint * snapshot update * review feedback * review feedback * update snapshot * make EMS default source * tslint Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* [Maps] choropleth layer wizard * add boundaries radio group * geo_index_pattern_select * consolidate more logic into geo_index_pattern_select * small clean-up * left geo field and join field * move EuiPanel into render wizard * cleanup * right panel * createEmsChoroplethLayerDescriptor * createEsChoroplethLayerDescriptor * i18n cleanup * tslint * snapshot update * review feedback * review feedback * update snapshot * make EMS default source * tslint Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
…ata-streams * 'master' of github.com:elastic/kibana: (50 commits) [Logs UI] [Alerting] "Group by" functionality (elastic#68250) [Discover] Deangularize Skip to bottom button (elastic#69811) Implement recursive plugin discovery (elastic#68811) Use ts-expect-error in platform code (elastic#69883) [SIEM][Detection Engine][Lists] Moves getQueryFilter to common folder for use by both front and backend [Ingest Manager][SECURITY SOLUTION] adjust config reassign link and add roundtrip to Reassignment flow (elastic#70208) [Security][Lists] Add API functions and react hooks for value list APIs (elastic#69603) [ILM] Fix bug when clearing priority field (elastic#70154) [Platform][Security] Updates cluster_manager ignorePaths to include security scripts (elastic#70139) [IngestManager] Allow to filter agent by packages (elastic#69731) [code coverage] exclude folders: test_helpers, tests_bundle (elastic#70199) [Metrics UI] UX improvements for saved views (elastic#69910) [APM] docs: unique transaction troubleshooting (elastic#69831) Cross cluster search functional test with minimun privileges assigned to the test_user (elastic#70007) [Maps] choropleth layer wizard (elastic#69699) Make custom errors by extending Error (elastic#69966) [Ingest Manager] Support updated package output structure (elastic#69864) Resolver test coverage (elastic#70246) Async Discover search test (elastic#64388) [ui-shared-deps] include styled-components (elastic#69322) ... # Conflicts: # x-pack/plugins/snapshot_restore/server/types.ts
…bana into alerting/consumer-based-rbac * 'alerting/consumer-based-rbac' of github.com:gmmorris/kibana: (49 commits) [Discover] Deangularize Skip to bottom button (elastic#69811) Implement recursive plugin discovery (elastic#68811) Use ts-expect-error in platform code (elastic#69883) [SIEM][Detection Engine][Lists] Moves getQueryFilter to common folder for use by both front and backend [Ingest Manager][SECURITY SOLUTION] adjust config reassign link and add roundtrip to Reassignment flow (elastic#70208) [Security][Lists] Add API functions and react hooks for value list APIs (elastic#69603) [ILM] Fix bug when clearing priority field (elastic#70154) [Platform][Security] Updates cluster_manager ignorePaths to include security scripts (elastic#70139) [IngestManager] Allow to filter agent by packages (elastic#69731) [code coverage] exclude folders: test_helpers, tests_bundle (elastic#70199) [Metrics UI] UX improvements for saved views (elastic#69910) [APM] docs: unique transaction troubleshooting (elastic#69831) Cross cluster search functional test with minimun privileges assigned to the test_user (elastic#70007) [Maps] choropleth layer wizard (elastic#69699) Make custom errors by extending Error (elastic#69966) [Ingest Manager] Support updated package output structure (elastic#69864) Resolver test coverage (elastic#70246) Async Discover search test (elastic#64388) [ui-shared-deps] include styled-components (elastic#69322) SECURITY-ENDPOINT: add host properties (elastic#70238) ...
* [Maps] choropleth layer wizard * add boundaries radio group * geo_index_pattern_select * consolidate more logic into geo_index_pattern_select * small clean-up * left geo field and join field * move EuiPanel into render wizard * cleanup * right panel * createEmsChoroplethLayerDescriptor * createEsChoroplethLayerDescriptor * i18n cleanup * tslint * snapshot update * review feedback * review feedback * update snapshot * make EMS default source * tslint Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Fixes #69515
This PR adds a layer wizard for creating choropleth layers.
The wizard allows for creating choropleth maps from EMS our Elasticsearch