address comments

Signed-off-by: Joachim Schuler <[email protected]>

Author: jschuler

packages/forklift-console-plugin/src/modules/Plans/views/details/tabs/VirtualMachines/components/MigrationVMsCancelButton.tsx

@@ -3,10 +3,12 @@ import { useModal } from 'src/modules/Providers/modals';
 import { useForkliftTranslation } from 'src/utils/i18n';
 
 import { V1beta1Migration } from '@kubev2v/types';
-import { Button, ToolbarItem, Tooltip } from '@patternfly/react-core';
+import { ToolbarItem } from '@patternfly/react-core';
 
 import { MigrationVMsCancelModal } from '../modals';
 
+import { VMsActionButton } from './VMsActionButton';
+
 export const MigrationVMsCancelButton: FC<{
   selectedIds: string[];
   migration: V1beta1Migration;
@@ -19,13 +21,11 @@ export const MigrationVMsCancelButton: FC<{
 
   const reason = selectedIds?.length < 1 && t('Select at least one virtual machine.');
 
-  const button = (
-    <Button variant="secondary" onClick={onClick} isAriaDisabled={Boolean(reason)}>
-      {t('Cancel virtual machines')}
-    </Button>
-  );
-
   return (
-    <ToolbarItem>{reason ? <Tooltip content={reason}>{button}</Tooltip> : button}</ToolbarItem>
+    <ToolbarItem>
+      <VMsActionButton onClick={onClick} disabledReason={reason}>
+        {t('Cancel virtual machines')}
+      </VMsActionButton>
+    </ToolbarItem>
   );
 };

packages/forklift-console-plugin/src/modules/Plans/views/details/tabs/VirtualMachines/components/PlanVMsDeleteButton.tsx

@@ -4,10 +4,12 @@ import { useModal } from 'src/modules/Providers/modals';
 import { useForkliftTranslation } from 'src/utils/i18n';
 
 import { V1beta1Plan } from '@kubev2v/types';
-import { Button, ToolbarItem, Tooltip } from '@patternfly/react-core';
+import { ToolbarItem } from '@patternfly/react-core';
 
 import { PlanVMsDeleteModal } from '../modals';
 
+import { VMsActionButton } from './VMsActionButton';
+
 export const PlanVMsDeleteButton: FC<{
   selectedIds: string[];
   plan: V1beta1Plan;
@@ -27,24 +29,22 @@ export const PlanVMsDeleteButton: FC<{
     if (isPlanArchived(plan)) {
       return t('Deleting virtual machines from an archived migration plan is not allowed.');
     }
-    if (!!selectedIds?.length) {
+    if (!selectedIds?.length) {
       return t('Select at least one virtual machine.');
     }
-    if (!!remainingVms.length) {
+    if (!remainingVms?.length) {
       return t(
         'All virtual machines planned for migration are selected for deletion, deleting all virtual machines from a migration plan is not allowed.',
       );
     }
-    return reason;
+    return '';
   }, [plan, selectedIds, remainingVms]);
 
-  const button = (
-    <Button variant="secondary" onClick={onClick} isAriaDisabled={Boolean(reason)}>
-      {t('Remove virtual machines')}
-    </Button>
-  );
-
   return (
-    <ToolbarItem>{reason ? <Tooltip content={reason}>{button}</Tooltip> : button}</ToolbarItem>
+    <ToolbarItem>
+      <VMsActionButton onClick={onClick} disabledReason={reason}>
+        {t('Remove virtual machines')}
+      </VMsActionButton>
+    </ToolbarItem>
   );
 };

packages/forklift-console-plugin/src/modules/Plans/views/details/tabs/VirtualMachines/components/VMsActionButton.tsx

@@ -0,0 +1,21 @@
+import React, { FC, ReactNode } from 'react';
+
+import { Button, ButtonVariant, Tooltip } from '@patternfly/react-core';
+
+export const VMsActionButton: FC<{
+  children: ReactNode;
+  onClick: () => void;
+  disabledReason?: string;
+}> = ({ children, onClick, disabledReason }) => {
+  const button = (
+    <Button
+      variant={ButtonVariant.secondary}
+      onClick={onClick}
+      isAriaDisabled={Boolean(disabledReason)}
+    >
+      {children}
+    </Button>
+  );
+
+  return disabledReason ? <Tooltip content={disabledReason}>{button}</Tooltip> : button;
+};

packages/forklift-console-plugin/src/modules/Plans/views/details/tabs/VirtualMachines/components/index.ts

@@ -4,4 +4,5 @@ export * from './MigrationVMsCancelButton';
 export * from './NameCellRenderer';
 export * from './PlanVMsCellProps';
 export * from './PlanVMsDeleteButton';
+export * from './VMsActionButton';
 // @endindex

packages/forklift-console-plugin/src/modules/Plans/views/details/tabs/VirtualMachines/modals/MigrationVMsCancelModal.tsx

@@ -5,7 +5,7 @@ import { useForkliftTranslation } from 'src/utils/i18n';
 
 import { MigrationModel, V1beta1Migration } from '@kubev2v/types';
 import { k8sPatch } from '@openshift-console/dynamic-plugin-sdk';
-import { Button, Modal, ModalVariant } from '@patternfly/react-core';
+import { Button, ButtonVariant, Modal, ModalVariant } from '@patternfly/react-core';
 
 import './PlanVMsDeleteModal.style.css';
 
@@ -53,13 +53,12 @@ export const MigrationVMsCancelModal: React.FC<MigrationVMsCancelModalProps> = (
     <Button
       key="confirm"
       onClick={handleSave}
-      variant="primary"
-      isDisabled={vms.length < 1}
+      variant={ButtonVariant.primary}
       isLoading={isLoading}
     >
       {t('Cancel migration')}
     </Button>,
-    <Button key="cancel" variant="secondary" onClick={toggleModal}>
+    <Button key="cancel" variant={ButtonVariant.secondary} onClick={toggleModal}>
       {t('Cancel')}
     </Button>,
   ];

packages/forklift-console-plugin/src/modules/Plans/views/details/tabs/VirtualMachines/modals/PlanVMsDeleteModal.tsx

@@ -5,7 +5,7 @@ import { useForkliftTranslation } from 'src/utils/i18n';
 
 import { PlanModel, V1beta1Plan } from '@kubev2v/types';
 import { k8sPatch } from '@openshift-console/dynamic-plugin-sdk';
-import { Button, Modal, ModalVariant } from '@patternfly/react-core';
+import { Button, ButtonVariant, Modal, ModalVariant } from '@patternfly/react-core';
 
 import './PlanVMsDeleteModal.style.css';
 
@@ -43,10 +43,10 @@ export const PlanVMsDeleteModal: React.FC<PlanVMsDeleteModalProps> = ({ plan, se
   }, [selected]);
 
   const actions = [
-    <Button key="confirm" onClick={handleSave} variant="danger" isLoading={isLoading}>
+    <Button key="confirm" onClick={handleSave} variant={ButtonVariant.danger} isLoading={isLoading}>
       {t('Delete')}
     </Button>,
-    <Button key="cancel" variant="secondary" onClick={toggleModal}>
+    <Button key="cancel" variant={ButtonVariant.secondary} onClick={toggleModal}>
       {t('Cancel')}
     </Button>,
   ];