[Vis: Default editor] Euification of heatmap options tab by sulemanof · Pull Request #45766 · elastic/kibana

sulemanof · 2019-09-16T11:50:43Z

Summary

A part of #38273.
EUIfication of the Options tab in the Heatmap visualization.
Also fix #43666.

The PR also includes next changes:

created a common component <ColorRanges />, which is used in Gauge, Goal, Heatmap vis. It will be also used in Metric vis --->

created a common component <ColorSchemaOptions />, which is used in Gauge, Goal, Heatmap vis.

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/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/number_input.tsx # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/gauge/ranges_panel.tsx # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/index.ts # src/legacy/core_plugins/kbn_vislib_vis_types/public/types.ts # src/legacy/ui/public/vis/vis_types/vislib_vis_type.js

elasticmachine · 2019-09-16T12:09:18Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-16T15:04:26Z

Pinging @elastic/kibana-app

elasticmachine · 2019-09-16T15:56:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

sulemanof · 2019-09-17T11:23:23Z

Hi @cchaos !
Could you please take a look and help with UI suggestions!
Thanks in advance ❤

elasticmachine · 2019-09-17T12:55:54Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…ns/heatmap # Conflicts: # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json

elasticmachine · 2019-09-17T15:55:40Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-17T16:42:43Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cchaos · 2019-09-18T12:24:22Z

Here is some initial feedback from your screenshot.

…ns/heatmap

elasticmachine · 2019-09-18T15:50:00Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-18T16:30:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/gauge.js # src/legacy/core_plugins/kbn_vislib_vis_types/public/goal.js # src/legacy/core_plugins/kbn_vislib_vis_types/public/heatmap.js # src/legacy/core_plugins/kbn_vislib_vis_types/public/pie.js # src/legacy/ui/public/vis/vis_types/vislib_vis_type.js

maryia-lapata

LGTM, checked locally, also tested with dark mode and pseudo-localization.

cchaos

Just a couple more suggestions:

src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/color_schema.tsx

…ns/heatmap

sulemanof · 2019-09-25T13:37:26Z

There is a reason why I wouldn't swap switches positions:

Just a couple more suggestions:

There is a standalone component which represents Legend position and Show tooltip form rows -> <BasicOptions />. This component is used in several visualizations (with the same params order) ->

Area / Line / Bar:

Pie :

Coordinate Map :

and here in Heatmap.

To swap positions I need to restructure BasicOptions common component to accept children and to render it between Legend position and Show tooltip rows.
Could you please confirm that it's worth it?

…bana into EUIfication/options/heatmap

elasticmachine · 2019-09-25T15:32:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

flash1293

Code LGTM, tested and didn't notice any bugs

flash1293 · 2019-09-25T16:13:42Z

src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/heatmap/index.tsx

+            defaultMessage: 'Percentage mode',
+          })}
+          paramName="percentageMode"
+          value={stateParams.setColorRange ? false : stateParams.percentageMode}


nit: Why hiding the value of this switch if it's disabled? This isn't done in other places (e.g. label settings):

Because it can be confusing if the user sees "4 colors" in the config panel but that's really being overridden by the custom ranges they've established which may be 3 colors.

@cchaos
The Percentage mode switch (which Joe asked about) does not have any relation to the Number of colors, it is a standalone param which is excluded from visualization config if Use custom ranges is turned on! The same work for Number of colors.
I suppose @flash1293 asked why we didn't do the same in the Labels panel -> means to turn off all of params (Rotate and Overwrite automatic color) if Show labels is off.
Could you please confirm it is a right UX solution ? Thanks

I think I'm still confused, can you show me some screenshots?

test/functional/apps/visualize/_heatmap_chart.js

cchaos · 2019-09-25T20:10:28Z

@sulemanof

Could you please confirm that it's worth it?

It's not. Thanks.

…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/heatmap_options.html

elasticmachine · 2019-09-26T12:18:14Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-26T14:33:50Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/metrics_axes/label_options.tsx

elasticmachine · 2019-09-27T10:45:49Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* EUIficate heatmap options * Move collections * Split labels in a panel * Remove angular templates * Remove unused translations * Fix functional tests * Fix validation * Fix UI details * Compress color picker * Improve types * Replace headings h2 with h3 * Add functional tests * Create setCustomRangeByIndex

