Skip to content

Commit

Permalink
[ML] Removing scss override files for anomaly detection jobs (#152240)
Browse files Browse the repository at this point in the history
Removes the scss files which were being used to override various eui
styles or add styles to custom components.
Affect the anomaly detection jobs list and wizards.

This is not an in-depth refactor of our styles, and so some overrides
are still necessary in order to retain an identical UI, in these cases
the style overrides have been moved to inline emotion `css`.

Part of #140695
  • Loading branch information
jgowdyelastic authored Mar 2, 2023
1 parent 6dcdabc commit 0d04fd7
Show file tree
Hide file tree
Showing 62 changed files with 240 additions and 636 deletions.
30 changes: 0 additions & 30 deletions x-pack/plugins/ml/public/application/_app.scss

This file was deleted.

33 changes: 0 additions & 33 deletions x-pack/plugins/ml/public/application/_hacks.scss

This file was deleted.

12 changes: 0 additions & 12 deletions x-pack/plugins/ml/public/application/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,18 @@
// Protect the rest of Kibana from ML generic namespacing
// SASSTODO: Prefix ml selectors instead
.ml-app {
// App level
@import 'app';

// Sub applications
@import 'data_frame_analytics/index';
@import 'explorer/index'; // SASSTODO: This file needs to be rewritten
@import 'jobs/index'; // SASSTODO: This collection of sass files has multiple problems
@import 'overview/index';
@import 'timeseriesexplorer/index';

// Components
@import 'components/annotations/annotation_description_list/index'; // SASSTODO: This file overwrites EUI directly
@import 'components/anomalies_table/index'; // SASSTODO: This file overwrites EUI directly
@import 'components/color_range_legend/index';
@import 'components/controls/index';
@import 'components/entity_cell/index';
@import 'components/influencers_list/index';
@import 'components/items_grid/index';
@import 'components/job_selector/index';
@import 'components/loading_indicator/index'; // SASSTODO: This component should be replaced with EuiLoadingSpinner
@import 'components/rule_editor/index'; // SASSTODO: This file overwrites EUI directly
@import 'components/stats_bar/index';
@import 'components/ml_embedded_map/index';

// Hacks are last so they can overwrite anything above if needed
@import 'hacks';
}
Original file line number Diff line number Diff line change
@@ -1,45 +1,5 @@
// SASSTODO: This file has several direct EUI overwrites that need to be removed
.ml-anomalies-table {
.ml-icon-severity-critical,
.ml-icon-severity-major,
.ml-icon-severity-minor,
.ml-icon-severity-warning,
.ml-icon-severity-unknown {
color: inherit;
text-shadow: none;
}

// SASSTODO: Should only be three options, logic moved to the JS, where EuiIcon accepts a color
.ml-icon-severity-critical {
.euiIcon {
fill: $mlColorCriticalText;
}
}

.ml-icon-severity-major {
.euiIcon {
fill: $mlColorMajorText;
}
}

.ml-icon-severity-minor {
.euiIcon {
fill: $mlColorMinorText;
}
}

.ml-icon-severity-warning {
.euiIcon {
fill: $mlColorWarningText;
}
}

.ml-icon-severity-unknown {
.euiIcon {
fill: $mlColorUnknownText;
}
}

tr th:first-child,
tr td:first-child {
width: $euiSizeXL;
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ export const JobMessages: FC<JobMessagesProps> = ({
<>
<EuiSpacer size="s" />
<EuiInMemoryTable
className="job-messages-table"
items={messages}
columns={columns}
sorting={defaultSorting}
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,23 @@

import React, { FC } from 'react';

import { EuiLoadingChart, EuiSpacer } from '@elastic/eui';
import { EuiFlexGroup, EuiFlexItem, EuiLoadingChart, EuiSpacer } from '@elastic/eui';

export const LoadingIndicator: FC<{ height?: number; label?: string }> = ({ height, label }) => {
height = height ? +height : 100;
return (
<div
className="ml-loading-indicator"
style={{ height: `${height}px` }}
data-test-subj="mlLoadingIndicator"
>
<EuiLoadingChart size="xl" mono />
{label && (
<>
<EuiSpacer size="s" />
<div>{label}</div>
</>
)}
</div>
<EuiFlexGroup justifyContent="spaceEvenly">
<EuiFlexItem grow={false}>
<div style={{ height: `${height}px` }} data-test-subj="mlLoadingIndicator">
<EuiLoadingChart size="xl" mono />
{label && (
<>
<EuiSpacer size="s" />
<div>{label}</div>
</>
)}
</div>
</EuiFlexItem>
</EuiFlexGroup>
);
};

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,14 @@ export function MlEmbeddedMapComponent({
return (
<div
data-test-subj="mlEmbeddedMapContent"
className="mlEmbeddedMapContent"
css={{
width: '100%',
height: '100%',
display: 'flex',
flex: '1 1 100%',
zIndex: 1,
minHeight: 0, // Absolute must for Firefox to scroll contents
}}
ref={embeddableRoot}
/>
);
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React, { FC } from 'react';
import { useEuiTheme } from '@elastic/eui';

export interface StatsBarStat {
label: string;
Expand All @@ -18,8 +19,9 @@ interface StatProps {
}

export const Stat: FC<StatProps> = ({ stat }) => {
const { euiTheme } = useEuiTheme();
return (
<span className="stat">
<span css={{ marginRight: euiTheme.size.s }}>
<span>{stat.label}</span>:{' '}
<strong data-test-subj={stat['data-test-subj']}>{stat.value}</strong>
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React, { FC } from 'react';
import { useEuiTheme } from '@elastic/eui';
import { Stat, StatsBarStat } from './stat';

interface Stats {
Expand Down Expand Up @@ -37,9 +38,13 @@ interface StatsBarProps {
}

export const StatsBar: FC<StatsBarProps> = ({ stats, dataTestSub }) => {
const { euiTheme } = useEuiTheme();
const statsList = Object.keys(stats).map((k) => stats[k as StatsKey]);
return (
<div className="mlStatsBar" data-test-subj={dataTestSub}>
<div
css={{ padding: euiTheme.size.m, backgroundColor: euiTheme.colors.lightestShade }}
data-test-subj={dataTestSub}
>
{statsList
.filter((s: StatsBarStat) => s.show)
.map((s: StatsBarStat) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('ExplorerChart', () => {
// the directive just ends up being empty.
expect(wrapper.isEmptyRender()).toBeTruthy();
expect(wrapper.find('.content-wrapper')).toHaveLength(0);
expect(wrapper.find('.ml-loading-indicator .euiLoadingChart')).toHaveLength(0);
expect(wrapper.find('.euiLoadingChart')).toHaveLength(0);
});

test('Loading status active, no chart', () => {
Expand All @@ -84,7 +84,7 @@ describe('ExplorerChart', () => {

// test if the loading indicator is shown
// Added span because class appears twice with classNames and Emotion
expect(wrapper.find('.ml-loading-indicator span.euiLoadingChart')).toHaveLength(1);
expect(wrapper.find('span.euiLoadingChart')).toHaveLength(1);
});

// For the following tests the directive needs to be rendered in the actual DOM,
Expand Down Expand Up @@ -121,7 +121,7 @@ describe('ExplorerChart', () => {
const wrapper = init(mockChartData);

// the loading indicator should not be shown
expect(wrapper.find('.ml-loading-indicator .euiLoadingChart')).toHaveLength(0);
expect(wrapper.find('.euiLoadingChart')).toHaveLength(0);

// test if all expected elements are present
// need to use getDOMNode() because the chart is not rendered via react itself
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('ExplorerChart', () => {
// the directive just ends up being empty.
expect(wrapper.isEmptyRender()).toBeTruthy();
expect(wrapper.find('.content-wrapper')).toHaveLength(0);
expect(wrapper.find('.ml-loading-indicator .euiLoadingChart')).toHaveLength(0);
expect(wrapper.find('.euiLoadingChart')).toHaveLength(0);
});

test('Loading status active, no chart', () => {
Expand All @@ -87,7 +87,7 @@ describe('ExplorerChart', () => {

// test if the loading indicator is shown
// Added span because class appears twice with classNames and Emotion
expect(wrapper.find('.ml-loading-indicator span.euiLoadingChart')).toHaveLength(1);
expect(wrapper.find('span.euiLoadingChart')).toHaveLength(1);
});

// For the following tests the directive needs to be rendered in the actual DOM,
Expand Down Expand Up @@ -126,7 +126,7 @@ describe('ExplorerChart', () => {
const wrapper = init(mockChartData);

// the loading indicator should not be shown
expect(wrapper.find('.ml-loading-indicator .euiLoadingChart')).toHaveLength(0);
expect(wrapper.find('.euiLoadingChart')).toHaveLength(0);

// test if all expected elements are present
// need to use getDOMNode() because the chart is not rendered via react itself
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/ml/public/application/jobs/_index.scss

This file was deleted.

Loading

0 comments on commit 0d04fd7

Please sign in to comment.