address UX concerns with connection types (#3201)

opendatahub-io · Sep 13, 2024 · 3a76f55 · 3a76f55
1 parent d0b5ce9
commit 3a76f55
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 45 deletions.
diff --git a/frontend/src/concepts/connectionTypes/fields/DropdownFormField.tsx b/frontend/src/concepts/connectionTypes/fields/DropdownFormField.tsx
@@ -19,18 +19,24 @@ const DropdownFormField: React.FC<FieldProps<DropdownField>> = ({
   const hasValidOption = field.properties.items?.find((f) => f.value || f.label);
 
   const menuToggleText = () => {
-    let text = field.name ? `Select ${field.name} ` : 'No values defined yet ';
-    if (!isMulti && isPreview) {
-      const defaultOption = field.properties.items?.find(
-        (i) => i.value === field.properties.defaultValue?.[0],
-      );
-      if (defaultOption) {
-        text = defaultOption.label || defaultOption.value;
-      }
-    } else if (!isMulti && !isPreview) {
-      const currentSelection = field.properties.items?.find((i) => value?.includes(i.value));
-      if (currentSelection) {
-        text = currentSelection.label || currentSelection.value;
+    let text = field.properties.items?.some((i) => i.label || i.value)
+      ? field.name
+        ? `Select ${field.name} `
+        : 'Select'
+      : 'No values defined';
+    if (!isMulti) {
+      if (isPreview) {
+        const defaultOption = field.properties.items?.find(
+          (i) => i.value === field.properties.defaultValue?.[0],
+        );
+        if (defaultOption) {
+          text = defaultOption.label || defaultOption.value;
+        }
+      } else {
+        const currentSelection = field.properties.items?.find((i) => value?.includes(i.value));
+        if (currentSelection) {
+          text = currentSelection.label || currentSelection.value;
+        }
       }
     }
     return text;
@@ -74,7 +80,7 @@ const DropdownFormField: React.FC<FieldProps<DropdownField>> = ({
             <>
               {menuToggleText()}
               {isMulti && (
-                <Badge>
+                <Badge className="pf-v5-u-ml-xs">
                   {(isPreview ? field.properties.defaultValue?.length : value?.length) ?? 0}{' '}
                   selected
                 </Badge>

diff --git a/frontend/src/pages/connectionTypes/ConnectionTypesTable.tsx b/frontend/src/pages/connectionTypes/ConnectionTypesTable.tsx
@@ -20,7 +20,7 @@ type Props = {
 
 const ConnectionTypesTable: React.FC<Props> = ({ connectionTypes, onUpdate }) => {
   const [filterData, setFilterData] = React.useState<FilterDataType>(initialFilterData);
-  const onClearFilters = React.useCallback(() => setFilterData(initialFilterData), [setFilterData]);
+  const onClearFilters = React.useCallback(() => setFilterData(initialFilterData), []);
 
   const [deleteConnectionType, setDeleteConnectionType] = React.useState<
     ConnectionTypeConfigMapObj | undefined
@@ -30,7 +30,7 @@ const ConnectionTypesTable: React.FC<Props> = ({ connectionTypes, onUpdate }) =>
     () =>
       connectionTypes.filter((connectionType) => {
         const keywordFilter = filterData.Keyword?.toLowerCase();
-        const createFilter = filterData['Created by']?.toLowerCase();
+        const createFilter = filterData.Creator?.toLowerCase();
         const categoryFilter = filterData.Category?.toLowerCase();
 
         if (
@@ -58,10 +58,6 @@ const ConnectionTypesTable: React.FC<Props> = ({ connectionTypes, onUpdate }) =>
     [connectionTypes, filterData],
   );
 
-  const resetFilters = () => {
-    setFilterData(initialFilterData);
-  };
-
   return (
     <>
       <Table
@@ -88,7 +84,7 @@ const ConnectionTypesTable: React.FC<Props> = ({ connectionTypes, onUpdate }) =>
           />
         }
         disableItemCount
-        emptyTableView={<DashboardEmptyTableView onClearFilters={resetFilters} />}
+        emptyTableView={<DashboardEmptyTableView onClearFilters={onClearFilters} />}
         id="connectionTypes-list-table"
       />
       {deleteConnectionType ? (

diff --git a/frontend/src/pages/connectionTypes/ConnectionTypesTableToolbar.tsx b/frontend/src/pages/connectionTypes/ConnectionTypesTableToolbar.tsx
@@ -42,11 +42,11 @@ const ConnectionTypesTableToolbar: React.FC<Props> = ({
             onChange={(_event, value) => onChange(value)}
           />
         ),
-        [ConnectionTypesOptions.createdBy]: ({ onChange, ...props }) => (
+        [ConnectionTypesOptions.creator]: ({ onChange, ...props }) => (
           <SearchInput
             {...props}
-            aria-label="Created by"
-            placeholder="Created by"
+            aria-label="Creator"
+            placeholder="Creator"
             onChange={(_event, value) => onChange(value)}
           />
         ),

diff --git a/frontend/src/pages/connectionTypes/const.ts b/frontend/src/pages/connectionTypes/const.ts
@@ -1,21 +1,21 @@
 export enum ConnectionTypesOptions {
   keyword = 'Keyword',
   category = 'Category',
-  createdBy = 'Created by',
+  creator = 'Creator',
 }
 
 export const options = {
   [ConnectionTypesOptions.keyword]: 'Keyword',
   [ConnectionTypesOptions.category]: 'Category',
-  [ConnectionTypesOptions.createdBy]: 'Created By',
+  [ConnectionTypesOptions.creator]: 'Creator',
 };
 
 export type FilterDataType = Record<ConnectionTypesOptions, string | undefined>;
 
 export const initialFilterData: Record<ConnectionTypesOptions, string | undefined> = {
   [ConnectionTypesOptions.keyword]: '',
   [ConnectionTypesOptions.category]: '',
-  [ConnectionTypesOptions.createdBy]: '',
+  [ConnectionTypesOptions.creator]: '',
 };
 
 export const categoryOptions = ['Object storage', 'Database', 'Model registry', 'URI'];
diff --git a/frontend/src/pages/connectionTypes/manage/ConnectionTypeFieldMoveModal.tsx b/frontend/src/pages/connectionTypes/manage/ConnectionTypeFieldMoveModal.tsx
@@ -57,8 +57,10 @@ export const ConnectionTypeMoveFieldToSectionModal: React.FC<Props> = ({
         />
       }
     >
-      Select the section heading that <b>{row.field.name}</b> will be moved to.
       <Form>
+        <div>
+          Select the section heading that <b>{row.field.name}</b> will be moved to.
+        </div>
         <FormGroup fieldId="sectionHeading" label="Section heading" isRequired>
           <SimpleSelect
             id="sectionHeading"

diff --git a/frontend/src/pages/connectionTypes/manage/ManageConnectionTypeFieldsTableRow.tsx b/frontend/src/pages/connectionTypes/manage/ManageConnectionTypeFieldsTableRow.tsx
@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { ExclamationCircleIcon } from '@patternfly/react-icons';
-import { ActionsColumn, Td, Tr } from '@patternfly/react-table';
-import { Button, Icon, Label, Switch } from '@patternfly/react-core';
+import { ActionsColumn, TableText, Td, Tr } from '@patternfly/react-table';
+import { Button, Flex, FlexItem, Icon, Label, Switch, Truncate } from '@patternfly/react-core';
 import {
   ConnectionTypeField,
   ConnectionTypeFieldType,
@@ -90,7 +90,7 @@ const ManageConnectionTypeFieldsTableRow: React.FC<Props> = ({
               },
               {
                 title: 'Duplicate',
-                onClick: () => onDuplicate({ ...row, name: `Duplicate of ${row.name}` }),
+                onClick: () => onDuplicate({ ...row, name: `Copy of ${row.name}` }),
               },
               {
                 title: 'Remove',
@@ -133,19 +133,26 @@ const ManageConnectionTypeFieldsTableRow: React.FC<Props> = ({
       <Td dataLabel={columns[1].label} data-testid="field-type">
         {fieldTypeToString(row.type)}
       </Td>
-      <Td dataLabel={columns[2].label} data-testid="field-default" modifier="truncate">
-        {defaultValueToString(row) || '-'}
+      <Td dataLabel={columns[2].label} data-testid="field-default">
+        <TableText wrapModifier="truncate">{defaultValueToString(row) || '-'}</TableText>
       </Td>
       <Td dataLabel={columns[3].label} data-testid="field-env">
-        {row.envVar || '-'}
-        {isEnvVarConflict ? (
-          <>
-            <Icon status="danger" size="sm" className="pf-v5-u-ml-xs">
-              <ExclamationCircleIcon />
-            </Icon>
-            <span className="pf-v5-u-screen-reader">This environment variable is in conflict.</span>
-          </>
-        ) : undefined}
+        <Flex gap={{ default: 'gapSm' }} flexWrap={{ default: 'nowrap' }}>
+          <FlexItem>
+            <Truncate content={row.envVar || '-'} />
+          </FlexItem>
+          {isEnvVarConflict ? (
+            <FlexItem>
+              <Icon
+                status="danger"
+                size="sm"
+                aria-label="This environment variable is in conflict."
+              >
+                <ExclamationCircleIcon />
+              </Icon>
+            </FlexItem>
+          ) : undefined}
+        </Flex>
       </Td>
       <Td dataLabel={columns[4].label}>
         <Switch
@@ -164,7 +171,7 @@ const ManageConnectionTypeFieldsTableRow: React.FC<Props> = ({
             },
             {
               title: 'Duplicate',
-              onClick: () => onDuplicate(row),
+              onClick: () => onDuplicate({ ...row, name: `Copy of ${row.name}` }),
             },
             ...(showMoveToSection
               ? [

diff --git a/...nd/src/pages/connectionTypes/manage/__tests__/ManageConnectionTypeFieldsTableRow.spec.tsx b/...nd/src/pages/connectionTypes/manage/__tests__/ManageConnectionTypeFieldsTableRow.spec.tsx
@@ -48,9 +48,9 @@ describe('ManageConnectionTypeFieldsTableRow', () => {
     };
 
     const result = render(renderRow({ row: field, fields: [field] }));
-    expect(screen.getByTestId('field-env')).not.toHaveTextContent('conflict');
+    expect(screen.queryByLabelText(/conflict/)).not.toBeInTheDocument();
 
     result.rerender(renderRow({ row: field, fields: [field, field2] }));
-    expect(screen.getByTestId('field-env')).toHaveTextContent('conflict');
+    expect(screen.queryByLabelText(/conflict/)).toBeInTheDocument();
   });
 });
diff --git a/frontend/src/pages/connectionTypes/manage/fieldTableColumns.ts b/frontend/src/pages/connectionTypes/manage/fieldTableColumns.ts
@@ -1,7 +1,7 @@
 import { ThProps } from '@patternfly/react-table';
 
 export const columns: ThProps[] = [
-  { label: 'Section heading/field name', width: 30 },
+  { label: 'Section heading / field name', width: 30 },
   { label: 'Type', width: 10 },
   { label: 'Default value', width: 30 },
   { label: 'Environment variable', width: 20 },