Skip to content

Commit d4a2c79

Browse files
[ML] Transforms/DFA: Refactor list action buttons so modals won't unmount after button click. (#70555) (#71050)
Related to #70383 and #63455. Refactors the action buttons of the transform and data frame analytics jobs list: Previously custom actions included state and JSX for e.g. confirmation modals. Problem with that: If the actions list popover hides, the modal would unmount too. Since EUI's behaviour will change with the release/merge of #70383, we needed a refactor that solves that issue right now. With this PR, state management for UI behaviour that follows after a button click like the confirmation modals was moved to a custom hook which is part of the outer level of the buttons itself. The modal now also gets mounted on the outer level. This way we won't loose the modals state and DOM rendering when the action button hides. Note that this PR doesn't fix the nested buttons issue (#63455) yet. For that we need EUI issue #70383 to be in Kibana which will arrive with EUI v26.3.0 via #70243. So there will be one follow up to that which will focus on getting rid of the nested button structure. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent 64f9176 commit d4a2c79

File tree

66 files changed

+1523
-1108
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

66 files changed

+1523
-1108
lines changed

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/components/classification_exploration/evaluate_panel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
DataFrameAnalyticsConfig,
3131
} from '../../../../common';
3232
import { isKeywordAndTextType } from '../../../../common/fields';
33-
import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/columns';
33+
import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/use_columns';
3434
import { DATA_FRAME_TASK_STATE } from '../../../analytics_management/components/analytics_list/common';
3535
import {
3636
isResultsSearchBoolQuery,

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/components/exploration_results_table/exploration_results_table.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
SEARCH_SIZE,
3030
defaultSearchQuery,
3131
} from '../../../../common';
32-
import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/columns';
32+
import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/use_columns';
3333
import { DATA_FRAME_TASK_STATE } from '../../../analytics_management/components/analytics_list/common';
3434
import { ExplorationTitle } from '../exploration_title';
3535
import { ExplorationQueryBar } from '../exploration_query_bar';

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/components/outlier_exploration/outlier_exploration.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { getToastNotifications } from '../../../../../util/dependency_cache';
2929

3030
import { defaultSearchQuery, useResultsViewConfig, INDEX_STATUS } from '../../../../common';
3131

32-
import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/columns';
32+
import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/use_columns';
3333

