address comments

opendatahub-io · Sep 30, 2022 · feff41f · feff41f
1 parent 347a945
commit feff41f
Show file tree

Hide file tree

Showing 6 changed files with 149 additions and 165 deletions.
diff --git a/frontend/src/pages/projects/modals/addStorageModal/AddExistingStorageForm.tsx b/frontend/src/pages/projects/modals/addStorageModal/AddExistingStorageForm.tsx
@@ -1,66 +1,74 @@
 import * as React from 'react';
-import { Select, Form, FormGroup, FlexItem, Flex } from '@patternfly/react-core';
+import { Select, Form, FormGroup } from '@patternfly/react-core';
 import { ExistingStorage } from './types';
 
 type AddExistingStorageFormProps = {
   selections: ExistingStorage;
-  onClear: (storageOnly: boolean) => void;
-  onUpdate: (selection: string, storageOnly: boolean) => void;
+  setSelections: (selections: ExistingStorage) => void;
 };
 
 const AddExistingStorageForm: React.FC<AddExistingStorageFormProps> = ({
   selections,
-  onClear,
-  onUpdate,
+  setSelections,
 }) => {
   const [projectSelectOpen, setProjectSelectOpen] = React.useState<boolean>(false);
   const [storageSelectOpen, setStorageSelectOpen] = React.useState<boolean>(false);
 
   return (
     <Form>
-      <Flex direction={{ default: 'column' }} spaceItems={{ default: 'spaceItemsSm' }}>
-        <FlexItem>
-          <FormGroup
-            label="Project where the PV resides"
-            fieldId="add-existing-storage-project-selection"
-          >
-            <Select
-              variant="typeahead"
-              selections={selections.project as string}
-              isOpen={projectSelectOpen}
-              onClear={() => {
-                onClear(false);
-                setProjectSelectOpen(false);
-              }}
-              onSelect={(e, selection) => {
-                onUpdate(selection as string, false);
-              }}
-              onToggle={(isOpen) => setProjectSelectOpen(isOpen)}
-              placeholderText="Select a project"
-              menuAppendTo="parent"
-            ></Select>
-          </FormGroup>
-        </FlexItem>
-        <FlexItem>
-          <FormGroup label="PV" fieldId="add-existing-storage-pv-selection">
-            <Select
-              variant="typeahead"
-              selections={selections.storage as string}
-              isOpen={storageSelectOpen}
-              onClear={() => {
-                onClear(true);
-                setStorageSelectOpen(false);
-              }}
-              onSelect={(e, selection) => {
-                onUpdate(selection as string, true);
-              }}
-              onToggle={(isOpen) => setStorageSelectOpen(isOpen)}
-              placeholderText="Select a PV"
-              menuAppendTo="parent"
-            ></Select>
-          </FormGroup>
-        </FlexItem>
-      </Flex>
+      <FormGroup
+        label="Project where the PV resides"
+        fieldId="add-existing-storage-project-selection"
+      >
+        <Select
+          variant="typeahead"
+          selections={selections.project}
+          isOpen={projectSelectOpen}
+          onClear={() => {
+            setSelections({
+              project: undefined,
+              storage: undefined,
+            });
+            setProjectSelectOpen(false);
+          }}
+          onSelect={(e, selection) => {
+            if (typeof selection === 'string') {
+              setSelections({
+                project: selection,
+                storage: undefined,
+              });
+            }
+          }}
+          onToggle={(isOpen) => setProjectSelectOpen(isOpen)}
+          placeholderText="Select a project"
+          menuAppendTo="parent"
+        />
+      </FormGroup>
+      <FormGroup label="PV" fieldId="add-existing-storage-pv-selection">
+        <Select
+          variant="typeahead"
+          selections={selections.storage}
+          isOpen={storageSelectOpen}
+          onClear={() => {
+            setSelections({
+              ...selections,
+              storage: undefined,
+            });
+            setStorageSelectOpen(false);
+          }}
+          onSelect={(e, selection) => {
+            if (typeof selection === 'string') {
+              setSelections({
+                ...selections,
+                storage: selection,
+              });
+            }
+          }}
+          onToggle={(isOpen) => setStorageSelectOpen(isOpen)}
+          placeholderText="Select a PV"
+          menuAppendTo="parent"
+        />
+      </FormGroup>
     </Form>
   );
 };

