[ML] Redesign index-based Data Visualizer by qn895 · Pull Request #85726 · elastic/kibana

qn895 · 2020-12-14T02:00:58Z

Summary

This PR brings a redesign to the index based data visualizer, where instead of cards for fields, we use table in order to save space and provide more information in a compact way.

With the Show empty fields enabled:

With Distributions preview off:

With Distributions preview on:

To follow up

Replace the file based data visualizer with the table
Expand the functional tests to validate the content of the expanded cards

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

…-redesign

- Clear charts data upon unamounting explorer page - Avoid rendering three times per ChartContainer - Avoid expensive chartData/scaleCategoriesSet lookup

…emove vertical spacker

walterra · 2020-12-15T15:36:53Z

Some suggestions:

The blue chart's axis labels are missing the padding the other chart has
Should we align the styling so both charts have the same thin white padding between bars?
Is there a need to use different colors? I'd opt to use the same color like we do in the data grid. If different colors, IMHO it should match the icon colors in the type column.

mdefazio · 2020-12-15T15:56:20Z

Overall layout is looking good. Just a few tiny items:

Add some space between the 'Distribution' column heading and the icon button to show distribution charts

With 4px between
The top chart, search/filter bar and field counters feel cramped—I would suggest perhaps a large spacer between these.
I think the distribution charts a bit big and could use some extra padding on the top / bottom so they aren't so close to the edges of the rows
- Some of these have their x-axis labels touching the chart
- Can we get the axis font size to be the same as the axis font size on the main document distribution chart?

walterra · 2020-12-15T15:46:47Z

x-pack/plugins/ml/public/application/datavisualizer/_index.scss

@@ -1,2 +1,3 @@
 @import 'file_based/index';
 @import 'index_based/index';
+@import "stats_datagrid/index";


The other imports have single quotes.

Updated here 0dcdf2e

walterra · 2020-12-15T16:02:05Z

...a_frame_analytics/pages/analytics_management/components/analytics_list/use_table_settings.ts

 import { Direction, EuiBasicTableProps, Pagination, PropertySort } from '@elastic/eui';
 import { useCallback, useMemo } from 'react';
 import { ListingPageUrlState } from '../../../../../../../common/types/common';
+import { DataVisualizerIndexBasedAppState } from '../../../../../../../common/types/ml_url_generator';


Since useTableSettings() is now used in other places than just from within the analytics management page, should we move the file to a more general place?

walterra · 2020-12-15T16:06:02Z

...ication/datavisualizer/index_based/components/field_data_card/content_types/date_content.tsx

