feat: WD-17682 review changes no.1 (To be squashed)

Signed-off-by: Mason Hu <[email protected]>
canonical · Dec 12, 2024 · f0e07a8 · f0e07a8
1 parent 0846d1f
commit f0e07a8
Show file tree

Hide file tree

Showing 12 changed files with 145 additions and 127 deletions.
diff --git a/src/components/DeviceListTable.tsx b/src/components/DeviceListTable.tsx
@@ -63,7 +63,7 @@ const DeviceListTable: FC<Props> = ({ configBaseURL, devices }) => {
       }
 
       if (isHostDiskDevice(device)) {
-        deviceType += " (host)";
+        deviceType += " (host path)";
       }
     }
 

diff --git a/src/components/forms/DiskDeviceFormCustom.tsx b/src/components/forms/DiskDeviceFormCustom.tsx
@@ -22,7 +22,7 @@ import {
   isRootDisk,
 } from "util/instanceValidation";
 import { ensureEditMode } from "util/instanceEdit";
-import { getExistingDeviceNames, isVolumeDevice } from "util/devices";
+import { getExistingDeviceNames, isVolume, isVolumeDevice } from "util/devices";
 import { LxdProfile } from "types/profile";
 import { focusField } from "util/formFields";
 import AttachDiskDeviceBtn from "pages/storage/AttachDiskDeviceBtn";
