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

legacy agg vis tag cloud convert to lens #159348

Merged
merged 24 commits into from
Jun 12, 2023

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Jun 8, 2023

Part of #154307

Add convert to lens logic for tag cloud visualization

@nreese
Copy link
Contributor Author

nreese commented Jun 8, 2023

@elasticmachine merge upstream

@nreese nreese force-pushed the tagcloud_convertotlens branch from c25f484 to 95c5071 Compare June 8, 2023 20:02
@nreese nreese force-pushed the tagcloud_convertotlens branch from 071d359 to 8a75918 Compare June 9, 2023 15:41
@nreese nreese marked this pull request as ready for review June 12, 2023 02:26
@nreese nreese requested a review from a team as a code owner June 12, 2023 02:26
@nreese nreese added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:feature Makes this part of the condensed release notes v8.9.0 labels Jun 12, 2023
Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

This works great, the only thing that is missing is to convert the tagcloud with significant terms to a terms breakdown with significance. Are we doing this in this PR?

@nreese
Copy link
Contributor Author

nreese commented Jun 12, 2023

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Jun 12, 2023

This works great, the only thing that is missing is to convert the tagcloud with significant terms to a terms breakdown with significance. Are we doing this in this PR?

No, I was going to add significance terms convert to lens support in a separate PR. Its in a different location in the code base and effects more then just tag cloud

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Changes LGTM! I tested it locally in Chrome and the navigation from legacy tagcloud to lens works as expected.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
visTypeTagcloud 9 11 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
visualizations 781 793 +12

Async chunks

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

id before after diff
lens 1.3MB 1.3MB +160.0B
visTypeGauge 7.1KB 8.9KB +1.8KB
visTypeHeatmap 7.5KB 9.8KB +2.3KB
visTypeMetric 493.0B 1.3KB +872.0B
visTypePie 8.7KB 11.2KB +2.5KB
visTypeTable 16.2KB 18.0KB +1.8KB
visTypeTagcloud 2.2KB 3.2KB +1.0KB
visTypeTimeseries 508.3KB 509.3KB +1.1KB
visTypeXy 39.6KB 42.2KB +2.6KB
total +14.1KB

Page load bundle

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

id before after diff
visTypeGauge 12.4KB 10.8KB -1.6KB
visTypeHeatmap 12.4KB 10.8KB -1.6KB
visTypeMetric 11.0KB 10.5KB -539.0B
visTypePie 10.5KB 8.4KB -2.1KB
visTypeTable 18.0KB 16.7KB -1.3KB
visTypeTagcloud 5.7KB 6.5KB +826.0B
visTypeTimeseries 19.1KB 18.5KB -674.0B
visTypeXy 31.1KB 28.5KB -2.6KB
visualizations 56.6KB 56.7KB +157.0B
total -9.4KB
Unknown metric groups

API count

id before after diff
visualizations 811 823 +12

async chunk count

id before after diff
visTypeGauge 3 2 -1
visTypeHeatmap 1 2 +1
visTypePie 1 2 +1
visTypeTable 2 3 +1
visTypeTagcloud 1 2 +1
visTypeTimeseries 16 17 +1
total +4

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 491 495 +4
total +6

History

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

@nreese nreese merged commit e8e5cec into elastic:main Jun 12, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 12, 2023
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 release_note:feature Makes this part of the condensed release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants