-
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
[UnifiedFieldList] Migrate field filters and field search from Lens #147255
[UnifiedFieldList] Migrate field filters and field search from Lens #147255
Conversation
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
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.
Sass changes LGTM.
@jughosta why can't I see the filters in Discover text based mode? |
Hi @stratoula, |
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.
Got it Julia! Visualizations team changes LGTM!
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.
Code LGTM, great refactoring, tests, and documentation. What a nice 🎁 for 🎄! Tested locally in Chrome, Firefox, Safari. Works as expected ✅
It might be worth adding information, that you can now search by text and keyword fields in Lens (Before it was "Text string")
And we might consider to handle _index
as a keyword field?
Unrelated to this PR, since it was already there, but does it make sense to filter by "Records"
|
||
function normalizeFieldType(type: string) { | ||
if (type === 'histogram') { | ||
// TODO: check why are we replacing this type |
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 assume because it's an array of numbers?
https://www.elastic.co/guide/en/elasticsearch/reference/current/histogram.html
A field to store pre-aggregated numerical data representing a histogram. This data is defined using two paired arrays:
@@ -47,7 +47,7 @@ export function convertDataViewIntoLensIndexPattern( | |||
customLabel: field.customLabel, | |||
runtimeField: field.runtimeField, | |||
runtime: Boolean(field.runtimeField), | |||
timeSeriesMetricType: field.timeSeriesMetric, | |||
timeSeriesMetric: field.timeSeriesMetric, |
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.
👍 for eye for details and consistency ... just saying!
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @jughosta |
A follow up for #147255 Copy is suggested by Gail. <img width="614" alt="Screenshot 2023-01-16 at 12 52 23" src="https://user-images.githubusercontent.com/1415710/212672146-ca3649d5-cb0d-42d3-b666-57ce51e09f29.png"> Co-authored-by: Kaarina Tungseth <[email protected]>
Closes #145081 Part 1 of this integration was done in #147255 <img width="640" alt="Screenshot 2023-01-16 at 16 58 59" src="https://user-images.githubusercontent.com/1415710/212720438-8f37eb69-635a-4611-89bb-34b095d79b8c.png"> ## Summary This PR integrates the unified field list filters and search into Discover. - [x] Integrate into Discover - [x] Clean up deprecated code ("searchable"/"aggregatable" filters were removed too) - [x] Refactor field icons, labels, desc to use the unified ones - [x] Field list in SQL view needs refactoring to use the received field types rather than data view field types - [x] Update tests ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Stratoula Kalafateli <[email protected]>
…c#148547) Closes elastic#145081 Part 1 of this integration was done in elastic#147255 <img width="640" alt="Screenshot 2023-01-16 at 16 58 59" src="https://user-images.githubusercontent.com/1415710/212720438-8f37eb69-635a-4611-89bb-34b095d79b8c.png"> ## Summary This PR integrates the unified field list filters and search into Discover. - [x] Integrate into Discover - [x] Clean up deprecated code ("searchable"/"aggregatable" filters were removed too) - [x] Refactor field icons, labels, desc to use the unified ones - [x] Field list in SQL view needs refactoring to use the received field types rather than data view field types - [x] Update tests ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Stratoula Kalafateli <[email protected]>
Closes #145089 and #147003
Summary
This PR migrates field filters and field search from Lens plugin to UnifiedFieldList plugin:
form_based
andtext_based
viewstext_based
view with search highlights and type filterstimeSeriesMetricType
withtimeSeriesMetric
to be able to use this logic for both DataViewField and IndexPatternFieldonFilterField
logic to Unified Field ListavailableFieldTypes
logic to Unified Field ListChecklist