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

[OneDiscover][UnifiedDocViewer] Allow filtering by field type #189981

Merged
merged 15 commits into from
Aug 9, 2024

Conversation

jughosta
Copy link
Contributor

@jughosta jughosta commented Aug 6, 2024

Summary

This PR adds Field type filter to Doc Viewer (same as in UnifiedFieldList as discussed with @MichaelMarcialis).

The selected field types would be persisted in Local Storage under unifiedDocViewer:selectedFieldTypes key.

Screenshot 2024-08-07 at 16 52 46

Checklist

@jughosta jughosta added release_note:enhancement backport:skip This commit does not require backporting Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Feature:UnifiedDocViewer Issues relating to the unified doc viewer component Project:OneDiscover Enrich Discover with contextual awareness labels Aug 6, 2024
@jughosta jughosta self-assigned this Aug 6, 2024
@jughosta
Copy link
Contributor Author

jughosta commented Aug 6, 2024

/ci

@jughosta
Copy link
Contributor Author

jughosta commented Aug 7, 2024

/ci

@jughosta
Copy link
Contributor Author

jughosta commented Aug 7, 2024

/ci

@jughosta
Copy link
Contributor Author

jughosta commented Aug 7, 2024

/ci

@jughosta
Copy link
Contributor Author

jughosta commented Aug 8, 2024

/ci

@jughosta jughosta marked this pull request as ready for review August 8, 2024 10:47
@jughosta jughosta requested a review from a team as a code owner August 8, 2024 10:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Code changes look good and it works great! Nice improvement 👍

@@ -261,7 +239,7 @@ export const DocViewerTable = ({
]
);

const { pinnedItems, restItems } = Object.keys(flattened)
const { pinnedItems, restItems, allFields } = Object.keys(flattened)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: unrelated to this PR, but I wonder if we should start memoizing this? It feels like it's expensive to run on each render, especially since this component gets rendered each frame when resizing.

import {
FieldTypeFilter,
type FieldTypeFilterProps,
} from '@kbn/unified-field-list/src/components/field_list_filters/field_type_filter';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we export this directly from Unified Field List?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could but that would increase the bundle size and not only in DocViewer but in all consumers of UnifiedFieldList. I would rather keep the current import.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting 🤔 I would have thought tree shaking on the package would take care of that. I'm not a fan of reaching into internal package folders for imports, but I don't want to hold up this PR over it, so I think it's ok to merge as is.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #17 / rules_list component empty hides MaintenanceWindowCallout if filterConsumers does not match the running maintenance window's category

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
unifiedDocViewer 276 278 +2

Async chunks

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

id before after diff
unifiedDocViewer 192.4KB 202.8KB +10.4KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
unifiedDocViewer 12.0KB 12.0KB +7.0B
Unknown metric groups

async chunk count

id before after diff
unifiedDocViewer 10 11 +1

History

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

cc @jughosta

@jughosta jughosta merged commit a8aa215 into elastic:main Aug 9, 2024
23 checks passed
@jughosta jughosta deleted the 188733-doc-viewer-filter-by-type branch August 9, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:UnifiedDocViewer Issues relating to the unified doc viewer component Project:OneDiscover Enrich Discover with contextual awareness release_note:enhancement Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OneDiscover][UnifiedDocViewer] Allow filtering by field type
5 participants