Skip to content

[ML] Data Frame Analytics: Fix confusion matrix data grid width. #65818

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
May 13, 2020
Merged

[ML] Data Frame Analytics: Fix confusion matrix data grid width. #65818

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5dd76c8
[ML] Data Frame Analytics: Fix confusion matrix data grid width.
walterra May 8, 2020
b5ebbd9
[ML] Styles cleanup.
walterra May 8, 2020
de5e1c0
[ML] Fix IE11 layout issues.
walterra May 8, 2020
248bd34
Merge branch 'master' into ml-analytics-fix-data-grid-width
elasticmachine May 11, 2020
fc67d76
Merge branch 'master' into ml-analytics-fix-data-grid-width
elasticmachine May 11, 2020
a6fd891
Merge branch 'master' into ml-analytics-fix-data-grid-width
walterra May 12, 2020
7905eec
[ML] calc() no longer necessary after flex removal.
walterra May 12, 2020
5dfeff6
Merge branch 'ml-analytics-fix-data-grid-width' of github.com:walterr…
walterra May 12, 2020
d67b062
[ML] Tweak padding.
walterra May 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion x-pack/plugins/ml/public/application/data_frame_analytics/_index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
@import 'pages/analytics_exploration/components/regression_exploration/index';
@import 'pages/analytics_exploration/components/classification_exploration/index';
@import 'pages/analytics_management/components/analytics_list/index';
@import 'pages/analytics_management/components/create_analytics_form/index';
@import 'pages/analytics_management/components/create_analytics_flyout/index';
35 changes: 33 additions & 2 deletions ...lytics_exploration/components/classification_exploration/_classification_exploration.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,35 @@
.euiFormRow.mlDataFrameAnalyticsClassification__actualLabel {
padding-top: $euiSize * 4;
/*
Workaround for EuiDataGrid within a Flex Layout,
this tricks browsers treating the width as a px value instead of %
*/
.mlDataFrameAnalyticsClassification {
width: calc(100% - 0px);
Copy link
Contributor

@darnautov darnautov May 8, 2020

Choose a reason for hiding this comment

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

why do you need calc here? also, keep in mind it's not supported by IE11 properly https://caniuse.com/#feat=calc

Copy link
Contributor Author

@walterra walterra May 8, 2020

Choose a reason for hiding this comment

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

The calc is used to work around an issue with EuiDataGrid and percentage based width of the wrapping container when used in flex layouts. The original issue in the EUI repo is here: elastic/eui#2618

calc in IE has limitations but this one should work hopefully. I cannot test it at the moment because it seems Kibana is broken for IE11 on master. Will try again once I hear back from QA team.

Copy link
Contributor Author

@walterra walterra May 8, 2020

Choose a reason for hiding this comment

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

Managed to test in IE. Pushed some fixes in de5e1c0 and updated the PR description with before and after screenshots.

Copy link
Contributor Author

@walterra walterra May 12, 2020

Choose a reason for hiding this comment

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

After the refactor to not use flex anymore for the layout, the calc() is no longer necessary. Removed in 7905eec.

}

.mlDataFrameAnalyticsClassification__confusionMatrix {
padding: 0 5%;
}

/*
The following two classes are a workaround to avoid having EuiDataGrid in a flex layout
and just uses a legacy approach for a two column layout so we don't break IE11.
*/
.mlDataFrameAnalyticsClassification__confusionMatrix:after {
content: "";
display: table;
clear: both;
}

.mlDataFrameAnalyticsClassification__actualLabel {
float: left;
width: 70px;
padding-top: $euiSize * 4.2;
Copy link
Contributor

@darnautov darnautov May 8, 2020

Choose a reason for hiding this comment

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

why 4.2? 🤔

Copy link
Contributor Author

@walterra walterra May 8, 2020

Choose a reason for hiding this comment

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

It's to tweak the vertical alignment of "Actual label" against the first row of the data grid. The version before that PR used EuiFormRow + custom padding. I removed the form row because it was used out of context and adjusted the padding.

image

Copy link
Contributor

@darnautov darnautov May 11, 2020

Choose a reason for hiding this comment

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

have you tried to make it as :before pseudo-element for the header row to avoid this padding?

Copy link
Contributor Author

@walterra walterra May 12, 2020

Choose a reason for hiding this comment

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

Hm not sure that would be feasible to do: The header row is rendered by EuiDataGrid so that would mean either reusing some EUI classes and using a pseudo element would move the text to the SCSS where we cannot use translation (unless I move the style specification back into JSX?). Note this is just about adjusted an already existing padding value, it's not something this PR introduced; the main goal of this PR is to provide a fix for the broken data grid menu and fix IE. I changed the value to $euiSize * 4 + $euiSizeXS; to avoid having the comma in d67b062.

}

/*
Gives EuiDataGrid a min-width of 480px, otherwise the columns options will disappear if you hide all columns.
*/
.mlDataFrameAnalyticsClassification__dataGridMinWidth {
min-width: 480px;
width: calc(100% - 0px);
}
1 change: 0 additions & 1 deletion ...e_analytics/pages/analytics_exploration/components/classification_exploration/_index.scss
View file
Open in desktop

This file was deleted.

279 changes: 118 additions & 161 deletions ...tics/pages/analytics_exploration/components/classification_exploration/evaluate_panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

import './_classification_exploration.scss';

import React, { FC, useState, useEffect, Fragment } from 'react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
Expand All @@ -12,7 +14,6 @@ import {
EuiDataGrid,
EuiFlexGroup,
EuiFlexItem,
EuiFormRow,
EuiIconTip,
EuiPanel,
EuiSpacer,
Expand Down Expand Up @@ -46,8 +47,6 @@ import {
getTrailingControlColumns,
} from './column_data';

const defaultPanelWidth = 500;

interface Props {
jobConfig: DataFrameAnalyticsConfig;
jobStatus?: DATA_FRAME_TASK_STATE;
Expand Down Expand Up @@ -104,7 +103,6 @@ export const EvaluatePanel: FC<Props> = ({ jobConfig, jobStatus, searchQuery })
const [docsCount, setDocsCount] = useState<null | number>(null);
const [error, setError] = useState<null | string>(null);
const [dataSubsetTitle, setDataSubsetTitle] = useState<SUBSET_TITLE>(SUBSET_TITLE.ENTIRE);
const [panelWidth, setPanelWidth] = useState<number>(defaultPanelWidth);
// Column visibility
const [visibleColumns, setVisibleColumns] = useState(() =>
columns.map(({ id }: { id: string }) => id)
Expand Down Expand Up @@ -168,24 +166,6 @@ export const EvaluatePanel: FC<Props> = ({ jobConfig, jobStatus, searchQuery })
}
};

const resizeHandler = () => {
const tablePanelWidth: number =
document.getElementById('mlDataFrameAnalyticsTableResultsPanel')?.clientWidth ||
defaultPanelWidth;
// Keep the evaluate panel width slightly smaller than the results table
// to ensure results table can resize correctly. Temporary workaround DataGrid issue with flex
const newWidth = tablePanelWidth - 8;
setPanelWidth(newWidth);
};

useEffect(() => {
window.addEventListener('resize', resizeHandler);
resizeHandler();
return () => {
window.removeEventListener('resize', resizeHandler);
};
}, []);

useEffect(() => {
if (confusionMatrixData.length > 0) {
const { columns: derivedColumns, columnData } = getColumnData(confusionMatrixData);
Expand Down Expand Up @@ -310,158 +290,135 @@ export const EvaluatePanel: FC<Props> = ({ jobConfig, jobStatus, searchQuery })
return (
<EuiPanel
data-test-subj="mlDFAnalyticsClassificationExplorationEvaluatePanel"
style={{ width: `${panelWidth}px` }}
className="mlDataFrameAnalyticsClassification"
>
<EuiFlexGroup direction="column" gutterSize="s">
<EuiFlexItem>
<EuiFlexGroup alignItems="center" justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<EuiTitle size="xs">
<span>
{i18n.translate(
'xpack.ml.dataframe.analytics.classificationExploration.evaluateJobIdTitle',
{
defaultMessage: 'Evaluation of classification job ID {jobId}',
values: { jobId: jobConfig.id },
}
)}
</span>
</EuiTitle>
</EuiFlexItem>
{jobStatus !== undefined && (
<EuiFlexItem grow={false}>
<span>{getTaskStateBadge(jobStatus)}</span>
</EuiFlexItem>
)}
<EuiFlexItem>
<EuiSpacer />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonEmpty
target="_blank"
iconType="help"
iconSide="left"
color="primary"
href={`${ELASTIC_WEBSITE_URL}guide/en/machine-learning/${DOC_LINK_VERSION}/ml-dfanalytics-evaluate.html#ml-dfanalytics-classification`}
>
<div>
<EuiFlexGroup alignItems="center" justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<EuiTitle size="xs">
<span>
{i18n.translate(
'xpack.ml.dataframe.analytics.classificationExploration.classificationDocsLink',
'xpack.ml.dataframe.analytics.classificationExploration.evaluateJobIdTitle',
{
defaultMessage: 'Classification evaluation docs ',
defaultMessage: 'Evaluation of classification job ID {jobId}',
values: { jobId: jobConfig.id },
}
)}
</EuiButtonEmpty>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
{error !== null && (
<EuiFlexItem grow={false}>
<ErrorCallout error={error} />
</span>
</EuiTitle>
</EuiFlexItem>
)}
{error === null && (
<Fragment>
{jobStatus !== undefined && (
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="xs">
<EuiTitle size="xxs">
<span>{getHelpText(dataSubsetTitle)}</span>
</EuiTitle>
<EuiFlexItem grow={false}>
<EuiIconTip
anchorClassName="mlDataFrameAnalyticsClassificationInfoTooltip"
content={i18n.translate(
'xpack.ml.dataframe.analytics.classificationExploration.confusionMatrixTooltip',
{
defaultMessage:
'The multi-class confusion matrix contains the number of occurrences where the analysis classified data points correctly with their actual class as well as the number of occurrences where it misclassified them with another class',
}
)}
/>
</EuiFlexItem>
</EuiFlexGroup>
<span>{getTaskStateBadge(jobStatus)}</span>
</EuiFlexItem>
{docsCount !== null && (
)}
<EuiFlexItem />
<EuiFlexItem grow={false}>
<EuiButtonEmpty
target="_blank"
iconType="help"
iconSide="left"
color="primary"
href={`${ELASTIC_WEBSITE_URL}guide/en/machine-learning/${DOC_LINK_VERSION}/ml-dfanalytics-evaluate.html#ml-dfanalytics-classification`}
>
{i18n.translate(
Copy link
Contributor

@darnautov darnautov May 13, 2020

Choose a reason for hiding this comment

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

nit: could be replaced with FormattedMessage

'xpack.ml.dataframe.analytics.classificationExploration.classificationDocsLink',
{
defaultMessage: 'Classification evaluation docs ',
}
)}
</EuiButtonEmpty>
</EuiFlexItem>
</EuiFlexGroup>
</div>
{error !== null && <ErrorCallout error={error} />}
{error === null && (
<Fragment>
<div>
<EuiFlexGroup gutterSize="xs">
<EuiTitle size="xxs">
<span>{getHelpText(dataSubsetTitle)}</span>
</EuiTitle>
<EuiFlexItem grow={false}>
<EuiText size="xs" color="subdued">
<FormattedMessage
id="xpack.ml.dataframe.analytics.classificationExploration.generalizationDocsCount"
defaultMessage="{docsCount, plural, one {# doc} other {# docs}} evaluated"
values={{ docsCount }}
/>
</EuiText>
<EuiIconTip
anchorClassName="mlDataFrameAnalyticsClassificationInfoTooltip"
content={i18n.translate(
'xpack.ml.dataframe.analytics.classificationExploration.confusionMatrixTooltip',
{
defaultMessage:
'The multi-class confusion matrix contains the number of occurrences where the analysis classified data points correctly with their actual class as well as the number of occurrences where it misclassified them with another class',
}
)}
/>
</EuiFlexItem>
)}
{/* BEGIN TABLE ELEMENTS */}
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="s" style={{ paddingLeft: '5%', paddingRight: '5%' }}>
<EuiFlexItem grow={false}>
<EuiFormRow
className="mlDataFrameAnalyticsClassification__actualLabel"
helpText={i18n.translate(
'xpack.ml.dataframe.analytics.classificationExploration.confusionMatrixActualLabel',
</EuiFlexGroup>
</div>
{docsCount !== null && (
<EuiText size="xs" color="subdued">
<FormattedMessage
id="xpack.ml.dataframe.analytics.classificationExploration.generalizationDocsCount"
defaultMessage="{docsCount, plural, one {# doc} other {# docs}} evaluated"
values={{ docsCount }}
/>
</EuiText>
)}
{/* BEGIN TABLE ELEMENTS */}
<EuiSpacer size="m" />
<div className="mlDataFrameAnalyticsClassification__confusionMatrix">
<div className="mlDataFrameAnalyticsClassification__actualLabel">
<EuiText size="xs" color="subdued">
<FormattedMessage
id="xpack.ml.dataframe.analytics.classificationExploration.confusionMatrixActualLabel"
defaultMessage="Actual label"
/>
</EuiText>
</div>
<div className="mlDataFrameAnalyticsClassification__dataGridMinWidth">
{columns.length > 0 && columnsData.length > 0 && (
<>
<div>
<EuiText size="xs" color="subdued">
<FormattedMessage
id="xpack.ml.dataframe.analytics.classificationExploration.confusionMatrixPredictedLabel"
defaultMessage="Predicted label"
/>
</EuiText>
</div>
<EuiSpacer size="s" />
<EuiDataGrid
data-test-subj="mlDFAnalyticsClassificationExplorationConfusionMatrix"
aria-label={i18n.translate(
'xpack.ml.dataframe.analytics.classificationExploration.confusionMatrixLabel',
{
defaultMessage: 'Actual label',
defaultMessage: 'Classification confusion matrix',
}
)}
>
<Fragment />
</EuiFormRow>
</EuiFlexItem>
<EuiFlexItem grow={false}>
{columns.length > 0 && columnsData.length > 0 && (
<Fragment>
<EuiFlexGroup direction="column" justifyContent="center" gutterSize="s">
<EuiFlexItem grow={false}>
<EuiFormRow
helpText={i18n.translate(
'xpack.ml.dataframe.analytics.classificationExploration.confusionMatrixPredictedLabel',
{
defaultMessage: 'Predicted label',
}
)}
>
<Fragment />
</EuiFormRow>
</EuiFlexItem>
<EuiFlexItem grow={false} style={{ width: '90%' }}>
<EuiDataGrid
data-test-subj="mlDFAnalyticsClassificationExplorationConfusionMatrix"
aria-label={i18n.translate(
'xpack.ml.dataframe.analytics.classificationExploration.confusionMatrixLabel',
{
defaultMessage: 'Classification confusion matrix',
}
)}
columns={shownColumns}
columnVisibility={{ visibleColumns, setVisibleColumns }}
rowCount={rowCount}
renderCellValue={renderCellValue}
inMemory={{ level: 'sorting' }}
toolbarVisibility={{
showColumnSelector: true,
showStyleSelector: false,
showFullScreenSelector: false,
showSortSelector: false,
}}
popoverContents={popoverContents}
gridStyle={{ rowHover: 'none' }}
trailingControlColumns={
showTrailingColumns === true && showFullColumns === false
? getTrailingControlColumns(extraColumns, setShowFullColumns)
: undefined
}
/>
</EuiFlexItem>
</EuiFlexGroup>
</Fragment>
)}
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
</Fragment>
)}
{/* END TABLE ELEMENTS */}
</EuiFlexGroup>
columns={shownColumns}
columnVisibility={{ visibleColumns, setVisibleColumns }}
rowCount={rowCount}
renderCellValue={renderCellValue}
inMemory={{ level: 'sorting' }}
toolbarVisibility={{
showColumnSelector: true,
showStyleSelector: false,
showFullScreenSelector: false,
showSortSelector: false,
}}
popoverContents={popoverContents}
gridStyle={{ rowHover: 'none' }}
trailingControlColumns={
showTrailingColumns === true && showFullColumns === false
? getTrailingControlColumns(extraColumns, setShowFullColumns)
: undefined
}
/>
</>
)}
</div>
</div>
</Fragment>
)}
{/* END TABLE ELEMENTS */}
</EuiPanel>
);
};