[Vis: Default editor] EUIficate gauge/goal options tab #43265

sulemanof · 2019-08-14T12:51:55Z

Summary

A part of #38273.
EUIfication of the Options tab in the Gauge and Goal visualizations.

This includes:

get rid of all params, which were related to Metric type of gauge, since this type was removed -> removes left over code from the gauge-metric #15142 ;
all params, which were drawn using ng-hide, ng-show and ng-if becomes enabled/disabled for edit;
disable Alignment field when there is a single Metric aggregation and absence of Buckets:

added color ranges validation with an error hint:

Changes are done according to:

---- panel

## Style

Gauge Type --> Type
Alignment

--- panel

## Ranges

{List of ranges}
Auto extend range
Percentage mode
Color schema
  -- `Reset colors` as hidden/visible xs empty button in labelAppend
Reverse schema
Show legend
Show scale

--- panel

### Labels

Show labels
Sub label [disabled not hidden if show labels is false]
Display warnings [disabled if show labels is false]

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

…ns/gauge

elasticmachine · 2019-08-14T14:00:36Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

src/legacy/core_plugins/kbn_vislib_vis_types/public/editors/gauge.tsx

…ns/gauge # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/select.tsx # src/legacy/ui/public/vis/editors/default/vis_options.js

elasticmachine · 2019-08-19T12:02:41Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-08-19T14:55:07Z

Pinging @elastic/kibana-app

elasticmachine · 2019-08-19T16:09:03Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

maryia-lapata

I think maybe it makes sense to split each panel into a separate component. It'll allow to keep components smaller and a bit simpler. Also probably we will be able reuse Range panel in Heat map vis

src/legacy/core_plugins/kbn_vislib_vis_types/public/editors/gauge.tsx

…ns/gauge

elasticmachine · 2019-08-20T13:46:59Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

sulemanof · 2019-08-20T15:26:11Z

@maryia-lapata each panel was split. please, take a look

elasticmachine · 2019-08-20T16:18:05Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

maryia-lapata

LGTM, testes locally on Chrome, Mac

elasticmachine · 2019-08-21T10:40:30Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-08-21T12:22:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cchaos

👍

dmlemeshko

Functional tests update LGTM. Passed locally on both Chrome/Firefox 👍

dmlemeshko

Functional tests update LGTM. Passed locally on both Chrome/Firefox 👍

…ns/gauge

flash1293

Code LGTM

flash1293 · 2019-08-22T08:38:35Z

src/legacy/core_plugins/kbn_vislib_vis_types/public/gauge.d.ts

