From cfc492ebdc9b9e2fa89b7e23f8427a2b6ad90bfa Mon Sep 17 00:00:00 2001
From: Juntao Wang <juntwang@redhat.com>
Date: Wed, 13 Sep 2023 13:57:40 -0400
Subject: [PATCH] Minor UX fixes for custom notebook images

---
 .../BYONImageModal/ImageLocationField.tsx       |  8 +++++++-
 .../BYONImageModal/ManageBYONImageModal.tsx     | 17 ++++++++++++-----
 .../src/pages/BYONImages/BYONImagesTableRow.tsx |  8 +++++---
 frontend/src/pages/BYONImages/utils.ts          |  5 ++++-
 4 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/frontend/src/pages/BYONImages/BYONImageModal/ImageLocationField.tsx b/frontend/src/pages/BYONImages/BYONImageModal/ImageLocationField.tsx
index 189f5ae99f..0412b04725 100644
--- a/frontend/src/pages/BYONImages/BYONImageModal/ImageLocationField.tsx
+++ b/frontend/src/pages/BYONImages/BYONImageModal/ImageLocationField.tsx
@@ -5,9 +5,14 @@ import { HelpIcon } from '@patternfly/react-icons';
 type ImageLocationFieldProps = {
   location: string;
   setLocation: (location: string) => void;
+  isDisabled: boolean;
 };
 
-const ImageLocationField: React.FC<ImageLocationFieldProps> = ({ location, setLocation }) => (
+const ImageLocationField: React.FC<ImageLocationFieldProps> = ({
+  location,
+  setLocation,
+  isDisabled,
+}) => (
   <FormGroup
     label="Image location"
     isRequired
@@ -45,6 +50,7 @@ const ImageLocationField: React.FC<ImageLocationFieldProps> = ({ location, setLo
       onChange={(value) => {
         setLocation(value);
       }}
+      isDisabled={isDisabled}
     />
   </FormGroup>
 );
diff --git a/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx b/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx
index ffa8b1648e..b43d077bfe 100644
--- a/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx
+++ b/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx
@@ -13,6 +13,7 @@ import { importBYONImage, updateBYONImage } from '~/services/imagesService';
 import { ResponseStatus, BYONImagePackage, BYONImage } from '~/types';
 import { useAppSelector } from '~/redux/hooks';
 import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter';
+import { filterBlankPackages } from '~/pages/BYONImages/utils';
 import ImageLocationField from './ImageLocationField';
 import DisplayedContentTabContent from './DisplayedContentTabContent';
 
@@ -90,8 +91,8 @@ export const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({
         // eslint-disable-next-line camelcase
         display_name: displayName,
         description: description,
-        packages: packages,
-        software: software,
+        packages: filterBlankPackages(packages),
+        software: filterBlankPackages(software),
       }).then(handleResponse);
     } else {
       importBYONImage({
@@ -100,8 +101,8 @@ export const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({
         url: repository,
         description: description,
         provider: userName,
-        software: software,
-        packages: packages,
+        packages: filterBlankPackages(packages),
+        software: filterBlankPackages(software),
       }).then(handleResponse);
     }
   };
@@ -132,7 +133,13 @@ export const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({
           submit();
         }}
       >
-        {!existingImage && <ImageLocationField location={repository} setLocation={setRepository} />}
+        {
+          <ImageLocationField
+            isDisabled={!!existingImage}
+            location={repository}
+            setLocation={setRepository}
+          />
+        }
         <FormGroup label="Name" isRequired fieldId="byon-image-name-input">
           <TextInput
             id="byon-image-name-input"
diff --git a/frontend/src/pages/BYONImages/BYONImagesTableRow.tsx b/frontend/src/pages/BYONImages/BYONImagesTableRow.tsx
index 3869d2369d..0544f234ea 100644
--- a/frontend/src/pages/BYONImages/BYONImagesTableRow.tsx
+++ b/frontend/src/pages/BYONImages/BYONImagesTableRow.tsx
@@ -47,7 +47,7 @@ const BYONImagesTableRow: React.FC<BYONImagesTableRowProps> = ({
             onToggle: () => setExpanded(!isExpanded),
           }}
         />
-        <Td dataLabel="Name">
+        <Td dataLabel="Name" modifier="nowrap">
           <Flex
             spaceItems={{ default: 'spaceItemsSm' }}
             alignItems={{ default: 'alignItemsCenter' }}
@@ -62,8 +62,10 @@ const BYONImagesTableRow: React.FC<BYONImagesTableRowProps> = ({
             </FlexItem>
           </Flex>
         </Td>
-        <Td dataLabel="Description">{obj.description}</Td>
-        <Td dataLabel="Enable">
+        <Td dataLabel="Description" modifier="breakWord">
+          {obj.description}
+        </Td>
+        <Td dataLabel="Enable" modifier="nowrap">
           <BYONImageStatusToggle image={obj} />
         </Td>
         <Td dataLabel="Provider">{obj.provider}</Td>
diff --git a/frontend/src/pages/BYONImages/utils.ts b/frontend/src/pages/BYONImages/utils.ts
index 28aa6a98d7..228156a86b 100644
--- a/frontend/src/pages/BYONImages/utils.ts
+++ b/frontend/src/pages/BYONImages/utils.ts
@@ -1,5 +1,5 @@
 import { K8sResourceCommon } from '@openshift/dynamic-plugin-sdk-utils';
-import { BYONImage } from '~/types';
+import { BYONImage, BYONImagePackage } from '~/types';
 
 export const convertBYONImageToK8sResource = (image: BYONImage): K8sResourceCommon => ({
   kind: 'ImageStream',
@@ -24,3 +24,6 @@ export const getEnabledStatus = (image: BYONImage): number =>
     : image.error
     ? ImageEnabledStatus.ERROR
     : ImageEnabledStatus.DISABLED;
+
+export const filterBlankPackages = (packages: BYONImagePackage[]) =>
+  packages.filter((p) => p.name.trim() || p.version.trim());