3434
import { ExplorationQueryBar } from '../exploration_query_bar';
3535
import { ExplorationTitle } from '../exploration_title';

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/components/regression_exploration/evaluate_panel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
Eval,
2828
DataFrameAnalyticsConfig,
2929
} from '../../../../common';
30-
import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/columns';
30+
import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/use_columns';
3131
import { DATA_FRAME_TASK_STATE } from '../../../analytics_management/components/analytics_list/common';
3232
import { EvaluateStat } from './evaluate_stat';
3333
import {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { isAdvancedConfig } from './action_clone';
7+
import { isAdvancedConfig } from './clone_button';
88

99
describe('Analytics job clone action', () => {
1010
describe('isAdvancedConfig', () => {
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES,
2020
} from '../../hooks/use_create_analytics_form';
2121
import { State } from '../../hooks/use_create_analytics_form/state';
22-
import { DataFrameAnalyticsListRow } from './common';
22+
import { DataFrameAnalyticsListRow } from '../analytics_list/common';
2323
import { checkPermission } from '../../../../../capabilities/check_capabilities';
2424
import { extractErrorMessage } from '../../../../../../../common/util/errors';
2525

@@ -343,7 +343,7 @@ export function getCloneAction(createAnalyticsForm: CreateAnalyticsFormProps) {
343343
};
344344
}
345345

346-
interface CloneActionProps {
346+
interface CloneButtonProps {
347347
item: DataFrameAnalyticsListRow;
348348
createAnalyticsForm: CreateAnalyticsFormProps;
349349
}
@@ -353,7 +353,7 @@ interface CloneActionProps {
353353
* Replace with {@link getCloneAction} as soon as all the actions are refactored
354354
* to support EuiContext with a valid DOM structure without nested buttons.
355355
*/
356-
export const CloneAction: FC<CloneActionProps> = ({ createAnalyticsForm, item }) => {
356+
export const CloneButton: FC<CloneButtonProps> = ({ createAnalyticsForm, item }) => {
357357
const canCreateDataFrameAnalytics: boolean = checkPermission('canCreateDataFrameAnalytics');
358358

359359
const buttonText = i18n.translate('xpack.ml.dataframe.analyticsList.cloneJobButtonLabel', {
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
export {
8+
extractCloningConfig,
9+
isAdvancedConfig,
10+
CloneButton,
11+
CloneDataFrameAnalyticsConfig,
12+
} from './clone_button';
Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@
77
import React from 'react';
88
import { fireEvent, render } from '@testing-library/react';
99
import * as CheckPrivilige from '../../../../../capabilities/check_capabilities';
10-
import mockAnalyticsListItem from './__mocks__/analytics_list_item.json';
11-
import { DeleteAction } from './action_delete';
10+
import mockAnalyticsListItem from '../analytics_list/__mocks__/analytics_list_item.json';
1211
import { I18nProvider } from '@kbn/i18n/react';
1312
import {
1413
coreMock as mockCoreServices,
1514
i18nServiceMock,
1615
} from '../../../../../../../../../../src/core/public/mocks';
1716

17+
import { DeleteButton } from './delete_button';
18+
import { DeleteButtonModal } from './delete_button_modal';
19+
import { useDeleteAction } from './use_delete_action';
20+
1821
jest.mock('../../../../../capabilities/check_capabilities', () => ({
1922
checkPermission: jest.fn(() => false),
2023
createPermissionFailureMessage: jest.fn(),
@@ -41,14 +44,18 @@ describe('DeleteAction', () => {
4144
});
4245

4346
test('When canDeleteDataFrameAnalytics permission is false, button should be disabled.', () => {
44-
const { getByTestId } = render(<DeleteAction item={mockAnalyticsListItem} />);
47+
const { getByTestId } = render(
48+
<DeleteButton item={mockAnalyticsListItem} onClick={() => {}} />
49+
);
4550
expect(getByTestId('mlAnalyticsJobDeleteButton')).toHaveAttribute('disabled');
4651
});
4752

4853
test('When canDeleteDataFrameAnalytics permission is true, button should not be disabled.', () => {
4954
const mock = jest.spyOn(CheckPrivilige, 'checkPermission');
5055
mock.mockImplementation((p) => p === 'canDeleteDataFrameAnalytics');
51-
const { getByTestId } = render(<DeleteAction item={mockAnalyticsListItem} />);
56+
const { getByTestId } = render(
57+
<DeleteButton item={mockAnalyticsListItem} onClick={() => {}} />
58+
);
5259

5360
expect(getByTestId('mlAnalyticsJobDeleteButton')).not.toHaveAttribute('disabled');
5461

@@ -57,11 +64,12 @@ describe('DeleteAction', () => {
5764

5865
test('When job is running, delete button should be disabled.', () => {
5966
const { getByTestId } = render(
60-
<DeleteAction
67+
<DeleteButton
6168
item={{
6269
...mockAnalyticsListItem,
6370
stats: { state: 'started' },
6471
}}
72+
onClick={() => {}}
6573
/>
6674
);
6775

@@ -72,9 +80,21 @@ describe('DeleteAction', () => {
7280
test('should allow to delete target index by default.', () => {
7381
const mock = jest.spyOn(CheckPrivilige, 'checkPermission');
7482
mock.mockImplementation((p) => p === 'canDeleteDataFrameAnalytics');
83+
84+
const TestComponent = () => {
85+
const deleteAction = useDeleteAction();
86+
87+
return (
88+
<>
89+
{deleteAction.isModalVisible && <DeleteButtonModal {...deleteAction} />}
90+
<DeleteButton item={mockAnalyticsListItem} onClick={deleteAction.openModal} />
91+
</>
92+
);
93+
};
94+
7595
const { getByTestId, queryByTestId } = render(
7696
<I18nProvider>
77-
<DeleteAction item={mockAnalyticsListItem} />
97+
<TestComponent />
7898
</I18nProvider>
7999
);
80100
const deleteButton = getByTestId('mlAnalyticsJobDeleteButton');
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import React, { FC } from 'react';
8+
import { i18n } from '@kbn/i18n';
9+
import { EuiIcon, EuiLink, EuiToolTip } from '@elastic/eui';
10+
import {
11+
checkPermission,
12+
createPermissionFailureMessage,
13+
} from '../../../../../capabilities/check_capabilities';
14+
import { isDataFrameAnalyticsRunning, DataFrameAnalyticsListRow } from '../analytics_list/common';
15+
16+
interface DeleteButtonProps {
17+
item: DataFrameAnalyticsListRow;
18+
onClick: (item: DataFrameAnalyticsListRow) => void;
19+
}
20+
21+
export const DeleteButton: FC<DeleteButtonProps> = ({ item, onClick }) => {
22+
const disabled = isDataFrameAnalyticsRunning(item.stats.state);
23+
const canDeleteDataFrameAnalytics: boolean = checkPermission('canDeleteDataFrameAnalytics');
24+
25+
const buttonDeleteText = i18n.translate('xpack.ml.dataframe.analyticsList.deleteActionName', {
26+
defaultMessage: 'Delete',
27+
});
28+
29+
const buttonDisabled = disabled || !canDeleteDataFrameAnalytics;
30+
let deleteButton = (
31+
<EuiLink
32+
data-test-subj="mlAnalyticsJobDeleteButton"
33+
color={buttonDisabled ? 'subdued' : 'text'}
34+
disabled={buttonDisabled}
35+
onClick={buttonDisabled ? undefined : () => onClick(item)}
36+
aria-label={buttonDeleteText}
37+
style={{ padding: 0 }}
38+
>
39+
<EuiIcon type="trash" /> {buttonDeleteText}
40+
</EuiLink>
41+
);
42+
43+
if (disabled || !canDeleteDataFrameAnalytics) {
44+
deleteButton = (
45+
<EuiToolTip
46+
position="top"
47+
content={
48+
disabled
49+
? i18n.translate(
50+
'xpack.ml.dataframe.analyticsList.deleteActionDisabledToolTipContent',
51+
{
52+
defaultMessage: 'Stop the data frame analytics job in order to delete it.',
53+
}
54+
)
55+
: createPermissionFailureMessage('canStartStopDataFrameAnalytics')
56+
}
57+
>
58+
{deleteButton}
59+
</EuiToolTip>
60+
);
61+
}
62+
63+
return deleteButton;
64+
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import React, { FC } from 'react';
8+
import { i18n } from '@kbn/i18n';
9+
import {
10+
EuiConfirmModal,
11+
EuiOverlayMask,
12+
EuiSwitch,
13+
EuiFlexGroup,
14+
EuiFlexItem,
15+
EUI_MODAL_CONFIRM_BUTTON,
16+
} from '@elastic/eui';
17+
import { FormattedMessage } from '@kbn/i18n/react';
18+
19+
import { DeleteAction } from './use_delete_action';
20+
21+
export const DeleteButtonModal: FC<DeleteAction> = ({
22+
closeModal,
23+
deleteAndCloseModal,
24+
deleteTargetIndex,
25+
deleteIndexPattern,
26+
indexPatternExists,
27+
item,
28+
toggleDeleteIndex,
29+
toggleDeleteIndexPattern,
30+
userCanDeleteIndex,
31+
}) => {
32+
if (item === undefined) {
33+
return null;
34+
}
35+
36+
const indexName = item.config.dest.index;
37+
38+
return (
39+
<EuiOverlayMask data-test-subj="mlAnalyticsJobDeleteOverlay">
40+
<EuiConfirmModal
41+
data-test-subj="mlAnalyticsJobDeleteModal"
42+
title={i18n.translate('xpack.ml.dataframe.analyticsList.deleteModalTitle', {
43+
defaultMessage: 'Delete {analyticsId}',
44+
values: { analyticsId: item.config.id },
45+
})}
46+
onCancel={closeModal}
47+
onConfirm={deleteAndCloseModal}
48+
cancelButtonText={i18n.translate(
49+
'xpack.ml.dataframe.analyticsList.deleteModalCancelButton',
50+
{
51+
defaultMessage: 'Cancel',
52+
}
53+
)}
54+
confirmButtonText={i18n.translate(
55+
'xpack.ml.dataframe.analyticsList.deleteModalDeleteButton',
56+
{
57+
defaultMessage: 'Delete',
58+
}
59+
)}
60+
defaultFocusedButton={EUI_MODAL_CONFIRM_BUTTON}
61+
buttonColor="danger"
62+
>
63+
<p>
64+
<FormattedMessage
65+
id="xpack.ml.dataframe.analyticsList.deleteModalBody"
66+
defaultMessage="Are you sure you want to delete this analytics job?"
67+
/>
68+
</p>
69+
70+
<EuiFlexGroup direction="column" gutterSize="none">
71+
<EuiFlexItem>
72+
{userCanDeleteIndex && (
73+
<EuiSwitch
74+
data-test-subj="mlAnalyticsJobDeleteIndexSwitch"
75+
style={{ paddingBottom: 10 }}
76+
label={i18n.translate(
77+
'xpack.ml.dataframe.analyticsList.deleteDestinationIndexTitle',
78+
{
79+
defaultMessage: 'Delete destination index {indexName}',
80+
values: { indexName },
81+
}
82+
)}
83+
checked={deleteTargetIndex}
84+
onChange={toggleDeleteIndex}
85+
/>
86+
)}
87+
</EuiFlexItem>
88+
<EuiFlexItem>
89+
{userCanDeleteIndex && indexPatternExists && (
90+
<EuiSwitch
91+
data-test-subj="mlAnalyticsJobDeleteIndexPatternSwitch"
92+
label={i18n.translate(
93+
'xpack.ml.dataframe.analyticsList.deleteTargetIndexPatternTitle',
94+
{
95+
defaultMessage: 'Delete index pattern {indexPattern}',
96+
values: { indexPattern: indexName },
97+
}
98+
)}
99+
checked={deleteIndexPattern}
100+
onChange={toggleDeleteIndexPattern}
101+
/>
102+
)}
103+
</EuiFlexItem>
104+
</EuiFlexGroup>
105+
</EuiConfirmModal>
106+
</EuiOverlayMask>
107+
);
108+
};

0 commit comments

Comments
 (0)