+  addLegend: boolean;
+  isDisplayWarning: boolean;
+  gauge: {
+    readonly backStyle: 'Full';


Not sure why some of those are set to readonly - could you elaborate?

The readonly params are only need for initial config and won't be changed anywhere.
We decided to mark them as readonly..

As they only have a single allowed value, it shouldn't change anything. Even if you don't have it readonly, you wouldn't be able to do params.type = 'xy';, right?

src/legacy/core_plugins/kbn_vislib_vis_types/public/editors/gauge/style_panel.tsx

flash1293 · 2019-08-22T09:49:29Z

src/legacy/core_plugins/kbn_vislib_vis_types/public/editors/gauge/style_panel.tsx

+
+  const setGaugeType = useCallback(
+    (paramName: 'gaugeType', value: GaugeVisParams['gauge']['gaugeType']) => {
+      const minAngle = value === 'Arc' ? undefined : 0;


Probably out of scope for this PR, but it seems like the angle params don't do anything useful for gauge - could we have this min and max angle as default params in the actual visualization? Then we don't have to handle this here in the form and just pass undefined for all cases.

minAngel and maxAngel play role in building the gauge vis :

Not sure what do you mean by skipping them..

I suppose you mean something like that ? :

Exactly, that's what I meant - moving the default values into the actual rendering code so we don't have to care about it in the form.

This would make sense!
I'll create a PR, maybe someone could verify that this changes would not break a possible configuration issues! (I mean the possibility of creating custom minAngel and maxAngel in the default config or even in a custom plugin )

sulemanof · 2019-08-22T11:45:09Z

retest

elasticmachine · 2019-08-22T12:55:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Euificate gauge options * Changes params places * Add ranges validation * Get rid of legacy translates * Fix functional tests * Split gauge to panel components * Disable alignment option

sulemanof added 3 commits August 14, 2019 10:57

Euificate gauge options

eb8b98f

Merge remote-tracking branch 'upstream/master' into EUIfication/optio…

0ed8375

…ns/gauge

Euificate gauge options

86c40e2

timroes mentioned this pull request Aug 14, 2019

EUIficate visualization option panels #38273

Closed

10 tasks

sulemanof added the WIP Work in progress label Aug 14, 2019

sulemanof requested a review from cchaos August 14, 2019 13:11

cchaos reviewed Aug 14, 2019

View reviewed changes

sulemanof added 4 commits August 16, 2019 12:59

Changes params places

4728101

Add ranges validation

4157e9d

Add validation

b85ee96

Merge remote-tracking branch 'upstream/master' into EUIfication/optio…

082abf0

…ns/gauge # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/select.tsx # src/legacy/ui/public/vis/editors/default/vis_options.js

Add validation

e284ed1

sulemanof removed the WIP Work in progress label Aug 19, 2019

sulemanof changed the title ~~[WIP][Vis: Default editor] EUIficate gauge options tab~~ [Vis: Default editor] EUIficate gauge options tab Aug 19, 2019

sulemanof marked this pull request as ready for review August 19, 2019 14:54

sulemanof added release_note:skip Skip the PR/issue when compiling release notes Feature:Vis Editor Visualization editor issues Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.4.0 v8.0.0 labels Aug 19, 2019

sulemanof requested a review from maryia-lapata August 19, 2019 14:55

Get rid of legacy translates

95a8125

sulemanof requested a review from cchaos August 19, 2019 18:18

sulemanof changed the title ~~[Vis: Default editor] EUIficate gauge options tab~~ [Vis: Default editor] EUIficate gauge/goal options tab Aug 19, 2019

maryia-lapata reviewed Aug 20, 2019

View reviewed changes

src/legacy/core_plugins/kbn_vislib_vis_types/public/editors/gauge.tsx Outdated Show resolved Hide resolved

src/legacy/core_plugins/kbn_vislib_vis_types/public/editors/gauge.tsx Outdated Show resolved Hide resolved

Fix functional tests

0fb197b

Merge remote-tracking branch 'upstream/master' into EUIfication/optio…

dbc3126

…ns/gauge

Split gauge to panel components

d267c56

maryia-lapata requested a review from flash1293 August 21, 2019 08:32

maryia-lapata approved these changes Aug 21, 2019

View reviewed changes

maryia-lapata requested a review from dmlemeshko August 21, 2019 08:48

sulemanof added 2 commits August 21, 2019 12:32

Disable alignment option

6a52296

Remove unused translations

b314010

Merge branch 'master' into EUIfication/options/gauge

35e640d

cchaos approved these changes Aug 21, 2019

View reviewed changes

dmlemeshko approved these changes Aug 22, 2019

View reviewed changes

sulemanof added 2 commits August 22, 2019 12:29

Bind values to useEffects

87ffc07

Merge remote-tracking branch 'upstream/master' into EUIfication/optio…

0c1f78e

…ns/gauge

flash1293 approved these changes Aug 22, 2019

View reviewed changes

sulemanof merged commit e568c3d into elastic:master Aug 22, 2019

sulemanof deleted the EUIfication/options/gauge branch August 22, 2019 13:02

sulemanof mentioned this pull request Aug 22, 2019

[7.x] [Vis: Default editor] EUIficate gauge/goal options tab (#43265) #43768

Merged

sulemanof added the backported label Aug 23, 2019

sulemanof mentioned this pull request Aug 26, 2019

Move angel values into vis rendering code #43968

Merged

7 tasks

[Vis: Default editor] EUIficate gauge/goal options tab #43265

[Vis: Default editor] EUIficate gauge/goal options tab #43265

Uh oh!

Conversation

sulemanof commented Aug 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Aug 14, 2019

💔 Build Failed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Aug 19, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Aug 19, 2019

Uh oh!

elasticmachine commented Aug 19, 2019

💔 Build Failed

Uh oh!

maryia-lapata left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Aug 20, 2019

💔 Build Failed

Uh oh!

sulemanof commented Aug 20, 2019

Uh oh!

elasticmachine commented Aug 20, 2019

💔 Build Failed

Uh oh!

maryia-lapata left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Aug 21, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Aug 21, 2019

💚 Build Succeeded

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

dmlemeshko left a comment

Choose a reason for hiding this comment

Uh oh!

dmlemeshko left a comment

Choose a reason for hiding this comment

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sulemanof commented Aug 14, 2019 •

edited

Loading