…6886) * EUIficate heatmap options * Move collections * Split labels in a panel * Remove angular templates * Remove unused translations * Fix functional tests * Fix validation * Fix UI details * Compress color picker * Improve types * Replace headings h2 with h3 * Add functional tests * Create setCustomRangeByIndex

sulemanof added 2 commits September 16, 2019 14:19

EUIficate heatmap options

afe52aa

Move collections

aa5908f

timroes mentioned this pull request Sep 16, 2019

EUIficate visualization option panels #38273

Closed

10 tasks

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

sulemanof requested a review from cchaos September 17, 2019 11:21

Split labels in a panel

3aa727a

sulemanof added 3 commits September 17, 2019 17:09

Remove angular templates

2aba289

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

bcf83b9

…ns/heatmap # Conflicts: # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json

Remove unused translations

5a38e6c

sulemanof added 2 commits September 18, 2019 17:05

Fix functional tests

d22fe0e

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

58d8a59

…ns/heatmap

sulemanof requested a review from maryia-lapata September 18, 2019 14:16

Fix validation

16afb0d

sulemanof added 2 commits September 19, 2019 14:44

Fix UI details

f99b900

maryia-lapata approved these changes Sep 23, 2019

View reviewed changes

maryia-lapata added the Feature:Heatmap Heatmap visualization label Sep 24, 2019

cchaos suggested changes Sep 24, 2019

View reviewed changes

src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/color_schema.tsx Outdated Show resolved Hide resolved

maryia-lapata reviewed Sep 25, 2019

View reviewed changes

src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/color_schema.tsx Outdated Show resolved Hide resolved

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

9ea7d72

…ns/heatmap

sulemanof added 2 commits September 25, 2019 16:44

Fix comments

75c266b

Merge branch 'EUIfication/options/heatmap' of github.com:sulemanof/ki…

225e0b8

…bana into EUIfication/options/heatmap

sulemanof requested a review from cchaos September 25, 2019 14:18

maryia-lapata mentioned this pull request Sep 25, 2019

[Vis: Default Editor] Show reset colors in Gauge, Goal after refresh #46552

Closed

flash1293 approved these changes Sep 25, 2019

View reviewed changes

cchaos approved these changes Sep 25, 2019

View reviewed changes

sulemanof added 3 commits September 26, 2019 13:17

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

7563c8b

…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/heatmap_options.html

Replace headings h2 with h3

a7c1f96

Add functional tests

50eaf23

sulemanof requested a review from myasonik September 26, 2019 11:14

sulemanof mentioned this pull request Sep 26, 2019

Variety of quick a11y fixes #46569

Merged

3 tasks

Create setCustomRangeByIndex

ea0d897

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

e119c16

…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/metrics_axes/label_options.tsx

sulemanof merged commit b322b60 into elastic:master Sep 27, 2019

sulemanof deleted the EUIfication/options/heatmap branch September 27, 2019 14:00

sulemanof mentioned this pull request Sep 29, 2019

[7.x] [Vis: Default editor] Euification of heatmap options tab (#45766) #46886

Merged

sulemanof added the backported label Sep 30, 2019

Conversation

sulemanof commented Sep 16, 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 Sep 16, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 16, 2019

Uh oh!

elasticmachine commented Sep 16, 2019

💔 Build Failed

Uh oh!

sulemanof commented Sep 17, 2019

Uh oh!

elasticmachine commented Sep 17, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 17, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 17, 2019

💔 Build Failed

Uh oh!

cchaos commented Sep 18, 2019

Uh oh!

elasticmachine commented Sep 18, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Sep 18, 2019

💚 Build Succeeded

Uh oh!

maryia-lapata left a comment

Choose a reason for hiding this comment

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sulemanof commented Sep 25, 2019

Uh oh!

elasticmachine commented Sep 25, 2019

💚 Build Succeeded

Uh oh!

flash1293 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flash1293 Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

cchaos Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

sulemanof Sep 26, 2019

Choose a reason for hiding this comment

Uh oh!

cchaos Sep 26, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cchaos commented Sep 25, 2019

Uh oh!

elasticmachine commented Sep 26, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Sep 26, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 27, 2019

💚 Build Succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

sulemanof commented Sep 16, 2019 •

edited

Loading

flash1293 left a comment •

edited

Loading