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.

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] Add UI to disable style meta and get top categories from current features #59707

Merged
merged 16 commits into from
Mar 11, 2020

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Mar 9, 2020

This PR provides UI to allow users to turn off style meta and instead get top categories from current features. PR also adds a bunch of types for StyleProperty and DynamicStyleProperty.

Categorical style meta fetches the top 9 terms for styling. When styling on a field with high cardinality, the current view may not contain any features with the top 9 terms. Allowing users to disable field meta will allow them to style by current features on the map.

Screen Shot 2020-03-09 at 3 24 52 PM

@nreese nreese added release_note:enhancement [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.7.0 labels Mar 9, 2020
@nreese nreese requested a review from thomasneirynck March 9, 2020 21:22
@nreese nreese requested a review from a team as a code owner March 9, 2020 21:22
@elasticmachine
Copy link
Contributor

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

@@ -52,6 +53,10 @@ export class DynamicStyleProperty extends AbstractStyleProperty {
const fieldName = this.getFieldName();
const rangeFieldMetaFromLocalFeatures = styleMeta.getRangeFieldMetaDescriptor(fieldName);

if (!this.isFieldMetaEnabled()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes a bug introduced in #58766 where disabling style meta would still use style meta range, instead of range from local features.

Copy link
Contributor

Choose a reason for hiding this comment

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

🙇

@nreese nreese changed the title [Maps] Add UI to disable categorical field meta [Maps] Add UI to disable style meta and get top categories from current features Mar 9, 2020
@@ -52,6 +53,10 @@ export class DynamicStyleProperty extends AbstractStyleProperty {
const fieldName = this.getFieldName();
const rangeFieldMetaFromLocalFeatures = styleMeta.getRangeFieldMetaDescriptor(fieldName);

if (!this.isFieldMetaEnabled()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🙇

supportsFeatureState(): boolean;
pluckOrdinalStyleMetaFromFeatures(features: unknown[]): RangeFieldMeta;
pluckCategoricalStyleMetaFromFeatures(features: unknown[]): CategoryFieldMeta;
pluckOrdinalStyleMetaFromFieldMetaData(fieldMetaData: unknown): RangeFieldMeta;
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth creating types for the ES-responses? (don't feel strongly)

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 don't think this is worthwhile, at least not in this PR

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

thanks. The TS-typing is really starting to click now. lgtm when green.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@nreese nreese merged commit e4c4b7f into elastic:master Mar 11, 2020
nreese added a commit to nreese/kibana that referenced this pull request Mar 11, 2020
…nt features (elastic#59707)

* rough start

* remove unused files

* get everything working

* add style options types

* convert DynmaicStyleProperty to TS

* limit scope of PR

* CategoricalFieldMetaPopover

* update label and move functions to IStyleProperty interface

* fix ts lint errors

* replace unknown with IJoin now that its available

* remove duplicate import

* review feedback
nreese added a commit that referenced this pull request Mar 11, 2020
…nt features (#59707) (#59933)

* rough start

* remove unused files

* get everything working

* add style options types

* convert DynmaicStyleProperty to TS

* limit scope of PR

* CategoricalFieldMetaPopover

* update label and move functions to IStyleProperty interface

* fix ts lint errors

* replace unknown with IJoin now that its available

* remove duplicate import

* review feedback

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants