Skip to content

Conversation

@nickpeihl
Copy link
Member

@nickpeihl nickpeihl commented Oct 10, 2019

Summary

Fixes #39219. This adds date fields to the supported fields for dynamic styles in the maps app.

For example, this screenshot shows the most recent five locations of an airplane in flight. The older locations are smaller and lighter colored than more recent locations.

This builds upon the work in PR #47389. Ideally, we could merge that PR first.

Screen Shot 2019-10-10 at 1 55 11 PM

@nickpeihl nickpeihl added release_note:enhancement Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.5.0 labels Oct 10, 2019
@nickpeihl nickpeihl requested a review from nreese October 10, 2019 20:57
@elasticmachine
Copy link
Contributor

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

});

return [...numberFieldOptions, ...joinFields];
return [...timeFieldOptions, ...numberFieldOptions, ...joinFields];
Copy link

Choose a reason for hiding this comment

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

Would this list time fields list before number fields in the UI? (Or is there something else sorting them later?) 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

The list in the dropdown appears to be alphabetical by field label.

@bcamper
Copy link

bcamper commented Oct 10, 2019

Love this feature and think it will be really useful!

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese
Copy link
Contributor

nreese commented Oct 11, 2019

I get the follow console errors when I create a grid aggregation layer or EMS boundaries layer and add it to the map. AbstractSource may need a default implementation of getTimeFields

Screen Shot 2019-10-11 at 10 15 29 AM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

const { supportsFeatureState, isScaled, name, range, computedName } = styleFields[j];
const value = parseFloat(feature.properties[name]);
//Date fields pulled from doc_values is an array of epoch_millis and a date string
const value = Array.isArray(feature.properties[name])
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move the logic for extracting date fields out of VectorStyle which is generic and should not have source specific details. You can put the logic into hitsToGeoJson

const initialSearchContext = {
docvalue_fields: searchFilters.fieldNames
};
initialSearchContext.docvalue_fields.push(...searchFilters.fieldNames);
Copy link
Contributor

Choose a reason for hiding this comment

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

The time field is getting included in docvalue_fields twice. Below is an example request. To prevent this, filter searchFilters.fieldNames to exclude time fields since they are already in docvalue_fields.

"docvalue_fields": [
    {
      "field": "@timestamp",
      "format": "epoch_millis"
    },
    "@timestamp",
    "geo.coordinates"
  ],

@bcamper
Copy link

bcamper commented Oct 11, 2019

Looks like the legend is showing a raw timestamp when styling by date?

Screen Shot 2019-10-11 at 5 33 37 PM

@nreese
Copy link
Contributor

nreese commented Oct 14, 2019

Looks like the legend is showing a raw timestamp when styling by date?

@bcamper I have created a separate PR, #48132, to format legend labels by field formatter. This will address this problem and format the value as a date.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@bcamper
Copy link

bcamper commented Oct 14, 2019

Currently this allows you to link symbol orientation to a date field (anecdotally it seems all icons are oriented at 0 degrees though). Unless we have a clear rationale for mapping date to orientation (🤔?), we may want to exclude dates there.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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.

lgtm
code review, tested in chrome

@nreese
Copy link
Contributor

nreese commented Oct 15, 2019

Currently this allows you to link symbol orientation to a date field (anecdotally it seems all icons are oriented at 0 degrees though). Unless we have a clear rationale for mapping date to orientation (🤔?), we may want to exclude dates there.

@bcamper Created #48294 to track

@nreese nreese merged commit f2bf35b into elastic:master Oct 15, 2019
nreese pushed a commit to nreese/kibana that referenced this pull request Oct 15, 2019
* [Maps] retrieve geo_point value from docvalue_fields instead of _source

* [Maps] retrieve geo_point value from docvalue_fields instead of _source

* Dynamically style by time field

* Add epoch_millis to date fields in docvalues

* add functional test ensuring _search request only pulls what is needed

* Add functional test for dynamic styles by date field

* Support dynamic styles in top hits agg

* Add getTimeFields fn to vector_source

* Retrieve only epoch_millis for date fields in docvalues

* use new Fields syntax, simplify docvalue_fields creation

* fix comment
@nreese
Copy link
Contributor

nreese commented Oct 15, 2019

7.x #48295

@bcamper
Copy link

bcamper commented Oct 15, 2019

💥💥💥

nreese added a commit that referenced this pull request Oct 16, 2019
* [Maps] retrieve geo_point value from docvalue_fields instead of _source

* [Maps] retrieve geo_point value from docvalue_fields instead of _source

* Dynamically style by time field

* Add epoch_millis to date fields in docvalues

* add functional test ensuring _search request only pulls what is needed

* Add functional test for dynamic styles by date field

* Support dynamic styles in top hits agg

* Add getTimeFields fn to vector_source

* Retrieve only epoch_millis for date fields in docvalues

* use new Fields syntax, simplify docvalue_fields creation

* fix comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:enhancement Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v7.5.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Maps] Dynamic style by document age

4 participants