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

[Lens] Replace expression_gauge from Goal to Bullet #177766

Merged
merged 39 commits into from
Mar 26, 2024

Conversation

nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Feb 23, 2024

Summary

Migrates the Lens Bullet Horizontal and Bullet Vertical charts to the new design. Adds the Semi-circular Gauge, Arc Gauge and Circular Gauge chart types to Lens. See comparison between old and new designs below.

Horizontal Bullet

image

Vertical Bullet

image

Semi-circular Gauge

image

Arc Gauge

image

Circular Gauge

image

Details

Refactors Lens expression_gauge from Goal to Bullet

  • Replace chart render spec and map previous inputs accordingly
  • Add support for Arc, Major Arc and Circle gauges
  • Add option to select gauge type in vis options
  • Update convert to lens for gauge vis_type renderer
  • Update gauge sizing in Lens container
  • Deprecates centralMajor and centralMajorMode on expression_gauge function

closes #171903
closes #155131

Checklist

Delete any items that are not applicable to this PR.

@nickofthyme nickofthyme marked this pull request as ready for review March 12, 2024 16:54
@nickofthyme nickofthyme requested review from a team as code owners March 12, 2024 16:54
@nickofthyme nickofthyme added the release_note:feature Makes this part of the condensed release notes label Mar 12, 2024
@mbondyra mbondyra changed the title Refactor Lens expression_gauge from Goal to Bullet [Lens] Replace expression_gauge from Goal to Bullet Mar 13, 2024
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.

This is wonderful! 😍 I have some design questions but nothing stopping this PR from merging. Love it, really.

Copy link
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together, @nickofthyme. It looks great! I've left a few comments below for your initial review. Let me know if you have any questions.

@nickofthyme nickofthyme requested a review from markov00 March 25, 2024 17:48
@nickofthyme
Copy link
Contributor Author

@elastic/kibana-design could I get a CO review please.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Serverless Rule Management - Security Solution Cypress Tests #5 / Detection rules, bulk edit of rule actions Restricted action privileges User with no privileges can't add rule actions User with no privileges can't add rule actions

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1671 1674 +3
eventAnnotationListing 499 502 +3
expressionGauge 89 90 +1
expressionHeatmap 157 160 +3
expressionPartitionVis 172 175 +3
expressionTagcloud 146 149 +3
expressionXY 232 235 +3
infra 1429 1432 +3
lens 1370 1373 +3
maps 1153 1156 +3
total +28

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
@kbn/chart-icons 78 84 +6
expressionGauge 59 58 -1
visualizations 810 831 +21
total +26

Async chunks

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

id before after diff
expressionGauge 19.0KB 22.5KB +3.5KB
lens 1.4MB 1.4MB +4.4KB
visTypeGauge 8.9KB 8.8KB -106.0B
total +7.8KB

Page load bundle

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

id before after diff
expressionGauge 14.7KB 13.9KB -836.0B
kbnUiSharedDeps-npmDll 6.3MB 6.3MB +887.0B
visTypeGauge 10.8KB 11.2KB +471.0B
visualizations 58.1KB 58.2KB +150.0B
total +672.0B
Unknown metric groups

API count

id before after diff
@kbn/chart-icons 78 84 +6
visualizations 841 862 +21
total +27

ESLint disabled line counts

id before after diff
visualizations 15 17 +2

References to deprecated APIs

id before after diff
visTypeGauge 1 2 +1

Total ESLint disabled count

id before after diff
visualizations 17 19 +2

Unreferenced deprecated APIs

id before after diff
expressionGauge 0 2 +2

History

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

@gvnmagni gvnmagni self-requested a review March 26, 2024 15:26
Copy link

@gvnmagni gvnmagni left a comment

Choose a reason for hiding this comment

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

Everything works fine, only two small things that don't affect the result of the review and that we can evaluate separately since they are not blockers:

  1. We should evaluate the default height of panel for Horizontal gauges
  2. Setting up a goals as 0 (zero) makes it disappear, since it gets in contrast with the zero baseline. We should consider which one is more important and evaluate that

Great job Nick! 🚀

@nickofthyme
Copy link
Contributor Author

Thanks for pointing out the 0 issue for the goal value. This will be fixed on the charts side.

@nickofthyme nickofthyme merged commit af56452 into elastic:main Mar 26, 2024
35 checks passed
@nickofthyme nickofthyme deleted the goal-to-bullet branch March 26, 2024 15:42
@kibanamachine kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Mar 26, 2024
@markov00 markov00 added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Apr 24, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

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:Lens release_note:feature Makes this part of the condensed release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust EC gauges in kibana with the new implementation [Lens] Support arc gauge chart in lens
10 participants