address feedbacks

DaoDaoNoCode · Nov 15, 2024 · 8fda8f8 · 8fda8f8
1 parent e6c791d
commit 8fda8f8
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 65 deletions.
diff --git a/frontend/src/concepts/pipelines/content/PipelineVersionLink.tsx b/frontend/src/concepts/pipelines/content/PipelineVersionLink.tsx
@@ -1,37 +1,28 @@
 import React from 'react';
 import { Link } from 'react-router-dom';
-import { Skeleton, Tooltip } from '@patternfly/react-core';
+import { Skeleton } from '@patternfly/react-core';
+import { TableText } from '@patternfly/react-table';
 import { usePipelinesAPI } from '~/concepts/pipelines/context';
 import { PipelineVersionKF } from '~/concepts/pipelines/kfTypes';
 import { pipelineVersionDetailsRoute } from '~/routes';
 import { NoRunContent } from './tables/renderUtils';
 
 interface PipelineVersionLinkProps {
-  displayName?: string;
   loadingIndicator?: React.ReactElement;
   version?: PipelineVersionKF | null;
   error?: Error;
   loaded: boolean;
 }
 
 export const PipelineVersionLink: React.FC<PipelineVersionLinkProps> = ({
-  displayName,
   loadingIndicator,
   version,
   error,
   loaded,
 }) => {
   const { namespace } = usePipelinesAPI();
 
-  if (error) {
-    return (
-      <Tooltip content={error.message} position="right">
-        <div className="pf-v5-u-disabled-color-100 pf-v5-c-truncate__start">{displayName}</div>
-      </Tooltip>
-    );
-  }
-
-  if (!loaded) {
+  if (!loaded && !error) {
     return loadingIndicator || <Skeleton />;
   }
 
@@ -43,7 +34,7 @@ export const PipelineVersionLink: React.FC<PipelineVersionLinkProps> = ({
     <Link
       to={pipelineVersionDetailsRoute(namespace, version.pipeline_id, version.pipeline_version_id)}
     >
-      {version.display_name}
+      <TableText wrapModifier="truncate">{version.display_name}</TableText>
     </Link>
   );
 };
diff --git a/...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunTabDetails.tsx b/...end/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunTabDetails.tsx
@@ -58,24 +58,21 @@ const PipelineRunTabDetails: React.FC<PipelineRunTabDetailsProps> = ({ run, work
       key: 'Project',
       value: <Link to={`/projects/${namespace}`}>{getDisplayNameFromK8sResource(project)}</Link>,
     },
-    ...(version
-      ? [
+    ...(versionError
+      ? [{ key: 'Pipeline version', value: 'No pipeline version' }]
+      : [
           {
             key: 'Pipeline version',
             value: (
               <PipelineVersionLink
-                displayName={version.display_name}
                 loadingIndicator={<Spinner size="sm" />}
                 loaded={versionLoaded}
                 version={version}
                 error={versionError}
               />
             ),
           },
-        ]
-      : versionError
-      ? [{ key: 'Pipeline version', value: 'No pipeline version' }]
-      : []),
+        ]),
     ...(pipeline
       ? [
           {

diff --git a/...c/concepts/pipelines/content/tables/pipelineRecurringRun/PipelineRecurringRunTableRow.tsx b/...c/concepts/pipelines/content/tables/pipelineRecurringRun/PipelineRecurringRunTableRow.tsx
@@ -14,7 +14,7 @@ import {
   RecurringRunTrigger,
 } from '~/concepts/pipelines/content/tables/renderUtils';
 import PipelineRunTableRowExperiment from '~/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRowExperiment';
-import useExperimentById from '~/concepts/pipelines/apiHooks/useExperimentById';
+import usePipelineRunExperimentInfo from '~/concepts/pipelines/content/tables/usePipelineRunExperimentInfo';
 
 type PipelineRecurringRunTableRowProps = {
   isChecked: boolean;
@@ -34,11 +34,16 @@ const PipelineRecurringRunTableRow: React.FC<PipelineRecurringRunTableRowProps>
   const navigate = useNavigate();
   const { experimentId, pipelineId, pipelineVersionId } = useParams();
   const { namespace, api } = usePipelinesAPI();
-  const { version, loaded, error } = usePipelineRunVersionInfo(recurringRun);
-  const pipelineRecurringExperimentId = !experimentId ? recurringRun.experiment_id : '';
-  const [pipelineRecurringExperiment, pipelineRecurringExperimentLoaded] = useExperimentById(
-    pipelineRecurringExperimentId,
-  );
+  const {
+    version,
+    loaded: isVersionLoaded,
+    error: versionError,
+  } = usePipelineRunVersionInfo(recurringRun);
+  const {
+    experiment,
+    loaded: isExperimentLoaded,
+    error: experimentError,
+  } = usePipelineRunExperimentInfo(recurringRun);
 
   return (
     <Tr>
@@ -68,19 +73,15 @@ const PipelineRecurringRunTableRow: React.FC<PipelineRecurringRunTableRowProps>
       </Td>
       {!pipelineVersionId && (
         <Td modifier="truncate" dataLabel="Pipeline">
-          <PipelineVersionLink
-            displayName={version?.display_name}
-            version={version}
-            error={error}
-            loaded={loaded}
-          />
+          <PipelineVersionLink version={version} error={versionError} loaded={isVersionLoaded} />
         </Td>
       )}
       {!experimentId && (
         <Td modifier="truncate" dataLabel="Experiment">
           <PipelineRunTableRowExperiment
-            experiment={pipelineRecurringExperiment}
-            loaded={pipelineRecurringExperimentLoaded}
+            experiment={experiment}
+            error={experimentError}
+            loaded={isExperimentLoaded}
           />
         </Td>
       )}
@@ -92,7 +93,7 @@ const PipelineRecurringRunTableRow: React.FC<PipelineRecurringRunTableRowProps>
       </Td>
       <Td dataLabel="Status">
         <RecurringRunStatus
-          experiment={pipelineRecurringExperiment}
+          experiment={experiment}
           recurringRun={recurringRun}
           onToggle={(checked) =>
             api

diff --git a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRow.tsx b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRow.tsx
@@ -52,6 +52,7 @@ const PipelineRunTableRow: React.FC<PipelineRunTableRowProps> = ({
   const [isRestoreModalOpen, setIsRestoreModalOpen] = React.useState(false);
   const [isArchiveModalOpen, setIsArchiveModalOpen] = React.useState(false);
   const isExperimentArchived = useContextExperimentArchived();
+  const isExperimentDeleted = !experiment && !experimentId;
 
   const actions: IAction[] = React.useMemo(() => {
     const isGlobal = !experimentId && !pipelineId && !pipelineVersionId;
@@ -76,11 +77,12 @@ const PipelineRunTableRow: React.FC<PipelineRunTableRowProps> = ({
         {
           title: 'Restore',
           onClick: () => setIsRestoreModalOpen(true),
-          isAriaDisabled: isExperimentArchived,
-          ...(isExperimentArchived && {
+          isAriaDisabled: isExperimentArchived || isExperimentDeleted,
+          ...((isExperimentArchived || isExperimentDeleted) && {
             tooltipProps: {
-              content:
-                'Archived runs cannot be restored until its associated experiment is restored.',
+              content: isExperimentArchived
+                ? 'Archived runs cannot be restored until its associated experiment is restored.'
+                : 'Archived runs cannot be restored because its associated experiment is deleted.',
             },
           }),
         },
@@ -119,20 +121,21 @@ const PipelineRunTableRow: React.FC<PipelineRunTableRowProps> = ({
       },
     ];
   }, [
+    experimentId,
+    pipelineId,
+    pipelineVersionId,
     runType,
     run.state,
     run.run_id,
     version,
+    navigate,
+    namespace,
     isExperimentArchived,
+    isExperimentDeleted,
     onDelete,
     api,
     refreshAllAPI,
     notification,
-    navigate,
-    namespace,
-    experimentId,
-    pipelineId,
-    pipelineVersionId,
   ]);
 
   return (
@@ -152,18 +155,12 @@ const PipelineRunTableRow: React.FC<PipelineRunTableRowProps> = ({
       </Td>
       {!pipelineVersionId && (
         <Td modifier="truncate" dataLabel="Pipeline">
-          <PipelineVersionLink
-            displayName={version?.display_name}
-            version={version}
-            error={versionError}
-            loaded={isVersionLoaded}
-          />
+          <PipelineVersionLink version={version} error={versionError} loaded={isVersionLoaded} />
         </Td>
       )}
       {!experimentId && (
         <Td modifier="truncate" dataLabel="Experiment">
           <PipelineRunTableRowExperiment
-            displayName={experiment?.display_name}
             experiment={experiment}
             error={experimentError}
             loaded={isExperimentLoaded}

diff --git a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRowExperiment.tsx b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRowExperiment.tsx
@@ -1,35 +1,26 @@
 import React from 'react';
-import { Skeleton, Tooltip } from '@patternfly/react-core';
+import { Skeleton } from '@patternfly/react-core';
 import { Link } from 'react-router-dom';
+import { TableText } from '@patternfly/react-table';
 import { usePipelinesAPI } from '~/concepts/pipelines/context';
 import { experimentRunsRoute } from '~/routes';
 import { ExperimentKF } from '~/concepts/pipelines/kfTypes';
 import { NoRunContent } from '~/concepts/pipelines/content/tables/renderUtils';
 
 type PipelineRunTableRowExperimentProps = {
-  displayName?: string;
   experiment?: ExperimentKF | null;
   loaded: boolean;
   error?: Error;
 };
 
 const PipelineRunTableRowExperiment: React.FC<PipelineRunTableRowExperimentProps> = ({
-  displayName,
   experiment,
   loaded,
   error,
 }) => {
   const { namespace } = usePipelinesAPI();
 
-  if (error) {
-    return (
-      <Tooltip content={error.message} position="right">
-        <div className="pf-v5-u-disabled-color-100 pf-v5-c-truncate__start">{displayName}</div>
-      </Tooltip>
-    );
-  }
-
-  if (!loaded) {
+  if (!loaded && !error) {
     return <Skeleton />;
   }
 
@@ -38,7 +29,7 @@ const PipelineRunTableRowExperiment: React.FC<PipelineRunTableRowExperimentProps
   }
   return (
     <Link to={experimentRunsRoute(namespace, experiment.experiment_id)}>
-      {experiment.display_name}
+      <TableText wrapModifier="truncate">{experiment.display_name}</TableText>
     </Link>
   );
 };

diff --git a/frontend/src/concepts/pipelines/content/tables/renderUtils.tsx b/frontend/src/concepts/pipelines/content/tables/renderUtils.tsx
@@ -140,7 +140,7 @@ export const RecurringRunScheduled: RecurringRunUtil = ({ recurringRun }) => {
 
 export const RecurringRunStatus: RecurringRunUtil<{
   onToggle: (value: boolean) => Promise<void>;
-  experiment: ExperimentKF | null;
+  experiment?: ExperimentKF | null;
 }> = ({ recurringRun, onToggle, experiment }) => {
   const [error, setError] = React.useState<Error | null>(null);
   const [isChangingFlag, setIsChangingFlag] = React.useState(false);