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.

Sign up for GitHub

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

Jump to bottom

[Gauge] Visdimensions support. #124927

Merged
merged 18 commits into from
Feb 16, 2022
Merged

[Gauge] Visdimensions support. #124927

merged 18 commits into from
Feb 16, 2022

Conversation

Kuznietsov
Copy link
Contributor

@Kuznietsov Kuznietsov commented Feb 8, 2022 •
edited
Loading

Summary

Completes part of the issue

Changes performed:

  • Added support of visdimensions at min/max/metric/goal.
  • Changed expression args metricAccessor, minAccessor, maxAccessor goalAccessor to min/max/metric/goal.
  • Added some changes for consistency with other chart_expressions.
visdimensions.support.mov

@Kuznietsov Kuznietsov added the WIP Work in progress label Feb 8, 2022
@Kuznietsov Kuznietsov self-assigned this Feb 8, 2022
@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@Kuznietsov Kuznietsov changed the title [Gauge] Adaptation for VisEditors. [Gauge] Visdimensions support. Feb 11, 2022
@Kuznietsov Kuznietsov added backport:skip This commit does not require backporting impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort NeededFor:VisEditors release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.2.0 and removed WIP Work in progress labels Feb 11, 2022
@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@Kuznietsov Kuznietsov marked this pull request as ready for review February 15, 2022 08:13
@Kuznietsov Kuznietsov requested a review from a team as a code owner February 15, 2022 08:13
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

VladLasitsa
VladLasitsa approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@VladLasitsa VladLasitsa left a comment

Choose a reason for hiding this comment

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

LGTM, Tested locally and everything works fine! Great job!

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 735 736 +1
visualizations 207 209 +2
total +3

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
expressionGauge 62 66 +4
visualizations 320 326 +6
total +10

Async chunks

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

id before after diff
expressionGauge 8.5KB 8.6KB +76.0B
lens 1.0MB 1.0MB +103.0B
visualizations 157.2KB 157.2KB +2.0B
total +181.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
expressionGauge 1 2 +1

Page load bundle

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

id before after diff
expressionGauge 9.9KB 11.6KB +1.7KB
expressionHeatmap 13.4KB 13.4KB -12.0B
expressionMetricVis 7.1KB 7.1KB -12.0B
expressionPartitionVis 20.1KB 20.1KB -12.0B
expressionTagcloud 7.9KB 7.9KB -12.0B
visualizations 43.4KB 43.8KB +373.0B
total +2.0KB
Unknown metric groups

API count

id before after diff
expressionGauge 62 66 +4
visualizations 341 347 +6
total +10

History

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

cc @Kunzetsov

mbondyra
mbondyra approved these changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Great job, code LGTM! A shame we didn't release it in 8.1 to avoid the mapping min -> minAccessor for Lens 😅 Tested on Chrome and works as expected 👌🏼

@Kuznietsov Kuznietsov merged commit 47ec103 into elastic:main Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VladLasitsa VladLasitsa VladLasitsa approved these changes

@mbondyra mbondyra mbondyra approved these changes

@crob611 crob611 Awaiting requested review from crob611

@flash1293 flash1293 Awaiting requested review from flash1293

@stratoula stratoula Awaiting requested review from stratoula

@alexwizp alexwizp Awaiting requested review from alexwizp

Assignees

@Kuznietsov Kuznietsov

Labels
backport:skip This commit does not require backporting impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort NeededFor:VisEditors release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Kuznietsov @elasticmachine @kibana-ci @mbondyra @VladLasitsa @kibanamachine