[MetricVis] Add possibility to apply color background for whole container in metric by VladLasitsa · Pull Request #125217 · elastic/kibana

VladLasitsa · 2022-02-10T11:35:38Z

Part of: #123797

Summary

Add colorFullBackground arg which allow to colorize full container
Add new errors which prohibit to use colorFullBackground as true for configuration when we set several metrics (split by bucket, add several metrics, use head arg in canvas expression)

My.Canvas.Workpad.-.Kibana.mp4

…colorize_container

Kuznietsov

Code mostly looks good to me, it has left to address a few comments I've left.
@VladLasitsa, thanks for your efforts 👍

...ugins/chart_expressions/expression_metric/common/expression_functions/metric_vis_function.ts

src/plugins/chart_expressions/expression_metric/public/components/metric_value.test.tsx

elasticmachine · 2022-02-15T14:04:17Z

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

VladLasitsa · 2022-02-15T14:04:36Z

@elasticmachine merge upstream

flash1293 · 2022-02-15T17:21:17Z

Seems like this is not working together with autoScale=true:

kibana
| kibana_context savedSearchId=null timeRange={timerange from="now-7d" to="now"}
| esaggs index={indexPatternLoad id="90943e30-9a47-11e8-b64d-95841ca0b247"} aggs={aggAvg id="0" enabled=true schema="metric" field="bytes"} metricsAtAllLevels=false partialRows=false
| metricVis metric={visdimension accessor="col-0-0"} colorMode="Background" colorFullBackground=true autoScale=true 
  palette={palette "#ff0000" "#00ff00" "#00ff00" gradient=false stop=0 stop=1 stop=6000 range="number" continuity="below"}
| render

…colorize_container

VladLasitsa · 2022-02-17T07:46:00Z

Seems like this is not working together with autoScale=true:

kibana
| kibana_context savedSearchId=null timeRange={timerange from="now-7d" to="now"}
| esaggs index={indexPatternLoad id="90943e30-9a47-11e8-b64d-95841ca0b247"} aggs={aggAvg id="0" enabled=true schema="metric" field="bytes"} metricsAtAllLevels=false partialRows=false
| metricVis metric={visdimension accessor="col-0-0"} colorMode="Background" colorFullBackground=true autoScale=true 
  palette={palette "#ff0000" "#00ff00" "#00ff00" gradient=false stop=0 stop=1 stop=6000 range="number" continuity="below"}
| render

@flash1293 , I fixed it, could you please recheck?

flash1293 · 2022-02-17T10:44:38Z

@VladLasitsa Still doesn't work for me:

with autoScale:

without autoScale:

kibana
| kibana_context savedSearchId=null timeRange={timerange from="now-7d" to="now"}
| esaggs index={indexPatternLoad id="90943e30-9a47-11e8-b64d-95841ca0b247"} aggs={aggAvg id="0" enabled=true schema="metric" field="bytes"} metricsAtAllLevels=false partialRows=false
| metricVis metric={visdimension accessor="col-0-0"} colorMode="Background" colorFullBackground=true 
  palette={palette "#ff0000" "#00ff00" "#00ff00" gradient=false stop=0 stop=1 stop=6000 range="number" continuity="below"}
| render

flash1293

LGTM, works as expected 👍

VladLasitsa · 2022-02-17T12:30:25Z

@elasticmachine merge upstream

VladLasitsa · 2022-02-21T08:20:26Z

@elasticmachine merge upstream

Kuznietsov

Mostly LGTM 👍 Need to test it locally. Could you, please, address all the issues I've mentioned below. Thanks.

src/plugins/chart_expressions/expression_metric/public/components/with_auto_scale.tsx

src/plugins/chart_expressions/expression_metric/public/components/metric_component.tsx

Kuznietsov · 2022-02-22T08:20:54Z

Tested locally, works fine. Waiting for the code updates and going to approve this PR. @VladLasitsa, thanks for the efforts.

MichaelMarcialis

I've left a few small wording suggestions, but nothing worth holding you up for. Approving, assuming those get made.

...ugins/chart_expressions/expression_metric/common/expression_functions/metric_vis_function.ts

…ion_functions/metric_vis_function.ts Co-authored-by: Michael Marcialis <michael@marcial.is>

VladLasitsa · 2022-02-23T11:30:12Z

@elasticmachine merge upstream

VladLasitsa · 2022-02-23T13:07:50Z

@elasticmachine merge upstream

VladLasitsa · 2022-02-23T15:35:25Z

@elasticmachine merge upstream

kibana-ci · 2022-02-23T17:00:52Z

💚 Build Succeeded

Metrics [docs]

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
`expressionMetricVis`	44	46	+2

Async chunks

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

id	before	after	diff
`expressionMetricVis`	10.1KB	11.0KB	+960.0B