-            latestFormatted: formatDate(latest, TIME_FORMAT),
-          }}
+  return (
+    <EuiFlexGroup direction={'row'}>


Can you explain why we need a flex layout with a single item here?

Updated here 0dcdf2e

walterra · 2020-12-15T16:38:06Z

...pplication/datavisualizer/index_based/components/field_data_card/top_values/_top_values.scss

+.mlTopValuesLabelContainer {
+  padding-right: $euiSizeM;
+
+  &.small {
+    width: 50px !important;
+  }
+
+  &.large {
+    width: 200px !important;
+  }
+}


We should use a different structure, with this we end up with non-prefixed very generic css classes in the DOM like small/large. Consider using the BEM-style described on the bottom of this page (Sass best practices) https://elastic.github.io/eui/#/guidelines/sass.

You should end up creating classes that look like mlTopValuesLabelContainer--small. In the react code you will need to use both the base class and this modifier.

Updated here 0dcdf2e

walterra · 2020-12-15T16:40:35Z

.../application/datavisualizer/index_based/components/field_data_card/top_values/top_values.tsx

+              className={classNames(
+                'eui-textTruncate',
+                'mlTopValuesLabelContainer',
+                compressed === true ? 'small' : 'large'


With the SASS changes suggested in the other comment this should become e.g. mlTopValuesLabelContainer--${compressed === true ? 'small' : 'large'}

Updated here 0dcdf2e

…-redesign

qn895 · 2020-12-15T17:12:29Z

Add some space between the 'Distribution' column heading and the icon button to show distribution charts

The top chart, search/filter bar and field counters feel cramped—I would suggest perhaps a large spacer between these.

I think the distribution charts a bit big and could use some extra padding on the top / bottom so they aren't so close to the edges of the rows

Some of these have their x-axis labels touching the chart

Can we get the axis font size to be the same as the axis font size on the main document distribution chart?

@mdefazio @walterra I have updated the colors of the chart here, with additional paddings in between here 59e11f9

darnautov

LGTM

mdefazio

There were a few small things (also mentioned on Slack) that can be cleaned up. Fine if this is done in a follow-up PR.

mdefazio · 2020-12-15T18:51:47Z

...blic/application/datavisualizer/index_based/components/field_data_card/_field_data_card.scss


  .mlFieldDataCard__codeContent {
+    @include euiCodeFont;
    font-family: $euiCodeFontFamily;


You shouldn't need this extra declaration.

Updated here c16d60d

mdefazio · 2020-12-15T18:56:36Z

...pplication/datavisualizer/index_based/components/field_data_card/top_values/_top_values.scss

+  &.mlTopValuesLabelContainer--small {
+    width: 50px !important;
+  }
+
+  &.mlTopValuesLabelContainer--large {
+    width: 200px !important;
+  }


Ideally we would avoid both the pixel values and the use of !important. I understand if this needs to be done in a follow-up PR but we try to avoid these sort of things.

Updated here c16d60d

mdefazio · 2020-12-15T18:57:03Z

x-pack/plugins/ml/public/application/datavisualizer/stats_datagrid/_index.scss

+  }
+  .mlFieldDataCard__codeContent {
+    @include euiCodeFont;
+    font-family: $euiCodeFontFamily;


Again, don't need this with the mixin.

Updated here c16d60d

mdefazio · 2020-12-15T18:58:09Z

x-pack/plugins/ml/public/application/datavisualizer/stats_datagrid/_index.scss

+    .euiTableRowCell{
+      background-color: transparent !important;
+      border-top: 0px;
+      border-bottom: 1px solid $euiColorLightShade;


We can use $euiBorderThin instead of writing this out (both instances of this).

mdefazio · 2020-12-15T18:59:39Z

x-pack/plugins/ml/public/application/datavisualizer/stats_datagrid/_index.scss

+    text-transform: uppercase;
+    text-align: left;
+    color: $euiColorDarkShade;
+    font-weight: bold;


If we use EuiTitle we can do much of this as props. More of an FYI in the future.

kibanamachine · 2020-12-15T21:18:49Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c16d60d

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1593	1604	+11

Async chunks

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

id	before	after	diff
`ml`	5.3MB	5.3MB	+31.1KB

Distributable file count

id	before	after	diff
`default`	47241	48005	+764

History

💚 Build #94590 succeeded b2abf17
💔 Build #94569 failed 3305f1a
💚 Build #94286 succeeded 36497eb
💔 Build #94271 failed eea52b8
💔 Build #94155 failed 0c34e8a

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

qn895 added 30 commits November 23, 2020 11:33

[ML] Add multi filter components

3038aca

[ML] Add multi filter components

2783537

[ML] Add type to stats

aa426b7

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

8770f2c

…-redesign

[ML] Replace with new grid

4084c4e

[ML] Change order in Field Name

63f1607

[ML] Some improvements to multi select picker

5b46da8

[ML] Remove onchange call back

b8f5e32

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

e87cc71

…-redesign

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

3885bf4

…-redesign

[ML] Add previews for distribution

6cd9d2a

[ML] Rename DataVisualizerFieldNamesFilter

0f7f8ba

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

2fcb76f

…-redesign

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

2f00d2b

…-redesign

[ML] Save page state

76536d4

[ML] Update search logic

8f24a38

[ML] Remove redundant changes

f910267

[ML] Save field select params

e78e001

[ML] Update setters

57e56df

[ML] Remember searchQuery

012715d

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

f326a10

…-redesign

[ML] Update styling

f462340

[ML] Update styling

d7d60ed

[ML] Add charts to keyword

07e922d

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

11210e7

…-redesign

[ML] Add alignment for type

5a857bc

[ML] Fix typescript

d61271b

[ML] Update typescript for TopValuesPreview

7bac7ad

[ML] Update column test subj

8c18543

[ML] Move files into more appropriate folder

ce54544

qn895 added 3 commits December 15, 2020 08:28

[ML] Performance improvements for anomaly

078cb94

- Clear charts data upon unamounting explorer page - Avoid rendering three times per ChartContainer - Avoid expensive chartData/scaleCategoriesSet lookup

[ML] Update top 10 preview labels

286a76b

[ML] Remove any for FieldVisStats and multi select, use guttersize, r…

4ea6d17

…emove vertical spacker

[ML] Update colors of charts, styling, paddings

59e11f9

walterra reviewed Dec 15, 2020

View reviewed changes

qn895 added 6 commits December 15, 2020 10:45

[ML] Add type for OverallStats, remove flex groups

a021855

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

30c0394

…-redesign

Merge remote-tracking branch 'upstream/master' into ml-datavisualizer…

3305f1a

…-redesign

[ML] Update styling

0dcdf2e

[ML] Update translations

87a1377

[ML] Update paddings

b2abf17

[ML] Remove div

7c3da1a

darnautov approved these changes Dec 15, 2020

View reviewed changes

[ML] Fix styling of distributions

4bbcbf0

mdefazio approved these changes Dec 15, 2020

View reviewed changes

[ML] Add padding to percent value

c16d60d

qn895 merged commit 74ae348 into elastic:master Dec 15, 2020

qn895 deleted the ml-datavisualizer-redesign branch December 15, 2020 21:19

qn895 mentioned this pull request Dec 15, 2020

[7.x] [ML] Redesign index-based Data Visualizer (#85726) #86029

Merged

qn895 added a commit to qn895/kibana that referenced this pull request Dec 15, 2020

[ML] Redesign index-based Data Visualizer (elastic#85726)

e7bf9dd

qn895 added a commit that referenced this pull request Dec 15, 2020

[7.x] [ML] Redesign index-based Data Visualizer (#85726) (#86029)

f412820

This was referenced Dec 16, 2020

[ML] Fix functional tests for index based Data Visualizer #86071

Merged

[Meta][ML] Data Visualizer redesign & plugins integration #86387

Closed

This was referenced Dec 29, 2020

[DOCS] Refreshes Data Visualizer screenshot #87017

Merged

[DOCS] Update Data Visualizer details in machine learning tutorial elastic/stack-docs#1505

Merged

qn895 mentioned this pull request Jan 6, 2021

[ML] Redesign file-based Data Visualizer #87598

Merged

3 tasks

Conversation

qn895 commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

To follow up

Checklist

Uh oh!

walterra commented Dec 15, 2020

Uh oh!

mdefazio commented Dec 15, 2020

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!

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!

qn895 commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darnautov left a comment

Choose a reason for hiding this comment

Uh oh!

mdefazio 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!

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!

kibanamachine commented Dec 15, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

qn895 commented Dec 14, 2020 •

edited

Loading

qn895 commented Dec 15, 2020 •

edited

Loading