Update 'Resource name' fields to meet UX guidelines: Cluster storage

frontend/src/api/k8s/pvcs.ts

@@ -21,7 +21,7 @@
   hideFromUI?: boolean,
 ): PersistentVolumeClaimKind => {
   const { name: pvcName, description, size, storageClassName } = data;
-  const name = editName || translateDisplayNameForK8s(pvcName);
+  const name = editName || data.k8sName || translateDisplayNameForK8s(pvcName);
 
   return {
     apiVersion: 'v1',
@@ -85,8 +85,13 @@
 ): Promise<PersistentVolumeClaimKind> => {
   const pvc = assemblePvc(data, namespace, existingData.metadata.name);
 
+  const pvcResource = _.merge({}, existingData, pvc);
+  if (!data.description && pvcResource.metadata.annotations?.['openshift.io/description']) {
+    pvcResource.metadata.annotations['openshift.io/description'] = undefined;
+  }
+
   return k8sUpdateResource<PersistentVolumeClaimKind>(
-    applyK8sAPIOptions({ model: PVCModel, resource: _.merge({}, existingData, pvc) }, opts),
+    applyK8sAPIOptions({ model: PVCModel, resource: pvcResource }, opts),
   );
 };
 

frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx

@@ -2,7 +2,6 @@ import * as React from 'react';
 import {
   Button,
   FormGroup,
-  FormHelperText,
   HelperText,
   HelperTextItem,
   TextArea,
@@ -75,29 +74,31 @@ const K8sNameDescriptionField: React.FC<K8sNameDescriptionFieldProps> = ({
           value={name}
           onChange={(event, value) => onDataChange?.('name', value)}
         />
-        {!showK8sField && !k8sName.state.immutable && (
-          <FormHelperText>
-            <HelperText>
-              {nameHelperText}
-              {k8sName.value && (
+        {nameHelperText || (!showK8sField && !k8sName.state.immutable) ? (
+          <HelperText>
+            {nameHelperText && <HelperTextItem>{nameHelperText}</HelperTextItem>}
+            {!showK8sField && !k8sName.state.immutable && (
+              <>
+                {k8sName.value && (
+                  <HelperTextItem>
+                    The resource name will be <b>{k8sName.value}</b>.
+                  </HelperTextItem>
+                )}
                 <HelperTextItem>
-                  The resource name will be <b>{k8sName.value}</b>.
+                  <Button
+                    data-testid={`${dataTestId}-editResourceLink`}
+                    variant="link"
+                    isInline
+                    onClick={() => setShowK8sField(true)}
+                  >
+                    Edit resource name
+                  </Button>{' '}
+                  <ResourceNameDefinitionTooltip />
                 </HelperTextItem>
-              )}
-              <HelperTextItem>
-                <Button
-                  data-testid={`${dataTestId}-editResourceLink`}
-                  variant="link"
-                  isInline
-                  onClick={() => setShowK8sField(true)}
-                >
-                  Edit resource name
-                </Button>{' '}
-                <ResourceNameDefinitionTooltip />
-              </HelperTextItem>
-            </HelperText>
-          </FormHelperText>
-        )}
+              </>
+            )}
+          </HelperText>
+        ) : null}
       </FormGroup>
       <ResourceNameField
         allowEdit={showK8sField}

frontend/src/concepts/k8s/K8sNameDescriptionField/types.ts

@@ -60,6 +60,8 @@ export type UseK8sNameDescriptionDataConfiguration = {
   regexp?: RegExp;
   /** Optional invalid characters message */
   invalidCharsMessage?: string;
+  /** allow the k8sName value to be edited even though it is pre-set */
+  editableK8sName?: boolean;
 };
 
 type K8sNameDescriptionFieldUpdateFunctionTemplate<T> = (

frontend/src/concepts/k8s/K8sNameDescriptionField/utils.ts

@@ -46,6 +46,7 @@
   staticPrefix,
   regexp,
   invalidCharsMessage,
+  editableK8sName,
 }: UseK8sNameDescriptionDataConfiguration): K8sNameDescriptionFieldData => {
   let initialName = '';
   let initialDescription = '';
@@ -72,15 +73,18 @@
     k8sName: {
       value: initialK8sNameValue,
       state: {
-        immutable: initialK8sNameValue !== '',
+        immutable: !editableK8sName && initialK8sNameValue !== '',
         invalidCharacters: false,
         invalidLength: false,
         maxLength: configuredMaxLength,
         safePrefix,
         staticPrefix,
         regexp,
         invalidCharsMessage,
-        touched: false,
+        touched:
+          !!editableK8sName &&
+          initialK8sNameValue !== '' &&
+          initialK8sNameValue !== translateDisplayNameForK8s(initialName),
       },
     },
   })('name', initialName) satisfies K8sNameDescriptionFieldData;

frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx

@@ -41,7 +41,8 @@ const BaseStorageModal: React.FC<BaseStorageModalProps> = ({
   onClose,
   onNameChange,
 }) => {
-  const [createData, setCreateData, resetData] = useCreateStorageObject(existingPvc, existingData);
+  const [createData, setCreateData] = useCreateStorageObject(existingPvc, existingData);
+  const [nameDescValid, setNameDescValid] = React.useState<boolean>();
   const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status;
   const preferredStorageClass = usePreferredStorageClass();
   const [defaultStorageClass] = useDefaultStorageClass();
@@ -64,21 +65,14 @@ const BaseStorageModal: React.FC<BaseStorageModalProps> = ({
     setCreateData,
   ]);
 
-  const onBeforeClose = (submitted: boolean) => {
-    onClose(submitted);
-    setError(undefined);
-    setActionInProgress(false);
-    resetData();
-  };
-
-  const canCreate = !actionInProgress && createData.name.trim().length > 0 && isValid;
+  const canCreate = !actionInProgress && nameDescValid && isValid;
 
   const submit = () => {
     setError(undefined);
     setActionInProgress(true);
 
     onSubmit(createData)
-      .then(() => onBeforeClose(true))
+      .then(() => onClose(true))
       .catch((err) => {
         setError(err);
         setActionInProgress(false);
@@ -91,13 +85,13 @@ const BaseStorageModal: React.FC<BaseStorageModalProps> = ({
       description={description}
       variant="medium"
       isOpen
-      onClose={() => onBeforeClose(false)}
+      onClose={() => onClose(false)}
       showClose
       footer={
         <DashboardModalFooter
           submitLabel={submitLabel}
           onSubmit={submit}
-          onCancel={() => onBeforeClose(false)}
+          onCancel={() => onClose(false)}
           isSubmitDisabled={!canCreate}
           error={error}
           alertTitle="Error creating storage"
@@ -118,8 +112,10 @@ const BaseStorageModal: React.FC<BaseStorageModalProps> = ({
               currentStatus={existingPvc?.status}
               autoFocusName
               onNameChange={onNameChange}
+              setValid={setNameDescValid}
               hasDuplicateName={hasDuplicateName}
               disableStorageClassSelect={!!existingPvc}
+              editableK8sName={!existingPvc}
             />
           </StackItem>
           {children}

frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { StackItem } from '@patternfly/react-core';
+import { FormGroup } from '@patternfly/react-core';
 import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes';
 import { ForNotebookSelection, StorageData } from '~/pages/projects/types';
 import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext';
@@ -133,30 +133,26 @@
       {workbenchEnabled && (
         <>
           {hasExistingNotebookConnections && (
-            <StackItem>
-              <ExistingConnectedNotebooks
-                connectedNotebooks={removableNotebooks}
-                onNotebookRemove={(notebook: NotebookKind) =>
-                  setRemovedNotebooks([...removedNotebooks, notebook.metadata.name])
-                }
-                loaded={removableNotebookLoaded}
-                error={removableNotebookError}
-              />
-            </StackItem>
-          )}
-          <StackItem>
-            <StorageNotebookConnections
-              setForNotebookData={(forNotebookData) => {
-                setNotebookData(forNotebookData);
-              }}
-              forNotebookData={notebookData}
-              connectedNotebooks={connectedNotebooks}
+            <ExistingConnectedNotebooks
+              connectedNotebooks={removableNotebooks}
+              onNotebookRemove={(notebook: NotebookKind) =>
+                setRemovedNotebooks([...removedNotebooks, notebook.metadata.name])
+              }
+              loaded={removableNotebookLoaded}
+              error={removableNotebookError}
             />
-          </StackItem>
+          )}
+          <StorageNotebookConnections
+            setForNotebookData={(forNotebookData) => {
+              setNotebookData(forNotebookData);
+            }}
+            forNotebookData={notebookData}
+            connectedNotebooks={connectedNotebooks}
+          />
           {restartNotebooks.length !== 0 && (
-            <StackItem>
+            <FormGroup>
               <NotebookRestartAlert notebooks={restartNotebooks} />
-            </StackItem>
+            </FormGroup>
           )}
         </>
       )}

frontend/src/pages/projects/screens/detail/storage/utils.ts

@@ -1,4 +1,4 @@
-import { getDisplayNameFromK8sResource, getDescriptionFromK8sResource } from '~/concepts/k8s/utils';
+import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
 import { PersistentVolumeClaimKind } from '~/k8sTypes';
 import { StorageData } from '~/pages/projects/types';
 

frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx

@@ -1,11 +1,13 @@
 import * as React from 'react';
-import { Stack, StackItem } from '@patternfly/react-core';
+import { FormSection, HelperTextItem } from '@patternfly/react-core';
 import { StorageData, UpdateObjectAtPropAndValue } from '~/pages/projects/types';
 import PVSizeField from '~/pages/projects/components/PVSizeField';
-import NameDescriptionField from '~/concepts/k8s/NameDescriptionField';
 import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas';
 import { PersistentVolumeClaimKind } from '~/k8sTypes';
-import { DuplicateNameHelperText } from '~/concepts/pipelines/content/DuplicateNameHelperText';
+import K8sNameDescriptionField, {
+  useK8sNameDescriptionFieldData,
+} from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField';
+import { isK8sNameDescriptionDataValid } from '~/concepts/k8s/K8sNameDescriptionField/utils';
 import StorageClassSelect from './StorageClassSelect';
 
 type CreateNewStorageSectionProps<D extends StorageData> = {
@@ -16,7 +18,9 @@
   menuAppendTo?: HTMLElement;
   disableStorageClassSelect?: boolean;
   onNameChange?: (value: string) => void;
+  setValid?: (isValid: boolean) => void;
   hasDuplicateName?: boolean;
+  editableK8sName?: boolean;
 };
 
 const CreateNewStorageSection = <D extends StorageData>({
@@ -27,49 +31,62 @@
   autoFocusName,
   disableStorageClassSelect,
   onNameChange,
+  setValid,
   hasDuplicateName,
+  editableK8sName,
 }: CreateNewStorageSectionProps<D>): React.ReactNode => {
   const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status;
+  const { data: clusterStorageNameDesc, onDataChange: setClusterNameDesc } =
+    useK8sNameDescriptionFieldData({
+      initialData: {
+        name: data.name,
+        k8sName: data.k8sName,
+        description: data.description,
+      },
+      editableK8sName,
+    });
+
+  React.useEffect(() => {
+    setData('name', clusterStorageNameDesc.name);
+    setData('k8sName', clusterStorageNameDesc.k8sName.value);
+    setData('description', clusterStorageNameDesc.description);
+    onNameChange?.(clusterStorageNameDesc.name);
+    setValid?.(isK8sNameDescriptionDataValid(clusterStorageNameDesc));
+    // only update if the name description changes
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [clusterStorageNameDesc]);
 
   return (
-    <Stack hasGutter>
-      <StackItem>
-        <NameDescriptionField
-          nameFieldId="create-new-storage-name"
-          descriptionFieldId="create-new-storage-description"
-          data={{ name: data.name, description: data.description || '' }}
-          setData={(newData) => {
-            setData('name', newData.name);
-            setData('description', newData.description);
-          }}
-          onNameChange={onNameChange}
-          hasNameError={hasDuplicateName}
-          nameHelperText={
-            hasDuplicateName ? <DuplicateNameHelperText isError name={data.name} /> : undefined
-          }
-          autoFocusName={autoFocusName}
-        />
-      </StackItem>
-      <StackItem>
-        {isStorageClassesAvailable && (
-          <StorageClassSelect
-            storageClassName={data.storageClassName}
-            setStorageClassName={(name) => setData('storageClassName', name)}
-            disableStorageClassSelect={disableStorageClassSelect}
-            menuAppendTo={menuAppendTo}
-          />
-        )}
-      </StackItem>
-      <StackItem>
-        <PVSizeField
+    <FormSection>
+      <K8sNameDescriptionField
+        data={clusterStorageNameDesc}
+        onDataChange={setClusterNameDesc}
+        dataTestId="create-new-storage"
+        autoFocusName={autoFocusName}
+        nameHelperText={
+          hasDuplicateName ? (
+            <HelperTextItem variant="error" hasIcon>
+              <b>{data.name}</b> already exists. Try a different name.
+            </HelperTextItem>
+          ) : undefined
+        }
+      />
+      {isStorageClassesAvailable && (
+        <StorageClassSelect
+          storageClassName={data.storageClassName}
+          setStorageClassName={(name) => setData('storageClassName', name)}
+          disableStorageClassSelect={disableStorageClassSelect}
           menuAppendTo={menuAppendTo}
-          fieldID="create-new-storage-size"
-          currentStatus={currentStatus}
-          size={String(data.size)}
-          setSize={(size) => setData('size', size)}
         />
-      </StackItem>
-    </Stack>
+      )}
+      <PVSizeField
+        menuAppendTo={menuAppendTo}
+        fieldID="create-new-storage-size"
+        currentStatus={currentStatus}
+        size={String(data.size)}
+        setSize={(size) => setData('size', size)}
+      />
+    </FormSection>
   );
 };
 

frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx

@@ -1,5 +1,4 @@
 import * as React from 'react';
-import { StackItem } from '@patternfly/react-core';
 import { MountPath, StorageData } from '~/pages/projects/types';
 import BaseStorageModal from '~/pages/projects/screens/detail/storage/BaseStorageModal';
 import SpawnerMountPathField from './SpawnerMountPathField';
@@ -60,13 +59,11 @@ const WorkbenchStorageModal: React.FC<WorkbenchStorageModalProps> = ({
       isValid={!actionInProgress && !mountPath.error && !hasDuplicateName}
       onClose={onClose}
     >
-      <StackItem>
-        <SpawnerMountPathField
-          mountPath={mountPath}
-          inUseMountPaths={existingMountPaths}
-          onChange={setMountPath}
-        />
-      </StackItem>
+      <SpawnerMountPathField
+        mountPath={mountPath}
+        inUseMountPaths={existingMountPaths}
+        onChange={setMountPath}
+      />
     </BaseStorageModal>
   );
 };