diff --git a/frontend/src/pages/projects/modals/addStorageModal/AddStorageModal.tsx b/frontend/src/pages/projects/modals/addStorageModal/AddStorageModal.tsx
@@ -2,7 +2,6 @@ import * as React from 'react';
 import { Button, Modal, Radio, Stack, StackItem } from '@patternfly/react-core';
 import { DEFAULT_PVC_SIZE } from './const';
 import CreateNewStorageForm from './CreateNewStorageForm';
-import ConnectWorkbenchOptions from './ConnectWorkbenchOptions';
 import AddExistingStorageForm from './AddExistingStorageForm';
 import { ExistingStorage } from './types';
 
@@ -14,47 +13,25 @@ type AddStorageModalProps = {
 };
 
 const AddStorageModal: React.FC<AddStorageModalProps> = ({ isOpen, onClose }) => {
-  const [isCreateNewPV, setCreateNewPV] = React.useState<boolean>(true);
+  const [storageType, setStorageType] = React.useState<'create' | 'existing'>('create');
 
   // states for creating new PV
   const [name, setName] = React.useState<string>('');
   const [description, setDescription] = React.useState<string>('');
   const [size, setSize] = React.useState<string>(DEFAULT_PVC_SIZE);
-  const [workbenchSelection, setWorkbenchSelection] = React.useState<string | string[] | null>([]);
+  const [workbenchSelections, setWorkbenchSelections] = React.useState<string[]>([]);
 
   // states for adding existing PV
-  const [projectSelection, setProjectSelection] = React.useState<string | null>(null);
-  const [storageSelection, setStorageSelection] = React.useState<string | null>(null);
-  const existingSelections: ExistingStorage = React.useMemo(
-    () => ({ project: projectSelection, storage: storageSelection }),
-    [projectSelection, storageSelection],
-  );
-
-  const updateWorkbench = (workbench: string | string[] | null) => {
-    setWorkbenchSelection(workbench);
-  };
-
-  const updateExistingStorage = (selection: string, storageOnly: boolean) => {
-    if (!storageOnly) {
-      setProjectSelection(selection);
-      setStorageSelection(null);
-    } else {
-      setStorageSelection(selection);
-    }
-  };
-
-  const clearExistingStorage = (storageOnly: boolean) => {
-    if (!storageOnly) {
-      setProjectSelection(null);
-    }
-    setStorageSelection(null);
-  };
+  const [existingSelections, setExistingSelections] = React.useState<ExistingStorage>({
+    project: undefined,
+    storage: undefined,
+  });
 
   return (
     <Modal
       title="Add storage"
       description="Add and connect storage to your cluster"
-      variant="large"
+      variant="medium"
       isOpen={isOpen}
       onClose={onClose}
       showClose
@@ -74,24 +51,19 @@ const AddStorageModal: React.FC<AddStorageModalProps> = ({ isOpen, onClose }) =>
             id="create-new-storage-radio"
             name="create-new-storage-radio"
             label="Create new PV"
-            isChecked={isCreateNewPV}
-            onChange={() => setCreateNewPV(true)}
+            isChecked={storageType === 'create'}
+            onChange={() => setStorageType('create')}
             body={
-              isCreateNewPV && (
+              storageType === 'create' && (
                 <CreateNewStorageForm
                   name={name}
                   setName={setName}
                   description={description}
                   setDescription={setDescription}
                   size={size}
                   setSize={setSize}
-                  workbenchOptions={
-                    <ConnectWorkbenchOptions
-                      allWorkbenches={[]}
-                      workbenchSelection={workbenchSelection}
-                      onUpdate={updateWorkbench}
-                    />
-                  }
+                  selections={workbenchSelections}
+                  setSelections={setWorkbenchSelections}
                 />
               )
             }
@@ -103,14 +75,13 @@ const AddStorageModal: React.FC<AddStorageModalProps> = ({ isOpen, onClose }) =>
             id="add-existing-storage-radio"
             name="add-existing-storage-radio"
             label="Add exisiting PV"
-            isChecked={!isCreateNewPV}
-            onChange={() => setCreateNewPV(false)}
+            isChecked={storageType === 'existing'}
+            onChange={() => setStorageType('existing')}
             body={
-              !isCreateNewPV && (
+              storageType === 'existing' && (
                 <AddExistingStorageForm
                   selections={existingSelections}
-                  onClear={clearExistingStorage}
-                  onUpdate={updateExistingStorage}
+                  setSelections={setExistingSelections}
                 />
               )
             }

diff --git a/frontend/src/pages/projects/modals/addStorageModal/ConnectWorkbenchOptions.tsx b/frontend/src/pages/projects/modals/addStorageModal/ConnectWorkbenchOptions.tsx
@@ -1,61 +1,69 @@
 import * as React from 'react';
-import { Flex, FlexItem, Radio, Select } from '@patternfly/react-core';
+import { Radio, Select, Stack, StackItem } from '@patternfly/react-core';
 
 type ConnectWorkbenchOptionsProps = {
   allWorkbenches: string[]; // maybe workbench type in the future, take string now
-  workbenchSelection: string | string[] | null;
-  onUpdate: (workbench: string | string[] | null) => void;
+  selections: string[];
+  setSelections: (selections: string[]) => void;
 };
 
 const ConnectWorkbenchOptions: React.FC<ConnectWorkbenchOptionsProps> = ({
   allWorkbenches,
-  workbenchSelection,
-  onUpdate,
+  selections,
+  setSelections,
 }) => {
   const [workbenchSelectOpen, setWorkbenchSelectOpen] = React.useState<boolean>(false);
-  const isConnectToAll = Array.isArray(workbenchSelection);
+  const [isConnectToAll, setConnectToAll] = React.useState<boolean>(true);
   return (
-    <Flex direction={{ default: 'column' }} spaceItems={{ default: 'spaceItemsSm' }}>
-      <FlexItem>
+    <Stack hasGutter>
+      <StackItem>
         <Radio
           id="connect-to-all-workbenches-radio"
           name="connect-to-all-workbenches-radio"
           label="Connect to all workbenches"
           isChecked={isConnectToAll}
-          onChange={() => onUpdate(allWorkbenches)}
+          onChange={() => {
+            setConnectToAll(true);
+            setSelections(allWorkbenches);
+          }}
         />
-      </FlexItem>
-      <FlexItem>
+      </StackItem>
+      <StackItem>
         <Radio
           className="radio-fix-body-width"
           id="connect-to-specific-workbench-radio"
           name="connect-to-specific-workbench-radio"
           label="Connect to a specific workbench"
           isChecked={!isConnectToAll}
-          onChange={() => onUpdate(null)}
+          onChange={() => {
+            setConnectToAll(false);
+            setSelections([]);
+          }}
           body={
             !isConnectToAll && (
               <Select
                 variant="typeahead"
-                selections={workbenchSelection as string}
+                selections={selections}
                 isOpen={workbenchSelectOpen}
                 onClear={() => {
-                  onUpdate(null);
+                  setSelections([]);
                   setWorkbenchSelectOpen(false);
                 }}
                 onSelect={(e, selection) => {
-                  onUpdate(selection as string);
-                  setWorkbenchSelectOpen(false);
+                  if (typeof selection === 'string') {
+                    setSelections([selection]);
+                    setWorkbenchSelectOpen(false);
+                  }
                 }}
                 onToggle={(isOpen) => setWorkbenchSelectOpen(isOpen)}
                 placeholderText="Choose an existing workbench"
                 menuAppendTo="parent"
-              ></Select>
+              />
             )
           }
         />
-      </FlexItem>
-    </Flex>
+      </StackItem>
+    </Stack>
   );
 };