Page load bundle

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

id	before	after	diff
`expressionMetricVis`	8.1KB	8.9KB	+812.0B

Unknown metric groups

API count

id	before	after	diff
`expressionMetricVis`	44	46	+2

ESLint disabled line counts

id	before	after	diff
`expressionMetricVis`	6	7	+1

Total ESLint disabled count

id	before	after	diff
`expressionMetricVis`	6	7	+1

History

💔 Build #25865 failed c173a49
💔 Build #25828 failed b271972
💔 Build #25800 failed 4eed8d2
💛 Build #25477 was flaky 237a791
💚 Build #25210 succeeded 6558d12

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

cc @VladLasitsa

…iner in metric (elastic#125217) * Add `colorFullBackground` arg which allow to colorize full container * Fix Checks * Fix condition * Fix CI * Fix snapshots * Fixed remarks * Fix full color background for auto scaling * Fix CI * Fix autoscale styles * Fix nit's * Update src/plugins/chart_expressions/expression_metric/common/expression_functions/metric_vis_function.ts Co-authored-by: Michael Marcialis <michael@marcial.is> * Fix some nit's Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Michael Marcialis <michael@marcial.is>

Add colorFullBackground arg which allow to colorize full container

75f425a

VladLasitsa requested a review from Kuznietsov February 10, 2022 11:35

VladLasitsa added 4 commits February 10, 2022 15:06

Fix Checks

3fb7903

Fix condition

d4df724

Fix CI

6a7c2dc

Fix snapshots

bb4169c

VladLasitsa self-assigned this Feb 11, 2022

Merge remote-tracking branch 'upstream/main' into add_possibility_to_…

469a829

…colorize_container

VladLasitsa changed the title ~~Add possibility to apply color background for whole container in metric~~ [MetricVis] Add possibility to apply color background for whole container in metric Feb 14, 2022

Kuznietsov reviewed Feb 14, 2022

View reviewed changes

Fixed remarks

35f06bb

VladLasitsa requested a review from Kuznietsov February 14, 2022 11:19

VladLasitsa marked this pull request as ready for review February 15, 2022 14:04

VladLasitsa requested review from a team as code owners February 15, 2022 14:04

VladLasitsa requested a review from flash1293 February 15, 2022 14:04

Merge branch 'main' into add_possibility_to_colorize_container

b21333f

VladLasitsa mentioned this pull request Feb 16, 2022

[Lens] Switch to unified metric renderer #123797

Closed

VladLasitsa added 3 commits February 16, 2022 17:11

Fix full color background for auto scaling

2895228

Merge remote-tracking branch 'upstream/main' into add_possibility_to_…

4198e61

…colorize_container

Fix CI

89147da

Fix autoscale styles

4f6cde7

flash1293 approved these changes Feb 17, 2022

View reviewed changes

Merge branch 'main' into add_possibility_to_colorize_container

d53efbd

Merge branch 'main' into add_possibility_to_colorize_container

6558d12

Kuznietsov reviewed Feb 21, 2022

View reviewed changes

Fix nit's

237a791

VladLasitsa requested a review from Kuznietsov February 22, 2022 12:51

Kuznietsov approved these changes Feb 22, 2022

View reviewed changes

MichaelMarcialis approved these changes Feb 22, 2022

View reviewed changes

VladLasitsa and others added 2 commits February 23, 2022 12:50

Update src/plugins/chart_expressions/expression_metric/common/express…

4eed8d2

…ion_functions/metric_vis_function.ts Co-authored-by: Michael Marcialis <michael@marcial.is>

Fix some nit's

1951fa4

Merge branch 'main' into add_possibility_to_colorize_container

b271972

Merge branch 'main' into add_possibility_to_colorize_container

c173a49

Merge branch 'main' into add_possibility_to_colorize_container

2037574

VladLasitsa merged commit 4bffe73 into elastic:main Feb 23, 2022

Conversation

VladLasitsa commented Feb 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Kuznietsov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Feb 15, 2022

Uh oh!

VladLasitsa commented Feb 15, 2022

Uh oh!

flash1293 commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VladLasitsa commented Feb 17, 2022

Uh oh!

flash1293 commented Feb 17, 2022

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

VladLasitsa commented Feb 17, 2022

Uh oh!

VladLasitsa commented Feb 21, 2022

Uh oh!

Kuznietsov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kuznietsov commented Feb 22, 2022

Uh oh!

MichaelMarcialis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VladLasitsa commented Feb 23, 2022

Uh oh!

VladLasitsa commented Feb 23, 2022

Uh oh!

VladLasitsa commented Feb 23, 2022

Uh oh!

kibana-ci commented Feb 23, 2022

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

VladLasitsa commented Feb 10, 2022 •

edited

Loading

flash1293 commented Feb 15, 2022 •

edited

Loading