Skip to content
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

[ML] Remove data frame analytics scss files #199572

Merged
merged 8 commits into from
Nov 18, 2024
Merged

[ML] Remove data frame analytics scss files #199572

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7bde084
remove dfa scss files
alvarezmelissa87 Nov 9, 2024
2687d61
Merge branch 'main' into ml-dfa-scss-updates
elasticmachine Nov 11, 2024
999a7c7
update legend css overrides
alvarezmelissa87 Nov 12, 2024
b8c999e
ensure job messages style only applies where intended
alvarezmelissa87 Nov 12, 2024
b985829
Merge branch 'main' into ml-dfa-scss-updates
elasticmachine Nov 14, 2024
9ffc9a0
use eui variable for size calc
alvarezmelissa87 Nov 14, 2024
beefc58
Merge branch 'main' into ml-dfa-scss-updates
elasticmachine Nov 14, 2024
90a1db0
Merge branch 'main' into ml-dfa-scss-updates
elasticmachine Nov 18, 2024
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
3 changes: 1 addition & 2 deletions x-pack/plugins/ml/public/application/_index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
// SASSTODO: Prefix ml selectors instead
.ml-app {
// Sub applications
@import 'data_frame_analytics/index';
@import 'explorer/index'; // SASSTODO: This file needs to be rewritten

// Components
Expand All @@ -16,4 +15,4 @@
@import 'components/job_selector/index';
@import 'components/rule_editor/index'; // SASSTODO: This file overwrites EUI directly

}
}
3 changes: 0 additions & 3 deletions x-pack/plugins/ml/public/application/data_frame_analytics/_index.scss
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions ...l/public/application/data_frame_analytics/pages/analytics_creation/components/_index.scss
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion ...e_analytics/pages/analytics_creation/components/back_to_list_panel/back_to_list_panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const BackToListPanel: FC = () => {
return (
<Fragment>
<EuiCard
className="dfAnalyticsCreationWizard__card"
css={{ width: '300px' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use an EUI size variable to calculate this.

icon={<EuiIcon size="xxl" type="list" />}
title={i18n.translate('xpack.ml.dataframe.analytics.create.analyticsListCardTitle', {
defaultMessage: 'Data Frame Analytics',
Expand Down
2 changes: 1 addition & 1 deletion ...e_analytics/pages/analytics_creation/components/view_results_panel/view_results_panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const ViewResultsPanel: FC<Props> = ({ jobId, analysisType }) => {
return (
<Fragment>
<EuiCard
className="dfAnalyticsCreationWizard__card"
css={{ width: '300px' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use an EUI size variable to calculate this.

Copy link
Contributor Author

@alvarezmelissa87 alvarezmelissa87 Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for taking a look, @andreadelrio 🙏
I'm not sure that would be the best solution here. The size variables I don't think are intended for this as even for the largest we'd have to multiply and do something like size.xl * 6.25, which doesn't seem ideal, especially considering we use set variables for larger values in other places throughout. 🤔 This would then be affected if/when the size variable changes, which I don't think we'd want. I think for larger set widths it's okay to keep it as is.

Could you explain why it would be a better practice to use the variable?

Copy link
Contributor

Choose a reason for hiding this comment

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

@alvarezmelissa87 In general we recommend not using any hardcoded values even for larger values. The idea is precisely that if the EUI variable were to changes (which is highly unlikely) all the layouts and containers in Kibana change their size according to the new value defined in the design system.

Here are some examples of this practice in our codebase. See:

image image image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andreadelrio - Thanks for the response - updated to use eui size variable in 9ffc9a0

icon={<EuiIcon size="xxl" type="visTable" />}
title={i18n.translate('xpack.ml.dataframe.analytics.create.viewResultsCardTitle', {
defaultMessage: 'View Results',
Expand Down
23 changes: 0 additions & 23 deletions ...rame_analytics/pages/analytics_management/components/analytics_list/_analytics_table.scss
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion ...ion/data_frame_analytics/pages/analytics_management/components/analytics_list/_index.scss
View file
Open in desktop

This file was deleted.

11 changes: 6 additions & 5 deletions ...lytics/pages/analytics_management/components/analytics_list/expanded_row_details_pane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,17 @@ export const OverallDetails: FC<{
}> = ({ overallDetails }) => (
<EuiFlexGroup alignItems="center" wrap data-test-subj={overallDetails.dataTestSubj}>
{overallDetails.items.map((item) => {
const key = item.title;
if (item.title === 'badge') {
return (
<EuiFlexItem grow={false}>
<EuiFlexItem grow={false} key={key}>
<EuiBetaBadge label={item.description} color="subdued" title={item.title} />
</EuiFlexItem>
);
}

return (
<EuiFlexItem grow={false}>
<EuiFlexItem grow={false} key={key}>
<EuiFlexGroup gutterSize="xs">
<EuiFlexItem grow={false}>
<EuiDescriptionListDescription className="descriptionListTitle">
Expand Down Expand Up @@ -82,7 +83,7 @@ export const Stats = ({ section }: { section: SectionConfig }) => (
<EuiFlexItem grow={false}>
<EuiFlexGroup>
{section.items.map((item) => (
<EuiFlexItem grow={false}>
<EuiFlexItem grow={false} key={item.title}>
<EuiDescriptionListDescription className="descriptionListTitle">
<EuiText size="xs">{item.title}</EuiText>
</EuiDescriptionListDescription>
Expand Down Expand Up @@ -200,10 +201,10 @@ export const ExpandedRowDetailsPane: FC<ExpandedRowDetailsPaneProps> = ({
</EuiTitle>
<EuiSpacer size="xs" />
{progress.items.map((item) => (
<>
<span key={item.title}>
{item.description}
<EuiSpacer size="s" />
</>
</span>
))}
</EuiFlexItem>
<EuiFlexItem grow={3}>
Expand Down
8 changes: 0 additions & 8 deletions ...tics/pages/analytics_management/components/analytics_list/expanded_row_messages_pane.scss
View file
Open in desktop

This file was deleted.

23 changes: 19 additions & 4 deletions ...ytics/pages/analytics_management/components/analytics_list/expanded_row_messages_pane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
* 2.0.
*/

import './expanded_row_messages_pane.scss';

import type { FC } from 'react';
import React, { useState, useEffect, useCallback } from 'react';
import React, { useState, useEffect, useCallback, useMemo } from 'react';
import { useEuiTheme } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { css } from '@emotion/react';
import { useMlApi } from '../../../../../contexts/kibana';
import { useRefreshAnalyticsList } from '../../../../common';
import { JobMessages } from '../../../../../components/job_messages';
Expand All @@ -23,6 +23,21 @@ interface Props {

export const ExpandedRowMessagesPane: FC<Props> = ({ analyticsId, dataTestSubj }) => {
const mlApi = useMlApi();
const {
euiTheme: { size },
} = useEuiTheme();

const cssOverride = useMemo(
() =>
css({
padding: `0 ${size.s} ${size.s} ${size.s}`,
'.euiTable': {
backgroundColor: 'transparent',
},
}),
[size.s]
);

const [messages, setMessages] = useState<JobMessage[]>([]);
const [isLoading, setIsLoading] = useState(false);
const [errorMessage, setErrorMessage] = useState('');
Expand Down Expand Up @@ -63,7 +78,7 @@ export const ExpandedRowMessagesPane: FC<Props> = ({ analyticsId, dataTestSubj }
useRefreshAnalyticsList({ onRefresh: getMessages });

return (
<div className="mlExpandedRowJobMessages" data-test-subj={dataTestSubj}>
<div data-test-subj={dataTestSubj} css={cssOverride}>
<JobMessages
messages={messages}
loading={isLoading}
Expand Down
10 changes: 7 additions & 3 deletions ...data_frame_analytics/pages/analytics_management/components/analytics_list/use_columns.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ enum TASK_STATE_COLOR {
stopping = 'hollow',
}

const badgeStyle = {
maxWidth: '100px',
};

export const getTaskStateBadge = (
state: DataFrameAnalyticsStats['state'],
failureReason?: DataFrameAnalyticsStats['failure_reason']
Expand All @@ -58,22 +62,22 @@ export const getTaskStateBadge = (
if (isDataFrameAnalyticsFailed(state) && failureReason !== undefined) {
return (
<EuiToolTip content={failureReason}>
<EuiBadge className="mlTaskStateBadge" color={color}>
<EuiBadge css={badgeStyle} color={color}>
{state}
</EuiBadge>
</EuiToolTip>
);
}

return (
<EuiBadge className="mlTaskStateBadge" color={color}>
<EuiBadge css={badgeStyle} color={color}>
{state}
</EuiBadge>
);
};

export const getJobTypeBadge = (jobType: string) => (
<EuiBadge className="mlTaskStateBadge" color="hollow">
<EuiBadge css={badgeStyle} color="hollow">
{jobType}
</EuiBadge>
);
Expand Down
1 change: 0 additions & 1 deletion ...k/plugins/ml/public/application/data_frame_analytics/pages/job_map/components/_index.scss
View file
Open in desktop

This file was deleted.

56 changes: 0 additions & 56 deletions .../plugins/ml/public/application/data_frame_analytics/pages/job_map/components/_legend.scss
View file
Open in desktop

This file was deleted.

Loading