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

Prevent showing filter on unfilterable fields in data grid #103241

Merged
merged 1 commit into from
Jun 24, 2021

Conversation

timroes
Copy link
Contributor

@timroes timroes commented Jun 24, 2021

Summary

Fixes #91677

Currently we show the filter button for too many fields, e.g. geo_point fields, which are aggregatable and searchable. We should instead check on the filterable property of the fields (which is also what the legacy doc table does), which will correctly be false for geo_point fields.

Checklist

Delete any items that are not applicable to this PR.

@timroes timroes added Feature:Discover Discover Application release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 :KibanaApp/fix-it-week v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 24, 2021
@timroes timroes requested a review from a team June 24, 2021 11:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM 👍 , tested locally, works. While testing I noted, that we should improve the following message in the doc viewer table, because it's not telling the truth 😄
Bildschirmfoto 2021-06-24 um 15 07 37
Also we should display those buttons in a disabled state in this case, but it's not the scope of this PR.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
discover 431.2KB 431.1KB -20.0B

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

@timroes timroes merged commit 0a2042e into elastic:master Jun 24, 2021
@timroes timroes deleted the discover/fix-geo-filter branch June 24, 2021 13:14
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 24, 2021
kibanamachine added a commit that referenced this pull request Jun 24, 2021
Co-authored-by: Tim Roes <tim.roes@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Discover Discover Application release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover] Using EuiDataGrid geo_point data should not be filterable
4 participants