@@ -40,13 +40,12 @@ const DiskDeviceFormCustom: FC<Props> = ({ formik, project, profiles }) => {
 
   const addDiskDevice = (device: LxdStorageVolume | LxdDiskDevice) => {
     const copy = [...formik.values.devices];
-    const isVolume = "content_type" in device;
     const newDevice: FormDevice = {
       type: "disk",
       name: deduplicateName("disk-device", 1, existingDeviceNames),
     };
 
-    if (isVolume) {
+    if (isVolume(device)) {
       newDevice.pool = device.pool;
       newDevice.source = device.name;
       newDevice.path = device.content_type === "filesystem" ? "" : undefined;
@@ -67,8 +66,7 @@ const DiskDeviceFormCustom: FC<Props> = ({ formik, project, profiles }) => {
     formDiskDevice: FormDiskDevice,
     index: number,
   ) => {
-    const isVolume = "content_type" in diskDevice;
-    if (isVolume) {
+    if (isVolume(diskDevice)) {
       void formik.setFieldValue(`devices.${index}.pool`, diskDevice.pool);
       void formik.setFieldValue(`devices.${index}.source`, diskDevice.name);
 
@@ -81,12 +79,10 @@ const DiskDeviceFormCustom: FC<Props> = ({ formik, project, profiles }) => {
       if (diskDevice.content_type === "block") {
         void formik.setFieldValue(`devices.${index}.path`, undefined);
       }
-
-      return;
+    } else {
+      void formik.setFieldValue(`devices.${index}.source`, diskDevice.source);
+      void formik.setFieldValue(`devices.${index}.path`, diskDevice.path);
     }
-
-    void formik.setFieldValue(`devices.${index}.source`, diskDevice.source);
-    void formik.setFieldValue(`devices.${index}.path`, diskDevice.path);
   };
 
   const editButton = (fieldName: string) => (
@@ -138,75 +134,79 @@ const DiskDeviceFormCustom: FC<Props> = ({ formik, project, profiles }) => {
       }),
     );
 
-    const volumeDeviceSource = getConfigurationRowBase({
-      className: "no-border-top inherited-with-form",
-      configuration: (
-        <Label forId={`devices.${index}.pool`}>Pool / volume</Label>
-      ),
-      inherited: (
-        <div className="custom-disk-volume-source">
-          <div
-            className={classnames("mono-font", "u-truncate")}
-            title={`${item.pool} / ${item.source ?? ""}`}
-          >
-            <b>
-              {item.pool} / {item.source}
-            </b>
+    const volumeDeviceSource = () =>
+      getConfigurationRowBase({
+        className: "no-border-top inherited-with-form",
+        configuration: (
+          <Label forId={`devices.${index}.pool`}>Pool / volume</Label>
+        ),
+        inherited: (
+          <div className="custom-disk-volume-source">
+            <div
+              className={classnames("mono-font", "u-truncate")}
+              title={`${item.pool} / ${item.source ?? ""}`}
+            >
+              <b>
+                {item.pool} / {item.source}
+              </b>
+            </div>
+            <CustomVolumeSelectBtn
+              formik={formik}
+              project={project}
+              setValue={(volume) => {
+                ensureEditMode(formik);
+                changeDiskDevice(volume, item, index);
+              }}
+              buttonProps={{
+                id: `devices.${index}.pool`,
+                appearance: "base",
+                className: "u-no-margin--bottom",
+                title: "Select storage volume",
+                dense: true,
+              }}
+            >
+              <Icon name="edit" />
+            </CustomVolumeSelectBtn>
           </div>
-          <CustomVolumeSelectBtn
-            formik={formik}
-            project={project}
-            setValue={(volume) => {
-              ensureEditMode(formik);
-              changeDiskDevice(volume, item, index);
-            }}
-            buttonProps={{
-              id: `devices.${index}.pool`,
-              appearance: "base",
-              className: "u-no-margin--bottom",
-              title: "Select storage volume",
-              dense: true,
-            }}
-          >
-            <Icon name="edit" />
-          </CustomVolumeSelectBtn>
-        </div>
-      ),
-      override: "",
-    });
+        ),
+        override: "",
+      });
 
-    const hostDeviceSource = getConfigurationRowBase({
-      className: "no-border-top inherited-with-form",
-      configuration: <Label forId={`devices.${index}.source`}>Host path</Label>,
-      inherited: readOnly ? (
-        <div className="custom-disk-read-mode">
-          <div className="mono-font custom-disk-value u-truncate">
-            <b>{item.source}</b>
+    const hostDeviceSource = () =>
+      getConfigurationRowBase({
+        className: "no-border-top inherited-with-form",
+        configuration: (
+          <Label forId={`devices.${index}.source`}>Host path</Label>
+        ),
+        inherited: readOnly ? (
+          <div className="custom-disk-read-mode">
+            <div className="mono-font custom-disk-value u-truncate">
+              <b>{item.source}</b>
+            </div>
+            {editButton(`devices.${index}.source`)}
           </div>
-          {editButton(`devices.${index}.source`)}
-        </div>
-      ) : (
-        <Input
-          id={`devices.${index}.source`}
-          name={`devices.${index}.source`}
-          onBlur={formik.handleBlur}
-          onChange={(e) => {
-            void formik.setFieldValue(
-              `devices.${index}.source`,
-              e.target.value,
-            );
-          }}
-          value={item.source}
-          type="text"
-          placeholder="Enter full host path (e.g. /data)"
-          className={!item.source ? undefined : "u-no-margin--bottom"}
-          error={!item.source ? "Host path is required" : undefined}
-        />
-      ),
-      override: "",
-    });
+        ) : (
+          <Input
+            id={`devices.${index}.source`}
+            name={`devices.${index}.source`}
+            onBlur={formik.handleBlur}
+            onChange={(e) => {
+              void formik.setFieldValue(
+                `devices.${index}.source`,
+                e.target.value,
+              );
+            }}
+            value={item.source}
+            type="text"
+            placeholder="Enter full host path (e.g. /data)"
+            className={!item.source ? undefined : "u-no-margin--bottom"}
+            error={!item.source ? "Host path is required" : undefined}
+          />
+        ),
+        override: "",
+      });
 
-    rows.push(isVolumeDevice(item) ? volumeDeviceSource : hostDeviceSource);
+    rows.push(isVolumeDevice(item) ? volumeDeviceSource() : hostDeviceSource());
 
     if (!isVolumeDevice(item) || item.path !== undefined) {
       const hasError = isDiskDeviceMountPointMissing(formik, index);

diff --git a/src/pages/instances/actions/DetachDiskDeviceBtn.tsx b/src/pages/instances/actions/DetachDiskDeviceBtn.tsx
@@ -10,10 +10,10 @@ const DetachDiskDeviceBtn: FC<Props> = ({ onDetach }) => {
     <ConfirmationButton
       appearance="base"
       type="button"
-      title="Detach disk devicve"
+      title="Detach disk"
       className="has-icon u-no-margin--bottom is-dense"
       confirmationModalProps={{
-        title: "Confirm disk device detach",
+        title: "Confirm disk detachment",
         children: (
           <p>
             Are you sure you want to clear this disk attachment?

diff --git a/src/pages/storage/AttachDiskDeviceModal.tsx b/src/pages/storage/AttachDiskDeviceModal.tsx
@@ -8,7 +8,7 @@ import { LxdStorageVolume } from "types/storage";
 import { LxdDiskDevice } from "types/device";
 import HostPathDeviceModal from "./HostPathDeviceModal";
 
-type DiskDeviceType = "custom volume" | "host path" | "";
+type DiskDeviceType = "custom volume" | "host path" | "choose type";
 
 interface Props {
   close: () => void;
@@ -23,7 +23,7 @@ const AttachDiskDeviceModal: FC<Props> = ({
   project,
   onFinish,
 }) => {
-  const [type, setType] = useState<DiskDeviceType>("");
+  const [type, setType] = useState<DiskDeviceType>("choose type");
 
   const handleEscKey = (e: KeyboardEvent<HTMLElement>) => {
     if (e.key === "Escape") {
@@ -32,45 +32,42 @@ const AttachDiskDeviceModal: FC<Props> = ({
   };
 
   const handleGoBack = () => {
-    if (type) {
-      setType("");
-      return;
-    }
+    setType("choose type");
+    return;
   };
 
-  const modalTitle = !type ? (
-    "Choose disk device type"
-  ) : (
-    <BackLink
-      title={`Attach ${type} device`}
-      onClick={handleGoBack}
-      linkText={"Choose disk device type"}
-    />
-  );
+  const modalTitle =
+    type === "choose type" ? (
+      "Choose disk type"
+    ) : (
+      <BackLink
+        title={`Attach ${type}`}
+        onClick={handleGoBack}
+        linkText="Choose disk type"
+      />
+    );
 
   return (
     <>
-      {!type && (
+      {type === "choose type" && (
         <Modal
           close={close}
           className="migrate-instance-modal"
           title={modalTitle}
           onKeyDown={handleEscKey}
         >
-          {!type && (
-            <div className="choose-migration-type">
-              <FormLink
-                icon="add-logical-volume"
-                title="Attach custom volume device"
-                onClick={() => setType("custom volume")}
-              />
-              <FormLink
-                icon="mount"
-                title="Attach host path device"
-                onClick={() => setType("host path")}
-              />
-            </div>
-          )}
+          <div className="choose-migration-type">
+            <FormLink
+              icon="add-logical-volume"
+              title="Attach custom volume"
+              onClick={() => setType("custom volume")}
+            />
+            <FormLink
+              icon="mount"
+              title="Mount host path"
+              onClick={() => setType("host path")}
+            />
+          </div>
         </Modal>
       )}
 
@@ -80,6 +77,7 @@ const AttachDiskDeviceModal: FC<Props> = ({
           project={project}
           onFinish={onFinish}
           onCancel={handleGoBack}
+          onClose={close}
           title={modalTitle}
         />
       )}
@@ -89,6 +87,7 @@ const AttachDiskDeviceModal: FC<Props> = ({
           formik={formik}
           onFinish={onFinish}
           onCancel={handleGoBack}
+          onClose={close}
           title={modalTitle}
         />
       )}

diff --git a/src/pages/storage/CustomVolumeCreateModal.tsx b/src/pages/storage/CustomVolumeCreateModal.tsx
@@ -117,7 +117,7 @@ const CustomVolumeCreateModal: FC<Props> = ({
           className="u-no-margin--bottom"
           onClick={onCancel}
         >
-          Cancel
+          Back
         </Button>
         <ActionButton
           appearance="positive"

diff --git a/src/pages/storage/CustomVolumeModal.tsx b/src/pages/storage/CustomVolumeModal.tsx
@@ -12,6 +12,7 @@ interface Props {
   project: string;
   onFinish: (volume: LxdStorageVolume) => void;
   onCancel: () => void;
+  onClose: () => void;
   title?: ReactNode;
 }
 
@@ -23,6 +24,7 @@ const CustomVolumeModal: FC<Props> = ({
   project,
   onFinish,
   onCancel,
+  onClose,
   title,
 }) => {
   const [content, setContent] = useState(SELECT_VOLUME);
@@ -45,17 +47,17 @@ const CustomVolumeModal: FC<Props> = ({
   if (content === CREATE_VOLUME) {
     modalTitle = title ? (
       <BackLink
-        title={"Create volume"}
+        title="Create volume"
         onClick={handleGoBack}
-        linkText={"Attach custom volume device"}
+        linkText="Attach custom volume"
       />
     ) : (
       "Create volume"
     );
   }
 
   return (
-    <Modal className="custom-volume-modal" close={onCancel} title={modalTitle}>
+    <Modal className="custom-volume-modal" close={onClose} title={modalTitle}>
       {content === SELECT_VOLUME && (
         <CustomVolumeSelectModal
           project={project}
@@ -64,6 +66,7 @@ const CustomVolumeModal: FC<Props> = ({
           onFinish={onFinish}
           onCancel={onCancel}
           onCreate={() => setContent(CREATE_VOLUME)}
+          hasPrevStep={!!title}
         />
       )}
       {content === CREATE_VOLUME && (

diff --git a/src/pages/storage/CustomVolumeSelectBtn.tsx b/src/pages/storage/CustomVolumeSelectBtn.tsx
@@ -22,8 +22,6 @@ const CustomVolumeSelectBtn: FC<Props> = ({
 }) => {
   const { openPortal, closePortal, isOpen, Portal } = usePortal();
 
-  const handleCancel = () => closePortal();
-
   const handleFinish = (volume: LxdStorageVolume) => {
     setValue(volume);
     closePortal();
@@ -40,7 +38,8 @@ const CustomVolumeSelectBtn: FC<Props> = ({
             formik={formik}
             project={project}
             onFinish={handleFinish}
-            onCancel={handleCancel}
+            onCancel={closePortal}
+            onClose={closePortal}
           />
         </Portal>
       )}
-Original file line number
+Diff line change
@@ Expand Up @@
           }
           if (isHostDiskDevice(device)) {
-            deviceType += " (host)";
+            deviceType += " (host path)";
           }
         }
@@ Expand Down @@