Update to PF5 - part II - onClick event handler

Reference: #1098 Seems like onClick event handler parameters was changed in PF5 for few of the components by enforcing the event handler as the first parameter (E.g. https://www.patternfly.org/components/chip/#chip while in our code we are not passing the event https://github.com/kubev2v/forklift-console-plugin/blob/686daff412d61c5e84c14f7b87fe2867038fc14b/packages/forklift-console-plugin/src/modules/Plans/views/create/components/ChipsToolbarProviders.tsx#L40). For avoiding uncaught migration errors, this PR named and typed all callback appearances, when possible Signed-off-by: Sharon Gratch <[email protected]>
kubev2v · Jun 16, 2024 · fdaa1e7 · fdaa1e7
1 parent 290f96e
commit fdaa1e7
Show file tree

Hide file tree

Showing 36 changed files with 305 additions and 259 deletions.
diff --git a/...orklift-console-plugin/src/modules/NetworkMaps/actions/NetworkMapActionsDropdownItems.tsx b/...orklift-console-plugin/src/modules/NetworkMaps/actions/NetworkMapActionsDropdownItems.tsx
@@ -21,15 +21,15 @@ export const NetworkMapActionsDropdownItems = ({ data }: NetworkMapActionsDropdo
     namespace: networkMap?.metadata?.namespace,
   });
 
+  const onClick = () => {
+    showModal(<DeleteModal resource={networkMap} model={NetworkMapModel} />);
+  };
+
   return [
     <DropdownItemLink key="EditNetworkMapping" href={networkMapURL}>
       {t('Edit NetworkMap')}
     </DropdownItemLink>,
-    <DropdownItem
-      key="delete"
-      isDisabled={!data?.permissions?.canDelete}
-      onClick={() => showModal(<DeleteModal resource={networkMap} model={NetworkMapModel} />)}
-    >
+    <DropdownItem key="delete" isDisabled={!data?.permissions?.canDelete} onClick={onClick}>
       {t('Delete NetworkMap')}
     </DropdownItem>,
   ];

diff --git a/...ages/forklift-console-plugin/src/modules/NetworkMaps/componenets/NetworkMapsAddButton.tsx b/...ages/forklift-console-plugin/src/modules/NetworkMaps/componenets/NetworkMapsAddButton.tsx
@@ -19,12 +19,12 @@ export const NetworkMapsAddButton: React.FC<{ namespace: string; dataTestId?: st
     namespaced: namespace !== undefined,
   });
 
+  const onClick = () => {
+    history.push(`${NetworkMapsListURL}/~new`);
+  };
+
   return (
-    <Button
-      data-testid={dataTestId}
-      variant="primary"
-      onClick={() => history.push(`${NetworkMapsListURL}/~new`)}
-    >
+    <Button data-testid={dataTestId} variant="primary" onClick={onClick}>
       {t('Create NetworkMap')}
     </Button>
   );

diff --git a/...nsole-plugin/src/modules/NetworkMaps/views/details/components/MapsSection/MapsSection.tsx b/...nsole-plugin/src/modules/NetworkMaps/views/details/components/MapsSection/MapsSection.tsx
@@ -156,6 +156,10 @@ export const MapsSection: React.FC<MapsSectionProps> = ({ obj }) => {
     });
   };
 
+  const onClick = () => {
+    dispatch({ type: 'INIT', payload: obj });
+  };
+
   return (
     <Suspend obj={providers} loaded={providersLoaded} loadError={providersLoadError}>
       <Flex className="forklift-network-map__details-tab--update-button">
@@ -173,7 +177,7 @@ export const MapsSection: React.FC<MapsSectionProps> = ({ obj }) => {
         <FlexItem>
           <Button
             variant="secondary"
-            onClick={() => dispatch({ type: 'INIT', payload: obj })}
+            onClick={onClick}
             isDisabled={!state.hasChanges || state.updating}
           >
             {t('Cancel')}

diff --git a/...in/src/modules/NetworkMaps/views/details/components/ProvidersSection/ProvidersSection.tsx b/...in/src/modules/NetworkMaps/views/details/components/ProvidersSection/ProvidersSection.tsx
@@ -49,6 +49,10 @@ export const ProvidersSection: React.FC<ProvidersSectionProps> = ({ obj }) => {
     });
   };
 
