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.

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] DF Analytics / Transforms: Fix job row actions menu invalid DOM nesting warning #74499

Merged
merged 45 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
56f847d
eui to 27.1.0
thompsongl Jul 21, 2020
6e88169
eui to 27.2.0
thompsongl Jul 22, 2020
4d1fb82
buttoniconside type
thompsongl Jul 22, 2020
006b6fd
euiselectable type
thompsongl Jul 22, 2020
1bb3fa0
update onScroll callback and polyfill size references
thompsongl Jul 22, 2020
d97395c
findTestSubject ts
thompsongl Jul 22, 2020
9aa61d4
buttoncontent and collapsiblenav src snapshot updates
thompsongl Jul 22, 2020
5ccbd18
update prop retrieval
thompsongl Jul 22, 2020
810d4c1
xpack snapshots
thompsongl Jul 22, 2020
8cb46b5
jest updates
thompsongl Jul 23, 2020
92e92e5
Merge branch 'master' into eui/27
thompsongl Jul 23, 2020
e254e4c
Merge branch 'master' into eui/27
thompsongl Jul 27, 2020
d91ca43
type fixes
thompsongl Jul 27, 2020
5d20471
Merge branch 'master' into eui/27
thompsongl Jul 27, 2020
5bfcae2
more snapshots
thompsongl Jul 27, 2020
74ba69d
Merge branch 'master' into eui/27
thompsongl Jul 27, 2020
b9ba0b1
virtual list changes
thompsongl Jul 27, 2020
a17cc17
more virtualization changes
thompsongl Jul 27, 2020
3912ad2
Merge branch 'master' into eui/27
thompsongl Jul 28, 2020
2537aa9
merge
thompsongl Jul 28, 2020
b1d6710
Merge branch 'master' into eui/27
thompsongl Jul 28, 2020
79cc7fe
fix functional tests
thompsongl Jul 28, 2020
9b644b2
Merge branch 'master' into eui/27
thompsongl Jul 28, 2020
5b72e28
Merge branch 'master' into eui/27
thompsongl Jul 29, 2020
2f84eed
data-test-subj for indexPatter-switcher
thompsongl Jul 29, 2020
954e63c
Merge branch 'master' into eui/27
thompsongl Jul 29, 2020
2791b0c
storyshots
thompsongl Jul 29, 2020
6b92ad9
eui to 27.3.1
thompsongl Jul 31, 2020
735fb2e
Merge branch 'master' into eui/27
thompsongl Jul 31, 2020
c03ea3a
Merge branch 'master' into eui/27
thompsongl Aug 1, 2020
fc51e0a
Merge branch 'master' into ml-fix-list-action-buttons-3
walterra Aug 5, 2020
760e4b7
[ML] Fix nested button issue for transforms list.
walterra Aug 6, 2020
fe5ede2
[ML] Fix nested button issue for analytics list.
walterra Aug 6, 2020
65a4091
Merge branch 'master' into ml-fix-list-action-buttons-3
walterra Aug 17, 2020
e76996a
[ML] transform components naming fixed.
walterra Aug 17, 2020
dc8b6e5
[ML] Fix translations.
walterra Aug 17, 2020
ab1de2e
[ML] Fix transform bulk actions. Fixes imports.
walterra Aug 17, 2020
aaa9273
[ML] Analytics list action renaming.
walterra Aug 17, 2020
269c223
Merge branch 'master' into ml-fix-list-action-buttons-3
walterra Aug 18, 2020
74071d9
[ML] Fix comment typo.
walterra Aug 18, 2020
c9fc1b2
[ML] Consolidate button icons.
walterra Aug 18, 2020
3158d8d
[ML] Fix imports and types.
walterra Aug 18, 2020
f440ff5
[ML] Fix translations.
walterra Aug 18, 2020
a54d94f
[ML] Change icons for view-series and SMV-button to visLine.
walterra Aug 19, 2020
274e106
[ML] Remove obsolete comment.
walterra Aug 19, 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
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ export class AnnotationsTable extends Component {
<EuiButtonIcon
onClick={() => this.openSingleMetricView(annotation)}
disabled={!isDrillDownAvailable}
iconType="stats"
iconType="visLine"
aria-label={openInSingleMetricViewerAriaLabelText}
/>
</EuiToolTip>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const AnomalyResultsViewSelector: FC<Props> = ({ viewId }) => {
label: i18n.translate('xpack.ml.anomalyResultsViewSelector.anomalyExplorerLabel', {
defaultMessage: 'View results in the Anomaly Explorer',
}),
iconType: 'tableOfContents',
iconType: 'visTable',
value: 'explorer',
'data-test-subj': 'mlAnomalyResultsViewSelectorExplorer',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const ViewResultsPanel: FC<Props> = ({ jobId, analysisType }) => {
<Fragment>
<EuiCard
className="dfAnalyticsCreationWizard__card"
icon={<EuiIcon size="xxl" type="tableDensityNormal" />}
icon={<EuiIcon size="xxl" type="visTable" />}
title={i18n.translate('xpack.ml.dataframe.analytics.create.viewResultsCardTitle', {
defaultMessage: 'View Results',
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { isAdvancedConfig } from './clone_button';
import { isAdvancedConfig } from './clone_action_name';

describe('Analytics job clone action', () => {
describe('isAdvancedConfig', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { EuiButtonEmpty, EuiToolTip } from '@elastic/eui';
import { EuiToolTip } from '@elastic/eui';
import React, { FC } from 'react';
import { isEqual, cloneDeep } from 'lodash';
import { i18n } from '@kbn/i18n';
Expand All @@ -14,10 +14,7 @@ import { DataFrameAnalyticsConfig, isOutlierAnalysis } from '../../../../common'
import { isClassificationAnalysis, isRegressionAnalysis } from '../../../../common/analytics';
import { DEFAULT_RESULTS_FIELD } from '../../../../common/constants';
import { useMlKibana, useNavigateToPath } from '../../../../../contexts/kibana';
import {
CreateAnalyticsFormProps,
DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES,
} from '../../hooks/use_create_analytics_form';
import { DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES } from '../../hooks/use_create_analytics_form';
import { State } from '../../hooks/use_create_analytics_form/state';
import { DataFrameAnalyticsListRow } from '../analytics_list/common';
import { extractErrorMessage } from '../../../../../../../common/util/errors';
Expand Down Expand Up @@ -328,25 +325,12 @@ export function extractCloningConfig({
}) as unknown) as CloneDataFrameAnalyticsConfig;
}

const buttonText = i18n.translate('xpack.ml.dataframe.analyticsList.cloneJobButtonLabel', {
defaultMessage: 'Clone job',
});

export function getCloneAction(createAnalyticsForm: CreateAnalyticsFormProps) {
const { actions } = createAnalyticsForm;

const onClick = async (item: DeepReadonly<DataFrameAnalyticsListRow>) => {
await actions.setJobClone(item.config);
};

return {
name: buttonText,
description: buttonText,
icon: 'copy',
onClick,
'data-test-subj': 'mlAnalyticsJobCloneButton',
};
}
export const cloneActionNameText = i18n.translate(
'xpack.ml.dataframe.analyticsList.cloneActionNameText',
{
defaultMessage: 'Clone',
}
);

export const useNavigateToWizardWithClonedJob = () => {
const {
Expand Down Expand Up @@ -405,32 +389,16 @@ export const useNavigateToWizardWithClonedJob = () => {
};
};

interface CloneButtonProps {
interface CloneActionNameProps {
isDisabled: boolean;
onClick: () => void;
}

/**
* Temp component to have Clone job button with the same look as the other actions.
* Replace with {@link getCloneAction} as soon as all the actions are refactored
* to support EuiContext with a valid DOM structure without nested buttons.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

we probably don't need this comment anymore

export const CloneButton: FC<CloneButtonProps> = ({ isDisabled, onClick }) => {
const button = (
<EuiButtonEmpty
aria-label={buttonText}
color="text"
data-test-subj="mlAnalyticsJobCloneButton"
flush="left"
iconType="copy"
isDisabled={isDisabled}
onClick={onClick}
size="xs"
>
{buttonText}
</EuiButtonEmpty>
);

export const CloneActionName: FC<CloneActionNameProps> = ({ isDisabled }) => {
if (isDisabled) {
return (
<EuiToolTip
Expand All @@ -439,10 +407,10 @@ export const CloneButton: FC<CloneButtonProps> = ({ isDisabled, onClick }) => {
defaultMessage: 'You do not have permission to clone analytics jobs.',
})}
>
{button}
<>{cloneActionNameText}</>
</EuiToolTip>
);
}

return button;
return <>{cloneActionNameText}</>;
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
export {
extractCloningConfig,
isAdvancedConfig,
useNavigateToWizardWithClonedJob,
CloneButton,
CloneDataFrameAnalyticsConfig,
} from './clone_button';
} from './clone_action_name';
export { useCloneAction } from './use_clone_action';
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useCallback, useMemo } from 'react';

import { DataFrameAnalyticsListAction, DataFrameAnalyticsListRow } from '../analytics_list/common';

import {
cloneActionNameText,
useNavigateToWizardWithClonedJob,
CloneActionName,
} from './clone_action_name';

export type CloneAction = ReturnType<typeof useCloneAction>;
export const useCloneAction = (canCreateDataFrameAnalytics: boolean) => {
const navigateToWizardWithClonedJob = useNavigateToWizardWithClonedJob();

const clickHandler = useCallback((item: DataFrameAnalyticsListRow) => {
navigateToWizardWithClonedJob(item);
}, []);

const action: DataFrameAnalyticsListAction = useMemo(
() => ({
name: (item: DataFrameAnalyticsListRow) => (
<CloneActionName isDisabled={!canCreateDataFrameAnalytics} />
),
enabled: () => canCreateDataFrameAnalytics,
description: cloneActionNameText,
icon: 'copy',
type: 'icon',
onClick: clickHandler,
'data-test-subj': 'mlAnalyticsJobCloneButton',
}),
[]
);

return { action };
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { FormattedMessage } from '@kbn/i18n/react';

import { DeleteAction } from './use_delete_action';

export const DeleteButtonModal: FC<DeleteAction> = ({
export const DeleteActionModal: FC<DeleteAction> = ({
closeModal,
deleteAndCloseModal,
deleteTargetIndex,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import {
i18nServiceMock,
} from '../../../../../../../../../../src/core/public/mocks';

import { DeleteButton } from './delete_button';
import { DeleteButtonModal } from './delete_button_modal';
import { DeleteActionName } from './delete_action_name';
import { DeleteActionModal } from './delete_action_modal';
import { useDeleteAction } from './use_delete_action';

jest.mock('../../../../../capabilities/check_capabilities', () => ({
Expand Down Expand Up @@ -49,38 +49,39 @@ describe('DeleteAction', () => {
jest.clearAllMocks();
});

test('When isDisabled prop is true, inner button should be disabled.', () => {
const { getByTestId } = render(
<DeleteButton isDisabled={true} item={mockAnalyticsListItem} onClick={() => {}} />
it('should display a tooltip when isDisabled prop is true.', () => {
const { container } = render(
<DeleteActionName isDisabled={true} item={mockAnalyticsListItem} />
);

expect(getByTestId('mlAnalyticsJobDeleteButton')).toHaveAttribute('disabled');
expect(container.querySelector('.euiToolTipAnchor')).toBeInTheDocument();
});

test('When isDisabled prop is true, inner button should not be disabled.', () => {
const { getByTestId } = render(
<DeleteButton isDisabled={false} item={mockAnalyticsListItem} onClick={() => {}} />
it('should not display a tooltip when isDisabled prop is false.', () => {
const { container } = render(
<DeleteActionName isDisabled={false} item={mockAnalyticsListItem} />
);

expect(getByTestId('mlAnalyticsJobDeleteButton')).not.toHaveAttribute('disabled');
expect(container.querySelector('.euiToolTipAnchor')).not.toBeInTheDocument();
});

describe('When delete model is open', () => {
test('should allow to delete target index by default.', () => {
it('should allow to delete target index by default.', () => {
const mock = jest.spyOn(CheckPrivilige, 'checkPermission');
mock.mockImplementation((p) => p === 'canDeleteDataFrameAnalytics');

const TestComponent = () => {
const deleteAction = useDeleteAction();
const deleteAction = useDeleteAction(true);

return (
<>
{deleteAction.isModalVisible && <DeleteButtonModal {...deleteAction} />}
<DeleteButton
isDisabled={false}
item={mockAnalyticsListItem}
{deleteAction.isModalVisible && <DeleteActionModal {...deleteAction} />}
<button
data-test-subj="mlAnalyticsJobDeleteButton"
onClick={() => deleteAction.openModal(mockAnalyticsListItem)}
/>
>
<DeleteActionName isDisabled={false} item={mockAnalyticsListItem} />
</button>
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,23 @@

import React, { FC } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiButtonEmpty, EuiToolTip } from '@elastic/eui';
import { EuiToolTip } from '@elastic/eui';
import { createPermissionFailureMessage } from '../../../../../capabilities/check_capabilities';
import { isDataFrameAnalyticsRunning, DataFrameAnalyticsListRow } from '../analytics_list/common';

const buttonText = i18n.translate('xpack.ml.dataframe.analyticsList.deleteActionName', {
defaultMessage: 'Delete',
});
export const deleteActionNameText = i18n.translate(
'xpack.ml.dataframe.analyticsList.deleteActionNameText',
{
defaultMessage: 'Delete',
}
);

interface DeleteButtonProps {
interface DeleteActionNameProps {
isDisabled: boolean;
item: DataFrameAnalyticsListRow;
onClick: () => void;
}

export const DeleteButton: FC<DeleteButtonProps> = ({ isDisabled, item, onClick }) => {
const button = (
<EuiButtonEmpty
aria-label={buttonText}
color="text"
data-test-subj="mlAnalyticsJobDeleteButton"
flush="left"
iconType="trash"
isDisabled={isDisabled}
onClick={onClick}
size="xs"
>
{buttonText}
</EuiButtonEmpty>
);

export const DeleteActionName: FC<DeleteActionNameProps> = ({ isDisabled, item }) => {
if (isDisabled) {
return (
<EuiToolTip
Expand All @@ -51,10 +38,10 @@ export const DeleteButton: FC<DeleteButtonProps> = ({ isDisabled, item, onClick
: createPermissionFailureMessage('canStartStopDataFrameAnalytics')
}
>
{button}
<>{deleteActionNameText}</>
</EuiToolTip>
);
}

return button;
return <>{deleteActionNameText}</>;
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { DeleteButton } from './delete_button';
export { DeleteButtonModal } from './delete_button_modal';
export { DeleteActionName } from './delete_action_name';
export { DeleteActionModal } from './delete_action_modal';
export { useDeleteAction } from './use_delete_action';
Loading