Storage UI: Delete Drive (in proposal; Remove the configuration for t…

…his device) (#1934)

## Problem

https://trello.com/c/JQ3blpRm


![remove](https://github.com/user-attachments/assets/0b714e90-dacf-46f9-8ee4-e6e863a176ae)


## Solution

The implementation was easier because the UI model already had a
`removeDrive` method...
but testing was fun again, because imitating the tests that worked for
deleting a partition was not enough this time 😅


## Testing

- Added a new unit test
- Tested manually


## Screenshots

In action (used [Peek](https://github.com/phw/peek) for recording,
thanks Ancor for the tip)
1. Try to delete `nvme01` but not possible because it contains `/`
2. Delete `/` from it (#1915)... (and we have an issue box, that's new
but not from me 😄 )
3. Now can delete `nvme01`

![agama-delete-proposed-partition-and-drive](https://github.com/user-attachments/assets/81ae11b2-6fd3-4921-8d6e-d423af94cb1a)

web/package/agama-web-ui.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Thu Jan 30 12:04:23 UTC 2025 - Martin Vidner <[email protected]>
+
+- Make deletion of a proposed drive work:
+  "Do not use, Remove the configuration for this device"
+  (gh#agama-project/agama#1934)
+
 -------------------------------------------------------------------
 Fri Jan 24 09:34:24 UTC 2025 - Imobach Gonzalez Sosa <[email protected]>
 
@@ -11,11 +18,10 @@ Fri Jan 24 06:43:52 UTC 2025 - Imobach Gonzalez Sosa <[email protected]>
   the registration alert (gh#agama-project/agama#1938).
 
 -------------------------------------------------------------------
-
 Thu Jan 23 15:09:09 UTC 2025 - Martin Vidner <[email protected]>
 
 - Make the trash can icon (Delete) of a proposed partition work
-  gh#agama-project/agama#1915)
+  (gh#agama-project/agama#1915)
 
 -------------------------------------------------------------------
 Tue Jan 21 09:44:08 UTC 2025 - Imobach Gonzalez Sosa <[email protected]>

web/src/components/storage/DriveEditor.test.tsx

@@ -63,12 +63,18 @@ const mockDrive: ConfigModel.Drive = {
   ],
 };
 
+const mockDeleteDrive = jest.fn();
 const mockDeletePartition = jest.fn();
-// TODO: why does "~/queries/storage" work elsewhere??
+
+jest.mock("~/queries/storage", () => ({
+  ...jest.requireActual("~/queries/storage"),
+  useAvailableDevices: () => [sda],
+}));
+
 jest.mock("~/queries/storage/config-model", () => ({
   ...jest.requireActual("~/queries/storage/config-model"),
   useConfigModel: () => ({ drives: [mockDrive] }),
-  useDrive: () => mockDrive,
+  useDrive: () => ({ delete: mockDeleteDrive }),
   usePartition: () => ({ delete: mockDeletePartition }),
 }));
 
@@ -78,7 +84,7 @@ const props: DriveEditorProps = {
 };
 
 describe("PartitionMenuItem", () => {
-  it("allows users delete a the partition", async () => {
+  it("allows users to delete the partition", async () => {
     const { user } = plainRender(<DriveEditor {...props} />);
 
     const partitionsButton = screen.getByRole("button", { name: "Partitions" });
@@ -91,3 +97,18 @@ describe("PartitionMenuItem", () => {
     expect(mockDeletePartition).toHaveBeenCalled();
   });
 });
+
+describe("RemoveDriveOption", () => {
+  it("allows users to delete the drive", async () => {
+    const { user } = plainRender(<DriveEditor {...props} />);
+
+    const driveButton = screen.getByRole("button", { name: "Drive" });
+    await user.click(driveButton);
+    const drivesMenu = screen.getByRole("menu");
+    const deleteDriveButton = within(drivesMenu).getByRole("menuitem", {
+      name: /Do not use/,
+    });
+    await user.click(deleteDriveButton);
+    expect(mockDeleteDrive).toHaveBeenCalled();
+  });
+});

web/src/components/storage/DriveEditor.tsx

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2024] SUSE LLC
+ * Copyright (c) [2024-2025] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -171,7 +171,10 @@ const SpacePolicySelector = ({ drive, driveDevice }: DriveEditorProps) => {
 };
 
 const SearchSelectorIntro = ({ drive }: { drive: configModel.Drive }) => {
-  const { isBoot, isExplicitBoot } = useDrive(drive.name);
+  const driveModel = useDrive(drive.name);
+  if (!driveModel) return;
+
+  const { isBoot, isExplicitBoot } = driveModel;
   // TODO: Get volume groups associated to the drive.
   const volumeGroups = [];
 
@@ -357,7 +360,10 @@ const SearchSelectorSingleOption = ({ selected }) => {
 };
 
 const SearchSelectorOptions = ({ drive, selected, onChange }) => {
-  const { isExplicitBoot } = useDrive(drive.name);
+  const driveModel = useDrive(drive.name);
+  if (!driveModel) return;
+
+  const { isExplicitBoot } = driveModel;
   // const boot = isExplicitBoot(drive.name);
 
   if (driveUtils.hasReuse(drive)) return <SearchSelectorSingleOption selected={selected} />;
@@ -384,23 +390,32 @@ const SearchSelector = ({ drive, selected, onChange }) => {
 };
 
 const RemoveDriveOption = ({ drive }) => {
-  const { isExplicitBoot } = useDrive(drive.name);
+  const driveModel = useDrive(drive.name);
+
+  if (!driveModel) return;
+
+  const { isExplicitBoot, delete: deleteDrive } = driveModel;
 
-  if (driveUtils.hasPv(drive)) return;
   if (isExplicitBoot) return;
+  if (driveUtils.hasPv(drive)) return;
   if (driveUtils.hasRoot(drive)) return;
 
   return (
     <>
       <Divider component="hr" />
-      <MenuItem description={_("Remove the configuration for this device")}>
+      <MenuItem
+        isDanger
+        description={_("Remove the configuration for this device")}
+        onClick={deleteDrive}
+      >
         {_("Do not use")}
       </MenuItem>
     </>
   );
 };
 
-const DriveSelector = ({ drive, selected }) => {
+const DriveSelector = ({ drive, selected, toggleAriaLabel }) => {
+  const menuId = useId();
   const menuRef = useRef();
   const toggleMenuRef = useRef();
   const [isOpen, setIsOpen] = useState(false);
@@ -417,8 +432,14 @@ const DriveSelector = ({ drive, selected }) => {
       onOpenChange={setIsOpen}
       toggleRef={toggleMenuRef}
       toggle={
-        <InlineMenuToggle ref={toggleMenuRef} onClick={onToggle} isExpanded={isOpen}>
-          <b>{deviceLabel(selected)}</b>
+        <InlineMenuToggle
+          ref={toggleMenuRef}
+          onClick={onToggle}
+          isExpanded={isOpen}
+          aria-label={toggleAriaLabel}
+          aria-controls={menuId}
+        >
+          <b aria-hidden>{deviceLabel(selected)}</b>
         </InlineMenuToggle>
       }
       menuRef={menuRef}
@@ -496,11 +517,13 @@ const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => {
   };
 
   const [txt1, txt2] = text(drive).split("%s");
+  // TRANSLATORS: a disk drive
+  const toggleAriaLabel = _("Drive");
 
   return (
     <h4>
       <span>{txt1}</span>
-      <DriveSelector drive={drive} selected={driveDevice} />
+      <DriveSelector drive={drive} selected={driveDevice} toggleAriaLabel={toggleAriaLabel} />
       <span>{txt2}</span>
     </h4>
   );

web/src/queries/storage/config-model.ts

@@ -51,8 +51,9 @@ function findDrive(model: configModel.Config, driveName: string): configModel.Dr
   return drives.find((d) => d.name === driveName);
 }
 
-function removeDrive(model: configModel.Config, driveName: string) {
+function removeDrive(model: configModel.Config, driveName: string): configModel.Config {
   model.drives = model.drives.filter((d) => d.name !== driveName);
+  return model;
 }
 
 function isUsedDrive(model: configModel.Config, driveName: string) {
@@ -302,6 +303,7 @@ export type DriveHook = {
   isExplicitBoot: boolean;
   switch: (newName: string) => void;
   setSpacePolicy: (policy: configModel.SpacePolicy, actions?: SpacePolicyAction[]) => void;
+  delete: () => void;
 };
 
 export function useDrive(name: string): DriveHook | undefined {
@@ -317,5 +319,6 @@ export function useDrive(name: string): DriveHook | undefined {
     setSpacePolicy: (policy: configModel.SpacePolicy, actions?: SpacePolicyAction[]) => {
       mutate(setSpacePolicy(model, name, policy, actions));
     },
+    delete: () => mutate(removeDrive(model, name)),
   };
 }