Skip to content

Conversation

@thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Sep 14, 2020

Summary

Isolates access of mb-property name and mb-proeprty retrieval into single place.

This change has no end-user impact.

#76526 revealed the relationship between how geojson is processed and how the mb-style-rule is generated is not clear.

This simplifies the code, and will make #76526 smaller.

Checklist

Delete any items that are not applicable to this PR.

NA

For maintainers

@thomasneirynck thomasneirynck added chore 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.10.0 labels Sep 14, 2020
@thomasneirynck thomasneirynck requested a review from a team as a code owner September 14, 2020 15:54
@elasticmachine
Copy link
Contributor

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

@thomasneirynck thomasneirynck changed the title [Maps] [Maps] Simplify mb-propertyname and mb-value generation Sep 14, 2020
@thomasneirynck thomasneirynck changed the title [Maps] Simplify mb-propertyname and mb-value generation [Maps] Add DynamicStyleProperty#getMbPropertyName and DynamicStyleProperty#getMbPropertyValue Sep 14, 2020
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

thanks for moving this out into a separate PR. It makes it a lot easier to view these changes in isolation.

const fieldName = this.getFieldName();
const fieldFormatter = this._getFieldFormatter(fieldName);
return fieldFormatter ? fieldFormatter(value) : super.formatField(value);
return fieldFormatter && value !== null ? fieldFormatter(value) : super.formatField(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there a special check for just null? Is this still needed?

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

Thanks for defining a type for RawValue and cleaning up all the types scattered everywhere

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
maps 3.3MB +578.0B 3.3MB

History

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

@thomasneirynck thomasneirynck merged commit 8b001dc into elastic:master Sep 15, 2020
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Sep 15, 2020
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 16, 2020
…s-for-710

* 'master' of github.com:elastic/kibana: (95 commits)
  log request body in new ES client (elastic#77150)
  use `navigateToUrl` to navigate to recent nav links (elastic#77446)
  Move core config service to `kbn/config` package (elastic#76874)
  [UBI] Copy license to /licenses folder (elastic#77563)
  Skip flaky Events Viewer Cypress test
  [Lens] Remove dynamic names in telemetry fields (elastic#76988)
  [Maps] Add DynamicStyleProperty#getMbPropertyName and DynamicStyleProperty#getMbPropertyValue (elastic#77366)
  [Enterprise Search] Add flag to restrict width of layout (elastic#77539)
  [Security Solutions][Cases - Timeline] Fix bug when adding a timeline to a case (elastic#76967)
  [Security Solution][Detections] Integration test for Editing a Rule (elastic#77090)
  [Ingest pipelines] Polish pipeline debugging workflow (elastic#76058)
  [@kbn/utils] Adds missing dependency (elastic#77536)
  Add the Enterprise Search logo to the Overview search result (elastic#77514)
  [Logs UI] [Metrics UI] Remove saved object field mappings (elastic#75482)
  [Security Solution][Exceptions] Exception modal bulk close option only closes alerts generated by same rule (elastic#77402)
  Adds @kbn/utils package (elastic#76518)
  Map layout changes (elastic#77132)
  [Security Solution] [Detections] EQL Rule Creation (elastic#76831)
  Adding test user to maps tests under documents source folder  (elastic#77245)
  Suppress error logs when clients connect over HTTP instead of HTTPS (elastic#77397)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 16, 2020
* master: (55 commits)
  [Grok] Fix missing error message in error toasts (elastic#77499)
  [Alerting] Exempt Alerts pre 7.10 from RBAC on their Action execution until updated (elastic#75563)
  [ML] fix type in apply_influencer_filters_action (elastic#77495)
  [Lens] create reusable component for filters and range aggregation (elastic#77453)
  fix eslint violations
  Collapse alert chart previews by default (elastic#77479)
  [ML] Fixing field caps wrapper endpoint (elastic#77546)
  showing service maps when filte by environment not defined (elastic#77483)
  [Lens] Settings panel redesign and separate settings per y axis (elastic#76373)
  log request body in new ES client (elastic#77150)
  use `navigateToUrl` to navigate to recent nav links (elastic#77446)
  Move core config service to `kbn/config` package (elastic#76874)
  [UBI] Copy license to /licenses folder (elastic#77563)
  Skip flaky Events Viewer Cypress test
  [Lens] Remove dynamic names in telemetry fields (elastic#76988)
  [Maps] Add DynamicStyleProperty#getMbPropertyName and DynamicStyleProperty#getMbPropertyValue (elastic#77366)
  [Enterprise Search] Add flag to restrict width of layout (elastic#77539)
  [Security Solutions][Cases - Timeline] Fix bug when adding a timeline to a case (elastic#76967)
  [Security Solution][Detections] Integration test for Editing a Rule (elastic#77090)
  [Ingest pipelines] Polish pipeline debugging workflow (elastic#76058)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 16, 2020
* master: (54 commits)
  [Grok] Fix missing error message in error toasts (elastic#77499)
  [Alerting] Exempt Alerts pre 7.10 from RBAC on their Action execution until updated (elastic#75563)
  [ML] fix type in apply_influencer_filters_action (elastic#77495)
  [Lens] create reusable component for filters and range aggregation (elastic#77453)
  fix eslint violations
  Collapse alert chart previews by default (elastic#77479)
  [ML] Fixing field caps wrapper endpoint (elastic#77546)
  showing service maps when filte by environment not defined (elastic#77483)
  [Lens] Settings panel redesign and separate settings per y axis (elastic#76373)
  log request body in new ES client (elastic#77150)
  use `navigateToUrl` to navigate to recent nav links (elastic#77446)
  Move core config service to `kbn/config` package (elastic#76874)
  [UBI] Copy license to /licenses folder (elastic#77563)
  Skip flaky Events Viewer Cypress test
  [Lens] Remove dynamic names in telemetry fields (elastic#76988)
  [Maps] Add DynamicStyleProperty#getMbPropertyName and DynamicStyleProperty#getMbPropertyValue (elastic#77366)
  [Enterprise Search] Add flag to restrict width of layout (elastic#77539)
  [Security Solutions][Cases - Timeline] Fix bug when adding a timeline to a case (elastic#76967)
  [Security Solution][Detections] Integration test for Editing a Rule (elastic#77090)
  [Ingest pipelines] Polish pipeline debugging workflow (elastic#76058)
  ...
thomasneirynck added a commit that referenced this pull request Sep 16, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 16, 2020
* master: (76 commits)
  Fixing service maps API test (elastic#77586)
  [Grok] Fix missing error message in error toasts (elastic#77499)
  [Alerting] Exempt Alerts pre 7.10 from RBAC on their Action execution until updated (elastic#75563)
  [ML] fix type in apply_influencer_filters_action (elastic#77495)
  [Lens] create reusable component for filters and range aggregation (elastic#77453)
  fix eslint violations
  Collapse alert chart previews by default (elastic#77479)
  [ML] Fixing field caps wrapper endpoint (elastic#77546)
  showing service maps when filte by environment not defined (elastic#77483)
  [Lens] Settings panel redesign and separate settings per y axis (elastic#76373)
  log request body in new ES client (elastic#77150)
  use `navigateToUrl` to navigate to recent nav links (elastic#77446)
  Move core config service to `kbn/config` package (elastic#76874)
  [UBI] Copy license to /licenses folder (elastic#77563)
  Skip flaky Events Viewer Cypress test
  [Lens] Remove dynamic names in telemetry fields (elastic#76988)
  [Maps] Add DynamicStyleProperty#getMbPropertyName and DynamicStyleProperty#getMbPropertyValue (elastic#77366)
  [Enterprise Search] Add flag to restrict width of layout (elastic#77539)
  [Security Solutions][Cases - Timeline] Fix bug when adding a timeline to a case (elastic#76967)
  [Security Solution][Detections] Integration test for Editing a Rule (elastic#77090)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants