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

walterra · 2020-05-08T08:09:30Z

Summary

Part of #64418.

Fixes an issue where hiding all columns for the confusion matrix would hide the menu item to enable columns again.
Replaces resizeHandler based workaround for EuiDataGrid within flex layouts with a CSS based workaround.
Moves inline styles to SCSS file and imports that file directly.

Fixes IE11 layout issues:

Before:

After:

Checklist

This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-05-08T08:09:32Z

Pinging @elastic/ml-ui (:ml)

darnautov · 2020-05-08T08:14:20Z

...analytics_exploration/components/classification_exploration/_classification_exploration.scss

-  padding-top: $euiSize * 4;
+/* Workaround for EuiDataGrid within a Flex Layout */
+.mlDataFrameAnalyticsClassification {
+  width: calc(100% - 0px);


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

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.

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

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

darnautov · 2020-05-08T08:15:48Z

...analytics_exploration/components/classification_exploration/_classification_exploration.scss

+
+.mlDataFrameAnalyticsClassification__actualLabel {
+  min-width: 70px;
+  padding-top: $euiSize * 4.2;


why 4.2? 🤔

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.

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

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.

walterra · 2020-05-11T07:40:26Z

@elasticmachine merge upstream

walterra · 2020-05-11T09:47:32Z

@elasticmachine merge upstream

peteharverson

Tested on Chrome and IE11 and LGTM

alvarezmelissa87

Gave this a test and LGTM ⚡

…a/kibana into ml-analytics-fix-data-grid-width

kibanamachine · 2020-05-12T16:40:40Z

💚 Build Succeeded

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

History

💚 Build #47326 succeeded 5dfeff6
💚 Build #46859 succeeded fc67d76
💛 Build #46833 was flaky 248bd34
💚 Build #46597 succeeded de5e1c0
💚 Build #46555 succeeded b5ebbd9

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

darnautov

LGTM 👍

darnautov · 2020-05-13T07:25:31Z

...alytics/pages/analytics_exploration/components/classification_exploration/evaluate_panel.tsx

+              color="primary"
+              href={`${ELASTIC_WEBSITE_URL}guide/en/machine-learning/${DOC_LINK_VERSION}/ml-dfanalytics-evaluate.html#ml-dfanalytics-classification`}
+            >
+              {i18n.translate(


nit: could be replaced with FormattedMessage

…stic#65818) - Fixes an issue where hiding all columns for the confusion matrix would hide the menu item to enable columns again. - Replaces resizeHandler based workaround for EuiDataGrid within flex layouts with a CSS based workaround. - Moves inline styles to SCSS file and imports that file directly. - Fixes IE11 layout issues.

) (#66355) - Fixes an issue where hiding all columns for the confusion matrix would hide the menu item to enable columns again. - Replaces resizeHandler based workaround for EuiDataGrid within flex layouts with a CSS based workaround. - Moves inline styles to SCSS file and imports that file directly. - Fixes IE11 layout issues.

) (#66354) - Fixes an issue where hiding all columns for the confusion matrix would hide the menu item to enable columns again. - Replaces resizeHandler based workaround for EuiDataGrid within flex layouts with a CSS based workaround. - Moves inline styles to SCSS file and imports that file directly. - Fixes IE11 layout issues.

* upstream/master: (223 commits) [Ingest] Support root level yaml variables in agent stream template (elastic#66120) [Snapshot Restore] Fix error when deleting snapshots behind reverse proxy (elastic#66147) [Lens] fix empty state for pie (elastic#66206) [APM] Improve e2e tests (elastic#66373) [ML] Data Frame Analytics: Fix steps to be named phases. (elastic#65855) [Discover] Encode context link filter part (elastic#63831) [APM] Scope APM alert creation to environment (elastic#65681) Move Kibana Usage collectors outside the telemetry plugin (elastic#65663) [ML] Data Frame Analytics: Fix confusion matrix data grid width. (elastic#65818) Switch to core application service (elastic#63443) Removes use of prefer_v2_templates (elastic#66316) [Maps] handle case where fit to bounds does not match any documents (elastic#66307) log error instead of throw (elastic#66254) [plugin-helpers] remove outdated postinstall task (elastic#66324) Spaces - migrate default space and enter space view to KP (elastic#66098) [APM] Change plugin id for `apm_oss` to `apmOss` (elastic#66164) [Maps] convert map_selectors to TS (elastic#65905) [uptime/usage-collector] add missing await (elastic#66079) [Ingest] Add additional attributes to the Datasources Saved Object (elastic#66127) [Endpoint]EMT-339: add new policy response schema (elastic#66264) ...

walterra added 2 commits May 8, 2020 09:47

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

5dd76c8

[ML] Styles cleanup.

b5ebbd9

walterra added bug Fixes for quality problems that affect the customer experience :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Data Frame Analytics ML data frame analytics features v7.8.0 v7.9.0 labels May 8, 2020

walterra requested review from darnautov and alvarezmelissa87 May 8, 2020 08:09

walterra requested review from a team as code owners May 8, 2020 08:09

walterra self-assigned this May 8, 2020

walterra mentioned this pull request May 8, 2020

[ML] EuiDataGrid Improvements Meta Issue #64418

Closed

3 tasks

darnautov reviewed May 8, 2020

View reviewed changes

[ML] Fix IE11 layout issues.

de5e1c0

walterra requested a review from peteharverson May 8, 2020 13:59

Merge branch 'master' into ml-analytics-fix-data-grid-width

248bd34

Merge branch 'master' into ml-analytics-fix-data-grid-width

fc67d76

peteharverson approved these changes May 11, 2020

View reviewed changes

alvarezmelissa87 approved these changes May 11, 2020

View reviewed changes

walterra added 4 commits May 12, 2020 16:01

Merge branch 'master' into ml-analytics-fix-data-grid-width

a6fd891

[ML] calc() no longer necessary after flex removal.

7905eec

Merge branch 'ml-analytics-fix-data-grid-width' of github.com:walterr…

5dfeff6

…a/kibana into ml-analytics-fix-data-grid-width

[ML] Tweak padding.

d67b062

darnautov approved these changes May 13, 2020

View reviewed changes

walterra merged commit a149915 into elastic:master May 13, 2020

walterra deleted the ml-analytics-fix-data-grid-width branch May 13, 2020 07:30

walterra mentioned this pull request May 13, 2020

[7.x] [ML] Data Frame Analytics: Fix confusion matrix data grid width. (#65818) #66354

Merged

walterra mentioned this pull request May 13, 2020

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

walterra commented May 8, 2020 •

edited

Loading

elasticmachine commented May 8, 2020

darnautov May 8, 2020

walterra May 8, 2020

walterra May 8, 2020

walterra May 12, 2020

darnautov May 8, 2020

walterra May 8, 2020

darnautov May 11, 2020

walterra May 12, 2020

walterra commented May 11, 2020

walterra commented May 11, 2020

peteharverson left a comment

alvarezmelissa87 left a comment

kibanamachine commented May 12, 2020

darnautov left a comment

darnautov May 13, 2020

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

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

Conversation

walterra commented May 8, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented May 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra commented May 11, 2020

walterra commented May 11, 2020

peteharverson left a comment

Choose a reason for hiding this comment

alvarezmelissa87 left a comment

Choose a reason for hiding this comment

kibanamachine commented May 12, 2020

💚 Build Succeeded

History

darnautov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra commented May 8, 2020 •

edited

Loading