[ML] DF Analytics / Transforms: Fix job row actions menu invalid DOM …

…nesting warning (elastic#74499) Refactors the action buttons for transforms and analytics job list to no longer produce nested button elements and throw a React error.
walterra · Aug 19, 2020 · b8b521a · b8b521a
1 parent 8946b68
commit b8b521a
Show file tree

Hide file tree

Showing 79 changed files with 1,002 additions and 942 deletions.
diff --git a/...ugins/ml/public/application/components/annotations/annotations_table/annotations_table.js b/...ugins/ml/public/application/components/annotations/annotations_table/annotations_table.js
@@ -506,7 +506,7 @@ export class AnnotationsTable extends Component {
               <EuiButtonIcon
                 onClick={() => this.openSingleMetricView(annotation)}
                 disabled={!isDrillDownAvailable}
-                iconType="stats"
+                iconType="visLine"
                 aria-label={openInSingleMetricViewerAriaLabelText}
               />
             </EuiToolTip>

diff --git a/x-pack/plugins/ml/public/application/components/anomalies_table/links_menu.js b/x-pack/plugins/ml/public/application/components/anomalies_table/links_menu.js
@@ -415,7 +415,7 @@ class LinksMenuUI extends Component {
       items.push(
         <EuiContextMenuItem
           key="view_series"
-          icon="stats"
+          icon="visLine"
           onClick={() => {
             this.closePopover();
             this.viewSeries();

diff --git a/...ic/application/components/anomaly_results_view_selector/anomaly_results_view_selector.tsx b/...ic/application/components/anomaly_results_view_selector/anomaly_results_view_selector.tsx
@@ -27,7 +27,7 @@ export const AnomalyResultsViewSelector: FC<Props> = ({ viewId }) => {
         label: i18n.translate('xpack.ml.anomalyResultsViewSelector.singleMetricViewerLabel', {
           defaultMessage: 'View results in the Single Metric Viewer',
         }),
-        iconType: 'stats',
+        iconType: 'visLine',
         value: 'timeseriesexplorer',
         'data-test-subj': 'mlAnomalyResultsViewSelectorSingleMetricViewer',
       },
@@ -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',
       },

diff --git a/...e_analytics/pages/analytics_creation/components/view_results_panel/view_results_panel.tsx b/...e_analytics/pages/analytics_creation/components/view_results_panel/view_results_panel.tsx
@@ -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',
         })}

diff --git a/...ponents/action_clone/clone_button.test.ts → ...ts/action_clone/clone_action_name.test.ts b/...ponents/action_clone/clone_button.test.ts → ...ts/action_clone/clone_action_name.test.ts
@@ -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', () => {

diff --git a/.../components/action_clone/clone_button.tsx → ...onents/action_clone/clone_action_name.tsx b/.../components/action_clone/clone_button.tsx → ...onents/action_clone/clone_action_name.tsx
@@ -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';
@@ -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';
@@ -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 {
@@ -405,32 +389,11 @@ 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.
- */
-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
@@ -439,10 +402,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}</>;
 };
diff --git a/...lication/data_frame_analytics/pages/analytics_management/components/action_clone/index.ts b/...lication/data_frame_analytics/pages/analytics_management/components/action_clone/index.ts
@@ -7,7 +7,6 @@
 export {
   extractCloningConfig,
   isAdvancedConfig,
-  useNavigateToWizardWithClonedJob,
-  CloneButton,
   CloneDataFrameAnalyticsConfig,
-} from './clone_button';
+} from './clone_action_name';
+export { useCloneAction } from './use_clone_action';
diff --git a/...a_frame_analytics/pages/analytics_management/components/action_clone/use_clone_action.tsx b/...a_frame_analytics/pages/analytics_management/components/action_clone/use_clone_action.tsx
@@ -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 };
+};
diff --git a/...nts/action_delete/delete_button_modal.tsx → ...nts/action_delete/delete_action_modal.tsx b/...nts/action_delete/delete_button_modal.tsx → ...nts/action_delete/delete_action_modal.tsx
@@ -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,

diff --git a/...ents/action_delete/action_delete.test.tsx → ...action_delete/delete_action_name.test.tsx b/...ents/action_delete/action_delete.test.tsx → ...action_delete/delete_action_name.test.tsx
@@ -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', () => ({
@@ -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>
           </>
         );
       };

diff --git a/...omponents/action_delete/delete_button.tsx → ...ents/action_delete/delete_action_name.tsx b/...omponents/action_delete/delete_button.tsx → ...ents/action_delete/delete_action_name.tsx
@@ -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
@@ -51,10 +38,10 @@ export const DeleteButton: FC<DeleteButtonProps> = ({ isDisabled, item, onClick
             : createPermissionFailureMessage('canStartStopDataFrameAnalytics')
         }
       >
-        {button}
+        <>{deleteActionNameText}</>
       </EuiToolTip>
     );
   }
 
-  return button;
+  return <>{deleteActionNameText}</>;
 };
diff --git a/...ication/data_frame_analytics/pages/analytics_management/components/action_delete/index.ts b/...ication/data_frame_analytics/pages/analytics_management/components/action_delete/index.ts
@@ -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';