address comments

opendatahub-io · Jul 21, 2023 · 8de654f · 8de654f
1 parent 33cdefe
commit 8de654f
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 85 deletions.
diff --git a/frontend/src/concepts/dashboard/DashboardInlineHelpIcon.tsx b/frontend/src/concepts/dashboard/DashboardInlineHelpIcon.tsx
diff --git a/frontend/src/concepts/dashboard/DashboardSmallTextList.tsx b/frontend/src/concepts/dashboard/DashboardSmallTextList.tsx
diff --git a/frontend/src/pages/BYONImages/BYONImageDependenciesList.tsx b/frontend/src/pages/BYONImages/BYONImageDependenciesList.tsx
@@ -0,0 +1,39 @@
+import * as React from 'react';
+import {
+  DescriptionListDescription,
+  DescriptionListGroup,
+  DescriptionListTerm,
+  Text,
+  TextContent,
+} from '@patternfly/react-core';
+
+type BYONImageDependenciesListProps = {
+  dependencies: string[];
+  term: string;
+};
+
+const BYONImageDependenciesList: React.FC<BYONImageDependenciesListProps> = ({
+  term,
+  dependencies,
+}) => {
+  if (dependencies.length === 0) {
+    return null;
+  }
+
+  return (
+    <DescriptionListGroup>
+      <DescriptionListTerm>{term}</DescriptionListTerm>
+      <DescriptionListDescription>
+        <TextContent>
+          {dependencies.map((dep, i) => (
+            <Text style={{ marginBottom: 0 }} key={i} component="small">
+              {dep}
+            </Text>
+          ))}
+        </TextContent>
+      </DescriptionListDescription>
+    </DescriptionListGroup>
+  );
+};
+
+export default BYONImageDependenciesList;
diff --git a/frontend/src/pages/BYONImages/BYONImageModal/DisplayedContentTableRow.tsx b/frontend/src/pages/BYONImages/BYONImageModal/DisplayedContentTableRow.tsx
@@ -31,6 +31,8 @@ const DisplayedContentTableRow: React.FC<DisplayedContentTableRowProps> = ({
   const [name, setName] = React.useState(obj.name);
   const [version, setVersion] = React.useState(obj.version);
 
+  const dataLabel = tabKey === DisplayedContentTab.SOFTWARE ? 'Software' : 'Packages';
+
   const onKeyDown = React.useCallback(
     (event: React.KeyboardEvent<HTMLInputElement>) => {
       event.stopPropagation();
@@ -48,16 +50,27 @@ const DisplayedContentTableRow: React.FC<DisplayedContentTableRowProps> = ({
   return (
     <Tbody>
       <Tr>
-        <Td dataLabel={tabKey === DisplayedContentTab.SOFTWARE ? 'Software' : 'Packages'}>
+        <Td dataLabel={dataLabel}>
           {isActive ? (
-            <TextInput value={name} onChange={setName} onKeyDown={onKeyDown} autoFocus />
+            <TextInput
+              aria-label={`${dataLabel} name input`}
+              value={name}
+              onChange={setName}
+              onKeyDown={onKeyDown}
+              autoFocus
+            />
           ) : (
             <>{obj.name}</>
           )}
         </Td>
         <Td dataLabel="Version">
           {isActive ? (
-            <TextInput value={version} onChange={setVersion} onKeyDown={onKeyDown} />
+            <TextInput
+              aria-label={`${dataLabel} version input`}
+              value={version}
+              onChange={setVersion}
+              onKeyDown={onKeyDown}
+            />
           ) : (
             <>{obj.version}</>
           )}

diff --git a/frontend/src/pages/BYONImages/BYONImageModal/ImageLocationField.tsx b/frontend/src/pages/BYONImages/BYONImageModal/ImageLocationField.tsx
@@ -1,6 +1,6 @@
 import * as React from 'react';
 import { FormGroup, FormHelperText, Popover, TextInput } from '@patternfly/react-core';
-import DashboardInlineHelpIcon from '~/concepts/dashboard/DashboardInlineHelpIcon';
+import { HelpIcon } from '@patternfly/react-icons';
 
 type ImageLocationFieldProps = {
   location: string;
@@ -14,8 +14,7 @@ const ImageLocationField: React.FC<ImageLocationFieldProps> = ({ location, setLo
     fieldId="byon-image-location-input"
     helperText={
       <FormHelperText component="div" isHidden={false}>
-        The address where the notebook image is located. See <DashboardInlineHelpIcon /> for
-        examples.
+        The address where the notebook image is located. See the help icon for examples.
       </FormHelperText>
     }
     labelIcon={
@@ -31,7 +30,7 @@ const ImageLocationField: React.FC<ImageLocationFieldProps> = ({ location, setLo
           </>
         }
       >
-        <DashboardInlineHelpIcon />
+        <HelpIcon />
       </Popover>
     }
   >

diff --git a/frontend/src/pages/BYONImages/BYONImagesTableRow.tsx b/frontend/src/pages/BYONImages/BYONImagesTableRow.tsx
@@ -1,23 +1,13 @@
 import * as React from 'react';
 import { ActionsColumn, ExpandableRowContent, Tbody, Td, Tr } from '@patternfly/react-table';
-import {
-  DescriptionList,
-  DescriptionListDescription,
-  DescriptionListGroup,
-  DescriptionListTerm,
-  Flex,
-  Text,
-  TextContent,
-  Timestamp,
-  TimestampTooltipVariant,
-} from '@patternfly/react-core';
+import { DescriptionList, Flex, Timestamp, TimestampTooltipVariant } from '@patternfly/react-core';
 import { BYONImage } from '~/types';
 import { relativeTime } from '~/utilities/time';
 import ResourceNameTooltip from '~/components/ResourceNameTooltip';
-import DashboardSmallTextList from '~/concepts/dashboard/DashboardSmallTextList';
 import ImageErrorStatus from './ImageErrorStatus';
 import BYONImageStatusToggle from './BYONImageStatusToggle';
 import { convertBYONImageToK8sResource } from './utils';
+import BYONImageDependenciesList from './BYONImageDependenciesList';
 
 type BYONImagesTableRowProps = {
   obj: BYONImage;
@@ -108,36 +98,15 @@ const BYONImagesTableRow: React.FC<BYONImagesTableRowProps> = ({
         <Td dataLabel="Other information" colSpan={6}>
           <ExpandableRowContent>
             <DescriptionList columnModifier={{ default: columnModifier }}>
-              {obj.software.length > 0 && (
-                <DescriptionListGroup>
-                  <DescriptionListTerm>Displayed software</DescriptionListTerm>
-                  <DescriptionListDescription>
-                    <DashboardSmallTextList
-                      keyPrefix="software"
-                      textList={obj.software.map((s) => `${s.name} ${s.version}`)}
-                    />
-                  </DescriptionListDescription>
-                </DescriptionListGroup>
-              )}
-              {obj.packages.length > 0 && (
-                <DescriptionListGroup>
-                  <DescriptionListTerm>Displayed packages</DescriptionListTerm>
-                  <DescriptionListDescription>
-                    <DashboardSmallTextList
-                      keyPrefix="packages"
-                      textList={obj.packages.map((p) => `${p.name} ${p.version}`)}
-                    />
-                  </DescriptionListDescription>
-                </DescriptionListGroup>
-              )}
-              <DescriptionListGroup>
-                <DescriptionListTerm>Image location</DescriptionListTerm>
-                <DescriptionListDescription>
-                  <TextContent>
-                    <Text component="small">{obj.url}</Text>
-                  </TextContent>
-                </DescriptionListDescription>
-              </DescriptionListGroup>
+              <BYONImageDependenciesList
+                term="Displayed software"
+                dependencies={obj.software.map((s) => `${s.name} ${s.version}`)}
+              />
+              <BYONImageDependenciesList
+                term="Displayed packages"
+                dependencies={obj.packages.map((p) => `${p.name} ${p.version}`)}
+              />
+              <BYONImageDependenciesList term="Image location" dependencies={[obj.url]} />
             </DescriptionList>
           </ExpandableRowContent>
         </Td>

diff --git a/frontend/src/pages/BYONImages/utils.ts b/frontend/src/pages/BYONImages/utils.ts
@@ -12,6 +12,15 @@ export const convertBYONImageToK8sResource = (image: BYONImage): K8sResourceComm
   },
 });
 
-// 1 = Enabled, 0 = Disabled, -1 = Error
+enum ImageEnabledStatus {
+  ENABLED = 1,
+  DISABLED = 0,
+  ERROR = -1,
+}
+
 export const getEnabledStatus = (image: BYONImage): number =>
-  image.visible && !image.error ? 1 : image.error ? -1 : 0;
+  image.visible && !image.error
+    ? ImageEnabledStatus.ENABLED
+    : image.error
+    ? ImageEnabledStatus.ERROR
+    : ImageEnabledStatus.DISABLED;