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] Add gauges visualization #118616

Merged
merged 51 commits into from
Dec 7, 2021
Merged

[Lens] Add gauges visualization #118616

merged 51 commits into from
Dec 7, 2021

Conversation

mbondyra
Copy link
Contributor

@mbondyra mbondyra commented Nov 15, 2021

Summary

Leftovers: #121376

Screenshot 2021-11-26 at 15 55 03

Fixes #89859

Default settings:

  • chart displays when having a metric there. Minimum value defaults to 0 (or metric - 10 when less than zero) and maximum value defaults to the first 'nicely rounded number':
  • the title is taken from the metric dimension label.
  • the subtitle is not displayed. The colors are not displayed.
  • the numbers are rounded to whole unless (max-min<5), then it's 0.0.
  • the suggestion with the opposite gauge is only shown when gauge is displayed (so when having vertical active, we show horizontal)

Screenshot 2021-11-25 at 16 54 42

Visualization settings

Screenshot 2021-11-25 at 16 58 16

Dimensions settings

Metric

  • formatting
  • band colors
  • color palette - color palette is by default transparentized, but user can choose not transparent colors if they want to.

Screenshot 2021-11-25 at 17 00 06

- ticks and color bands are not aligned unless user click on the setting in Visualization settings

Screenshot 2021-11-25 at 17 01 49

Screenshot 2021-11-25 at 17 02 10

Screenshot 2021-11-25 at 17 04 06

@mbondyra mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens release_note:feature Makes this part of the condensed release notes v8.1.0 labels Nov 15, 2021
@mbondyra mbondyra force-pushed the lens/gauges branch 4 times, most recently from ec6359c to 25f430d Compare November 23, 2021 19:37
@elastic elastic deleted a comment from kibana-ci Nov 23, 2021
@mbondyra mbondyra force-pushed the lens/gauges branch 5 times, most recently from 037fcff to bb0db71 Compare November 26, 2021 14:32
@mbondyra mbondyra marked this pull request as ready for review November 26, 2021 14:36
@mbondyra mbondyra requested review from a team as code owners November 26, 2021 14:36
@elasticmachine
Copy link
Contributor

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

@mbondyra mbondyra force-pushed the lens/gauges branch 3 times, most recently from 4ff5a02 to 423743e Compare November 29, 2021 07:49
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 making those requested changes so quickly, @mbondyra! This looks wonderful. In my last test, I found two (hopefully small) items that I comment on below. Once these are addressed, I think I'm good to approve.

  • I noticed that the layer panels for gauge-type visualizations don't have a layer-level chart selector. Should we have a selector there that allows users to switch between compatible chart types (i.e. switch between horizontal and vertical gauges)?

image

  • For the band color configurations, do we need to include the color continuity options (at least until the new color range enhancements are integrated into Lens)? Right now it appears to the user cannot change from the default "above range" behavior.

image

@mbondyra
Copy link
Contributor Author

mbondyra commented Dec 2, 2021

Discussed offline:

  1. let’s keep it as you have it for now (i.e. only having the chart selector on visualization layer panels when it’s a chart type that supports multi-layer) and revisit whenever we get around to addressing the chart pickers.
  2. I added an item in the leftovers issue. It would be much easier to achieve something meaningful with the new color stops UI so I'll wait till this PR gets merged. If for some reason we would have problems with delivering it for 8.1, I am gonna add old color continuity options for 8.1.

Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Tested the feature locally on Safari and could not spot bugs 👍 .

I have still few questions for things which are not 100% clear to me:

  • Why having a static value by default for a metric? Maybe there's a use case I'm missing here.

  • What should happen for array values and last_value?

    • I've tried to use the ecommerce dataset with the price field and last_value and got "no results".

Screenshot 2021-12-03 at 14 33 20

  • I thought that maybe there were no values, so I've added a filtered metric there, still no results:

Screenshot 2021-12-03 at 14 33 48

  • Maybe it makes sense to not show anything as "multiple" values are in the field, but a warning might be helpful here. Also, suggestions should be filtered if leads to the same results, I think.

  • Ticks on bands could propose a help icon there to explain what that means.

    • sometimes enabling the feature doesn't do much as there's no misalignment yet between ticks and bands.

    gauge_follow_up_help

  • While I like the "suggested" mechanism here for min/max, I think that it is too easy to have a gauge with no defined min/max on a dashboard. I do not see this type of chart particularly useful, unless has been already defined a use case for that.

    gauge_suggested_minmax

    • If not preventing the user to save that type of gauge, can we, at least show some warning on save about that? Even better would be a warning on the Lens panel in the dashboard explaining the problem.

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.

Per our conversation yesterday, this looks good from my perspective. Thanks, @mbondyra!

@mbondyra
Copy link
Contributor Author

mbondyra commented Dec 3, 2021

What should happen for array values and last_value? - I've changed the behavior to instead take the last value from the array.

@mbondyra
Copy link
Contributor Author

mbondyra commented Dec 6, 2021

@elasticmachine merge upstream

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 fine Marta and the bug is solved! I am approving. The only thing that I found is that when I have a gauge and open the dimensions editor, the chart jumps
Lens-Elastic-3

I am not sure if it is caused by this PR, I can't replicate in master though

@mbondyra
Copy link
Contributor Author

mbondyra commented Dec 6, 2021

Thanks Stratoula, I'll css things around to see if it can help! EDIT: FIXED

@elastic elastic deleted a comment from kibana-ci Dec 7, 2021
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default CI Group #7 / InfraOps App Metrics UI Home page with metrics present group nodes by custom field

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
charts 66 71 +5
lens 674 721 +47
total +52

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
charts 281 282 +1
lens 243 244 +1
total +2

Async chunks

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

id before after diff
lens 969.7KB 1.1MB +112.4KB

Page load bundle

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

id before after diff
charts 56.6KB 57.4KB +820.0B
expressionHeatmap 26.5KB 26.3KB -222.0B
lens 40.3KB 43.5KB +3.1KB
total +3.7KB
Unknown metric groups

API count

id before after diff
charts 313 314 +1
lens 260 261 +1
total +2

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

@mbondyra mbondyra merged commit 5c585f5 into elastic:main Dec 7, 2021
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Dec 7, 2021
@mbondyra mbondyra deleted the lens/gauges branch December 16, 2021 08:15
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
* add gauge chart expression

* excluding staticValue from suggestion for other visualizations

* adding prioritized operation for visualization groups (when dropping, it should be chosen over the default)

* changing the group for metric in chart switcher

* only access the activeData if exists (avoids error in console)

* Adding gauge chart

* round nicely the maximum value

* fix warnings

* fixing rounding

* fixing tests

* suggestions tests

* suggestions tbc

* tests for staticValue limitations

* added tests to visualization.ts

* correct bands

* wip

* added tests

* suggestions

* palete fixes & tests

* fix magic max

* metric should not overflow

* address review

* [Lens] adding toolbar tests

* limit domain to <min, max>

* changes the order of experimental badge and dataLoss indicator

* fix i18n

* address feedback p1

* don't show required nor optional for max/min dimensions

* fix types

* tests fixed

* fix types

* last piece of gauge feedback

* change naming from title and subtitle to labelMajor and labelMinor

* fix bug with percent color bands

* functional tests

* metric shouldn't have static value

* pass formatter to metric

* fake formatter

* fix css and replace emptyPlaceholder

* fix tests

* fix static values

* name changes

* breaking functional

Co-authored-by: Kibana Machine <[email protected]>
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.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Support bullet/gauge chart
8 participants