+  const onClick = () => {
+    dispatch({ type: 'INIT', payload: obj });
+  };
+
   return (
     <Suspend obj={providers} loaded={providersLoaded} loadError={providersLoadError}>
       <Flex className="forklift-network-map__details-tab--update-button">
@@ -66,7 +70,7 @@ export const ProvidersSection: React.FC<ProvidersSectionProps> = ({ obj }) => {
         <FlexItem>
           <Button
             variant="secondary"
-            onClick={() => dispatch({ type: 'INIT', payload: obj })}
+            onClick={onClick}
             isDisabled={!state.hasChanges || state.updating}
           >
             {t('Cancel')}

diff --git a/...t-console-plugin/src/modules/Overview/views/overview/components/ShowWelcomeCardButton.tsx b/...t-console-plugin/src/modules/Overview/views/overview/components/ShowWelcomeCardButton.tsx
@@ -10,15 +10,16 @@ export const ShowWelcomeCardButton: React.FC = () => {
   // Set and use context data for the overview page state
   const { setData } = useCreateOverviewContext();
   const { data: { hideWelcomeCardByContext } = {} } = useCreateOverviewContext();
+  const onClick = () => {
+    setData({ hideWelcomeCardByContext: false });
+  };
 
   if (!hideWelcomeCardByContext) return null;
 
   return (
     <Label
       color="purple"
-      onClick={() => {
-        setData({ hideWelcomeCardByContext: false });
-      }}
+      onClick={onClick}
       onClose={() => null}
       style={{ cursor: 'pointer' }}
       data-testid="show-welcome-card"

diff --git a/...ole-plugin/src/modules/Overview/views/overview/tabs/Metrics/cards/MigrationsChartCard.tsx b/...ole-plugin/src/modules/Overview/views/overview/tabs/Metrics/cards/MigrationsChartCard.tsx
@@ -72,6 +72,21 @@ export const MigrationsChartCard: React.FC<MigrationsCardProps> = () => {
     ...migrationsDataPoints.succeeded.map((m) => m.value),
   );
 
+  const onClickLast7Days = () => {
+    onToggle();
+    setSelectedTimeRange(TimeRangeOptions.Last7Days);
+  };
+
+  const onClickLast31Days = () => {
+    onToggle();
+    setSelectedTimeRange(TimeRangeOptions.Last31Days);
+  };
+
+  const onClickLast24H = () => {
+    onToggle();
+    setSelectedTimeRange(TimeRangeOptions.Last24H);
+  };
+
   return (
     <Card>
       <CardHeader>
@@ -84,31 +99,13 @@ export const MigrationsChartCard: React.FC<MigrationsCardProps> = () => {
             isOpen={isDropdownOpened}
             isPlain
             dropdownItems={[
-              <DropdownItem
-                onClick={() => {
-                  onToggle();
-                  setSelectedTimeRange(TimeRangeOptions.Last7Days);
-                }}
-                key="7days"
-              >
+              <DropdownItem onClick={onClickLast7Days} key="7days">
                 {t('7 days')}
               </DropdownItem>,
-              <DropdownItem
-                onClick={() => {
-                  onToggle();
-                  setSelectedTimeRange(TimeRangeOptions.Last31Days);
-                }}
-                key="31days"
-              >
+              <DropdownItem onClick={onClickLast31Days} key="31days">
                 {t('31 days')}
               </DropdownItem>,
-              <DropdownItem
-                onClick={() => {
-                  onToggle();
-                  setSelectedTimeRange(TimeRangeOptions.Last24H);
-                }}
-                key="24hours"
-              >
+              <DropdownItem onClick={onClickLast24H} key="24hours">
                 {t('24 hours')}
               </DropdownItem>,
             ]}

diff --git a/...e-plugin/src/modules/Overview/views/overview/tabs/Metrics/cards/VmMigrationsChartCard.tsx b/...e-plugin/src/modules/Overview/views/overview/tabs/Metrics/cards/VmMigrationsChartCard.tsx
@@ -94,6 +94,21 @@ export const VmMigrationsChartCard: React.FC<MigrationsCardProps> = () => {
     ...VmMigrationsDataPoints.succeeded.map((m) => m.value),
   );
 
+  const onClickLast7Days = () => {
+    onToggle();
+    setSelectedTimeRange(TimeRangeOptions.Last7Days);
+  };
+
+  const onClickLast31Days = () => {
+    onToggle();
+    setSelectedTimeRange(TimeRangeOptions.Last31Days);
+  };
+
+  const onClickLast24H = () => {
+    onToggle();
+    setSelectedTimeRange(TimeRangeOptions.Last24H);
+  };
+
   return (
     <Card>
       <CardHeader>
@@ -106,31 +121,13 @@ export const VmMigrationsChartCard: React.FC<MigrationsCardProps> = () => {
             isOpen={isDropdownOpened}
             isPlain
             dropdownItems={[
-              <DropdownItem
-                onClick={() => {
-                  onToggle();
-                  setSelectedTimeRange(TimeRangeOptions.Last7Days);
-                }}
-                key="7days"
-              >
+              <DropdownItem onClick={onClickLast7Days} key="7days">
                 {t('7 days')}
               </DropdownItem>,
-              <DropdownItem
-                onClick={() => {
-                  onToggle();
-                  setSelectedTimeRange(TimeRangeOptions.Last31Days);
-                }}
-                key="31days"
-              >
+              <DropdownItem onClick={onClickLast31Days} key="31days">
                 {t('31 days')}
               </DropdownItem>,
-              <DropdownItem
-                onClick={() => {
-                  onToggle();
-                  setSelectedTimeRange(TimeRangeOptions.Last24H);
-                }}
-                key="24hours"
-              >
+              <DropdownItem onClick={onClickLast24H} key="24hours">
                 {t('24 hours')}
               </DropdownItem>,
             ]}

diff --git a/packages/forklift-console-plugin/src/modules/Plans/actions/PlanActionsDropdownItems.tsx b/packages/forklift-console-plugin/src/modules/Plans/actions/PlanActionsDropdownItems.tsx
@@ -36,51 +36,61 @@ export const PlanActionsDropdownItems = ({ data }: PlanActionsDropdownItemsProps
 
   const buttonStartLabel = canReStart ? t('Restart migration') : t('Start migration');
 
+  const onClickPlanStart = () => {
+    showModal(
+      <PlanStartMigrationModal resource={plan} model={PlanModel} title={buttonStartLabel} />,
+    );
+  };
+
+  const onClickPlanCutover = () => {
+    showModal(<PlanCutoverMigrationModal resource={plan} />);
+  };
+
+  const onClickDuplicate = () => {
+    showModal(<DuplicateModal resource={plan} model={PlanModel} />);
+  };
+
+  const onClickArchive = () => {
+    showModal(<ArchiveModal resource={plan} model={PlanModel} />);
+  };
+
+  const onClickPlanDelete = () => {
+    showModal(<PlanDeleteModal resource={plan} model={PlanModel} />);
+  };
+
   return [
     <DropdownItemLink key="EditPlan" href={planURL}>
       {t('Edit Plan')}
     </DropdownItemLink>,
 
-    <DropdownItem
-      key="start"
-      isDisabled={!canStart}
-      onClick={() =>
-        showModal(
-          <PlanStartMigrationModal resource={plan} model={PlanModel} title={buttonStartLabel} />,
-        )
-      }
-    >
+    <DropdownItem key="start" isDisabled={!canStart} onClick={onClickPlanStart}>
       {buttonStartLabel}
     </DropdownItem>,
 
-    <DropdownItem
-      key="cutover"
-      isDisabled={!isWarmAndExecuting}
-      onClick={() => showModal(<PlanCutoverMigrationModal resource={plan} />)}
-    >
+    <DropdownItem key="cutover" isDisabled={!isWarmAndExecuting} onClick={onClickPlanCutover}>
       {t('Cutover')}
     </DropdownItem>,
 
     <DropdownItem
       key="duplicate"
       isDisabled={!data?.permissions?.canDelete}
-      onClick={() => showModal(<DuplicateModal resource={plan} model={PlanModel} />)}
+      onClick={onClickDuplicate}
     >
       {t('Duplicate Plan')}
     </DropdownItem>,
 
     <DropdownItem
       key="archive"
       isDisabled={!data?.permissions?.canDelete || ['Archived', 'Archiving'].includes(phase)}
-      onClick={() => showModal(<ArchiveModal resource={plan} model={PlanModel} />)}
+      onClick={onClickArchive}
     >
       {t('Archive Plan')}
     </DropdownItem>,
 
     <DropdownItem
       key="delete"
       isDisabled={!data?.permissions?.canDelete}
-      onClick={() => showModal(<PlanDeleteModal resource={plan} model={PlanModel} />)}
+      onClick={onClickPlanDelete}
     >
       {t('Delete Plan')}
     </DropdownItem>,

diff --git a/...es/forklift-console-plugin/src/modules/Plans/views/details/components/MappingListItem.tsx b/...es/forklift-console-plugin/src/modules/Plans/views/details/components/MappingListItem.tsx
@@ -50,6 +50,10 @@ export const MappingListItem: FC<MappingListItemProps> = ({
   const [isSrcOpen, setToggleSrcOpen] = useToggle(false);
   const [isTrgOpen, setToggleTrgOpen] = useToggle(false);
 
+  const onClick = () => {
+    deleteMapping({ source, destination });
+  };
+
   return (
     <DataListItem aria-labelledby="">
       <DataListItemRow>
@@ -111,7 +115,7 @@ export const MappingListItem: FC<MappingListItemProps> = ({
             aria-labelledby=""
           >
             <Button
-              onClick={() => deleteMapping({ source, destination })}
+              onClick={onClick}
               variant="plain"
               aria-label={t('Delete mapping')}
               key="delete-action"

diff --git a/...s/forklift-console-plugin/src/modules/Plans/views/details/components/PlanPageHeadings.tsx b/...s/forklift-console-plugin/src/modules/Plans/views/details/components/PlanPageHeadings.tsx
@@ -58,21 +58,16 @@ export const PlanPageHeadings: React.FC<{ name: string; namespace: string }> = (
     );
   }
 
+  const onClick = () => {
+    showModal(
+      <PlanStartMigrationModal resource={plan} model={PlanModel} title={buttonStartLabel} />,
+    );
+  };
+
   const actions = (
     <Level hasGutter>
       {canStart && (
-        <Button
-          variant="primary"
-          onClick={() =>
-            showModal(
-              <PlanStartMigrationModal
-                resource={plan}
-                model={PlanModel}
-                title={buttonStartLabel}
-              />,
-            )
-          }
-        >
+        <Button variant="primary" onClick={onClick}>
           <Level hasGutter>
             <>
               <LevelItem>{buttonStartIcon}</LevelItem>

diff --git a/packages/forklift-console-plugin/src/modules/Plans/views/details/tabs/Hooks/PlanHooks.tsx b/packages/forklift-console-plugin/src/modules/Plans/views/details/tabs/Hooks/PlanHooks.tsx
@@ -50,6 +50,13 @@ export const PlanHooks: React.FC<{ name: string; namespace: string }> = ({ name,
     onUpdatePlanHooks({ plan, preHookResource, postHookResource, dispatch, state });
   }
 
+  const onClick = () => {
+    dispatch({
+      type: 'INIT',
+      payload: initialState(plan, preHookResource, postHookResource),
+    });
+  };
+
   const HooksTabAction = (
     <>
       <Flex>
@@ -65,16 +72,7 @@ export const PlanHooks: React.FC<{ name: string; namespace: string }> = ({ name,
         </FlexItem>
 
         <FlexItem>
-          <Button
-            variant="secondary"
-            isDisabled={!state.hasChanges}
-            onClick={() =>
-              dispatch({
-                type: 'INIT',
-                payload: initialState(plan, preHookResource, postHookResource),
-              })
-            }
-          >
+          <Button variant="secondary" isDisabled={!state.hasChanges} onClick={onClick}>
             {t('Cancel')}
           </Button>
         </FlexItem>

diff --git a/.../modules/Plans/views/details/tabs/VirtualMachines/components/MigrationVMsCancelButton.tsx b/.../modules/Plans/views/details/tabs/VirtualMachines/components/MigrationVMsCancelButton.tsx
@@ -13,16 +13,13 @@ export const MigrationVMsCancelButton: FC<{
 }> = ({ selectedIds, migration }) => {
   const { t } = useForkliftTranslation();
   const { showModal } = useModal();
+  const onClick = () => {
+    showModal(<MigrationVMsCancelModal migration={migration} selected={selectedIds} />);
+  };
 
   return (
     <ToolbarItem>
-      <Button
-        variant="secondary"
-        onClick={() =>
-          showModal(<MigrationVMsCancelModal migration={migration} selected={selectedIds} />)
-        }
-        isDisabled={!selectedIds?.length}
-      >
+      <Button variant="secondary" onClick={onClick} isDisabled={!selectedIds?.length}>
         {t('Cancel virtual machines')}
       </Button>
     </ToolbarItem>