From e431192b536ac90dca10bc4a0b7068d74798b3a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Wed, 21 Feb 2024 08:16:01 +0000
Subject: [PATCH 01/11] [web] Move device selection to its own section

And take the opportunity to avoid triggering the onChange callback when
the user actually does not change the selected device but clicks the
"Accept" button of the selection dialog.
---
 .../storage/ProposalDeviceSection.jsx         | 208 ++++++++++++++++++
 .../storage/ProposalDeviceSection.test.jsx    | 182 +++++++++++++++
 web/src/components/storage/ProposalPage.jsx   |   7 +
 .../storage/ProposalSettingsSection.jsx       | 148 -------------
 .../storage/ProposalSettingsSection.test.jsx  | 110 ---------
 web/src/components/storage/index.js           |   1 +
 6 files changed, 398 insertions(+), 258 deletions(-)
 create mode 100644 web/src/components/storage/ProposalDeviceSection.jsx
 create mode 100644 web/src/components/storage/ProposalDeviceSection.test.jsx

diff --git a/web/src/components/storage/ProposalDeviceSection.jsx b/web/src/components/storage/ProposalDeviceSection.jsx
new file mode 100644
index 0000000000..8c2f3dc61c
--- /dev/null
+++ b/web/src/components/storage/ProposalDeviceSection.jsx
@@ -0,0 +1,208 @@
+/*
+ * Copyright (c) [2024] SUSE LLC
+ *
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, contact SUSE LLC.
+ *
+ * To contact SUSE LLC about this file by physical or electronic mail, you may
+ * find current contact information at www.suse.com.
+ */
+
+import React, { useState } from "react";
+import { Button, Form, Skeleton } from "@patternfly/react-core";
+
+import { _ } from "~/i18n";
+import { If, Section, Popup } from "~/components/core";
+import { DeviceSelector } from "~/components/storage";
+import { deviceLabel } from '~/components/storage/utils';
+import { noop } from "~/utils";
+
+/**
+ * @typedef {import ("~/client/storage").ProposalManager.ProposalSettings} ProposalSettings
+ * @typedef {import ("~/client/storage").DevicesManager.StorageDevice} StorageDevice
+ */
+
+/**
+ * Form for selecting the installation device.
+ * @component
+ *
+ * @param {object} props
+ * @param {string} props.id - Form ID.
+ * @param {StorageDevice|undefined} [props.current] - Currently selected device, if any.
+ * @param {StorageDevice[]} [props.devices=[]] - Available devices for the selection.
+ * @param {onSubmitFn} [props.onSubmit=noop] - On submit callback.
+ *
+ * @callback onSubmitFn
+ * @param {string} device - Name of the selected device.
+ */
+const InstallationDeviceForm = ({
+  id,
+  current,
+  devices = [],
+  onSubmit = noop
+}) => {
+  const [device, setDevice] = useState(current || devices[0]);
+
+  const changeSelected = (deviceId) => {
+    setDevice(devices.find(d => d.sid === deviceId));
+  };
+
+  const submitForm = (e) => {
+    e.preventDefault();
+    if (device !== undefined) onSubmit(device);
+  };
+
+  return (
+    <Form id={id} onSubmit={submitForm}>
+      <DeviceSelector
+        selected={device}
+        devices={devices}
+        onChange={changeSelected}
+      />
+    </Form>
+  );
+};
+
+/**
+ * Allows to select the installation device.
+ * @component
+ *
+ * @callback onChangeFn
+ * @param {string} device - Name of the selected device.
+ *
+ * @param {object} props
+ * @param {string} [props.current] - Device name, if any.
+ * @param {StorageDevice[]} [props.devices=[]] - Available devices for the selection.
+ * @param {boolean} [props.isLoading=false] - Whether to show the selector as loading.
+ * @param {onChangeFn} [props.onChange=noop] - On change callback.
+ */
+const InstallationDeviceField = ({
+  current,
+  devices = [],
+  isLoading = false,
+  onChange = noop
+}) => {
+  const [device, setDevice] = useState(devices.find(d => d.name === current));
+  const [isFormOpen, setIsFormOpen] = useState(false);
+
+  const openForm = () => setIsFormOpen(true);
+
+  const closeForm = () => setIsFormOpen(false);
+
+  const acceptForm = (selectedDevice) => {
+    closeForm();
+    setDevice(selectedDevice);
+    onChange(selectedDevice);
+  };
+
+  /**
+   * Renders a button that allows changing selected device
+   *
+   * NOTE: if a device is already selected, its name and size will be used for
+   * the button text. Otherwise, a "No device selected" text will be shown.
+   *
+   * @param {object} props
+   * @param {StorageDevice|undefined} [props.current] - Currently selected device, if any.
+   */
+  const DeviceContent = ({ device }) => {
+    return (
+      <Button variant="link" isInline onClick={openForm}>
+        {device ? deviceLabel(device) : _("No device selected yet")}
+      </Button>
+    );
+  };
+
+  if (isLoading) {
+    return <Skeleton screenreaderText={_("Loading selected device")} width="25%" />;
+  }
+
+  const description = _("Select the device for installing the system.");
+
+  return (
+    <>
+      <div className="split">
+        <span>{_("Installation device")}</span>
+        <DeviceContent device={device} />
+      </div>
+      <Popup
+        title={_("Installation device")}
+        description={description}
+        isOpen={isFormOpen}
+      >
+        <If
+          condition={devices.length === 0}
+          then={_("No devices found.")}
+          else={
+            <InstallationDeviceForm
+              id="bootDeviceForm"
+              current={device}
+              devices={devices}
+              onSubmit={acceptForm}
+            />
+          }
+        />
+        <Popup.Actions>
+          <Popup.Confirm
+            form="bootDeviceForm"
+            type="submit"
+            isDisabled={devices.length === 0}
+          >
+            {_("Accept")}
+          </Popup.Confirm>
+          <Popup.Cancel onClick={closeForm} />
+        </Popup.Actions>
+      </Popup>
+    </>
+  );
+};
+
+/**
+ * Section for editing the selected device
+ * @component
+ *
+ * @callback onChangeFn
+ * @param {object} settings
+ *
+ * @param {object} props
+ * @param {ProposalSettings} props.settings
+ * @param {StorageDevice[]} [props.availableDevices=[]]
+ * @param {boolean} [isLoading=false]
+ * @param {onChangeFn} [props.onChange=noop]
+ */
+export default function ProposalDeviceSection({
+  settings,
+  availableDevices = [],
+  isLoading = false,
+  onChange = noop
+}) {
+  // FIXME: we should work with devices objects ASAP
+  const { bootDevice } = settings;
+
+  const changeBootDevice = (device) => {
+    if (device.name !== bootDevice) {
+      onChange({ bootDevice: device.name });
+    }
+  };
+
+  return (
+    <Section title={_("Device")}>
+      <InstallationDeviceField
+        current={bootDevice}
+        devices={availableDevices}
+        isLoading={isLoading && bootDevice === undefined}
+        onChange={changeBootDevice}
+      />
+    </Section>
+  );
+}
diff --git a/web/src/components/storage/ProposalDeviceSection.test.jsx b/web/src/components/storage/ProposalDeviceSection.test.jsx
new file mode 100644
index 0000000000..e90db6a054
--- /dev/null
+++ b/web/src/components/storage/ProposalDeviceSection.test.jsx
@@ -0,0 +1,182 @@
+/*
+ * Copyright (c) [2024] SUSE LLC
+ *
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, contact SUSE LLC.
+ *
+ * To contact SUSE LLC about this file by physical or electronic mail, you may
+ * find current contact information at www.suse.com.
+ */
+
+import React from "react";
+import { screen, within } from "@testing-library/react";
+import { plainRender } from "~/test-utils";
+import { ProposalDeviceSection } from "~/components/storage";
+
+const sda = {
+  sid: "59",
+  isDrive: true,
+  type: "disk",
+  vendor: "Micron",
+  model: "Micron 1100 SATA",
+  driver: ["ahci", "mmcblk"],
+  bus: "IDE",
+  busId: "",
+  transport: "usb",
+  dellBOSS: false,
+  sdCard: true,
+  active: true,
+  name: "/dev/sda",
+  size: 1024,
+  recoverableSize: 0,
+  systems : [],
+  udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"],
+  udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"],
+};
+
+const sdb = {
+  sid: "62",
+  isDrive: true,
+  type: "disk",
+  vendor: "Samsung",
+  model: "Samsung Evo 8 Pro",
+  driver: ["ahci"],
+  bus: "IDE",
+  busId: "",
+  transport: "",
+  dellBOSS: false,
+  sdCard: false,
+  active: true,
+  name: "/dev/sdb",
+  size: 2048,
+  recoverableSize: 0,
+  systems : [],
+  udevIds: [],
+  udevPaths: ["pci-0000:00-19"]
+};
+
+const props = {
+  settings: {
+    bootDevice: "/dev/sda",
+  },
+  availableDevices: [sda, sdb],
+  isLoading: false,
+  onChange: jest.fn()
+};
+
+describe("ProposalDeviceSection", () => {
+  describe("when set as loading", () => {
+    beforeEach(() => {
+      props.isLoading = true;
+    });
+
+    describe("and selected device is not defined yet", () => {
+      beforeEach(() => {
+        props.settings = { bootDevice: undefined };
+      });
+
+      it("renders a loading hint", () => {
+        plainRender(<ProposalDeviceSection {...props} />);
+        screen.getByText("Loading selected device");
+      });
+    });
+  });
+
+  describe("when installation device is not selected yet", () => {
+    beforeEach(() => {
+      props.settings = { bootDevice: "" };
+    });
+
+    it("uses a 'No device selected yet' text for the selection button", async () => {
+      const { user } = plainRender(<ProposalDeviceSection {...props} />);
+      const button = screen.getByRole("button", { name: "No device selected yet" });
+
+      await user.click(button);
+
+      screen.getByRole("dialog", { name: "Installation device" });
+    });
+  });
+
+  describe("when installation device is selected", () => {
+    beforeEach(() => {
+      props.settings = { bootDevice: "/dev/sda" };
+    });
+
+    it("uses its name as part of the text for the selection button", async () => {
+      const { user } = plainRender(<ProposalDeviceSection {...props} />);
+      const button = screen.getByRole("button", { name: /\/dev\/sda/ });
+
+      await user.click(button);
+
+      screen.getByRole("dialog", { name: "Installation device" });
+    });
+  });
+
+  it("allows changing the selected device", async () => {
+    const { user } = plainRender(<ProposalDeviceSection {...props} />);
+    const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
+
+    await user.click(button);
+
+    const selector = await screen.findByRole("dialog", { name: "Installation device" });
+    const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
+    const accept = within(selector).getByRole("button", { name: "Accept" });
+
+    await user.click(sdbOption);
+    await user.click(accept);
+
+    expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
+    expect(props.onChange).toHaveBeenCalledWith({ bootDevice: sdb.name });
+  });
+
+  it("allows canceling a device selection", async () => {
+    const { user } = plainRender(<ProposalDeviceSection {...props} />);
+    const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
+
+    await user.click(button);
+
+    const selector = await screen.findByRole("dialog", { name: "Installation device" });
+    const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
+    const cancel = within(selector).getByRole("button", { name: "Cancel" });
+
+    await user.click(sdbOption);
+    await user.click(cancel);
+
+    expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
+    expect(props.onChange).not.toHaveBeenCalled();
+  });
+
+  it("does not trigger the onChange callback when selection actually did not change", async () => {
+    const { user } = plainRender(<ProposalDeviceSection {...props} />);
+    const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
+
+    await user.click(button);
+
+    const selector = await screen.findByRole("dialog", { name: "Installation device" });
+    const sdaOption = within(selector).getByRole("radio", { name: /sda/ });
+    const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
+    const accept = within(selector).getByRole("button", { name: "Accept" });
+
+    // User selects a different device
+    await user.click(sdbOption);
+    // but then goes back to the selected device
+    await user.click(sdaOption);
+    // and clicks on Accept button
+    await user.click(accept);
+
+    expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
+    // There is no reason for triggering the onChange callback
+    expect(props.onChange).not.toHaveBeenCalled();
+  });
+});
diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx
index 4d730cce93..faeb34f240 100644
--- a/web/src/components/storage/ProposalPage.jsx
+++ b/web/src/components/storage/ProposalPage.jsx
@@ -32,6 +32,7 @@ import {
   ProposalPageMenu,
   ProposalSettingsSection,
   ProposalSpacePolicySection,
+  ProposalDeviceSection,
 } from "~/components/storage";
 import { IDLE } from "~/client/status";
 
@@ -215,6 +216,12 @@ export default function ProposalPage() {
           customIcon={<Icon name="info" size="xxs" />}
           title={_("Devices will not be modified until installation starts.")}
         />
+        <ProposalDeviceSection
+          settings={state.settings}
+          availableDevices={state.availableDevices}
+          isLoading={state.loading}
+          onChange={changeSettings}
+        />
         <ProposalSettingsSection
           availableDevices={state.availableDevices}
           volumeTemplates={usefulTemplates()}
diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx
index b84f5cb9b2..74b8dfdabf 100644
--- a/web/src/components/storage/ProposalSettingsSection.jsx
+++ b/web/src/components/storage/ProposalSettingsSection.jsx
@@ -21,7 +21,6 @@
 
 import React, { useEffect, useState } from "react";
 import {
-  Button,
   Form, Skeleton, Switch, Checkbox,
   ToggleGroup, ToggleGroupItem,
   Tooltip
@@ -30,7 +29,6 @@ import {
 import { _ } from "~/i18n";
 import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core";
 import { DeviceList, DeviceSelector, ProposalVolumes } from "~/components/storage";
-import { deviceLabel } from '~/components/storage/utils';
 import { Icon } from "~/components/layout";
 import { noop } from "~/utils";
 
@@ -40,140 +38,6 @@ import { noop } from "~/utils";
  * @typedef {import ("~/client/storage").ProposalManager.Volume} Volume
  */
 
-/**
- * Form for selecting the installation device.
- * @component
- *
- * @param {object} props
- * @param {string} props.id - Form ID.
- * @param {StorageDevice|undefined} [props.current] - Currently selected device, if any.
- * @param {StorageDevice[]} [props.devices=[]] - Available devices for the selection.
- * @param {onSubmitFn} [props.onSubmit=noop] - On submit callback.
- *
- * @callback onSubmitFn
- * @param {string} device - Name of the selected device.
- */
-const InstallationDeviceForm = ({
-  id,
-  current,
-  devices = [],
-  onSubmit = noop
-}) => {
-  const [device, setDevice] = useState(current || devices[0]);
-
-  const changeSelected = (deviceId) => {
-    setDevice(devices.find(d => d.sid === deviceId));
-  };
-
-  const submitForm = (e) => {
-    e.preventDefault();
-    if (device !== undefined) onSubmit(device);
-  };
-
-  return (
-    <Form id={id} onSubmit={submitForm}>
-      <DeviceSelector
-        selected={device}
-        devices={devices}
-        onChange={changeSelected}
-      />
-    </Form>
-  );
-};
-
-/**
- * Allows to select the installation device.
- * @component
- *
- * @param {object} props
- * @param {string} [props.current] - Device name, if any.
- * @param {StorageDevice[]} [props.devices=[]] - Available devices for the selection.
- * @param {boolean} [props.isLoading=false] - Whether to show the selector as loading.
- * @param {onChangeFn} [props.onChange=noop] - On change callback.
- *
- * @callback onChangeFn
- * @param {string} device - Name of the selected device.
- */
-const InstallationDeviceField = ({
-  current,
-  devices = [],
-  isLoading = false,
-  onChange = noop
-}) => {
-  const [device, setDevice] = useState(devices.find(d => d.name === current));
-  const [isFormOpen, setIsFormOpen] = useState(false);
-
-  const openForm = () => setIsFormOpen(true);
-
-  const closeForm = () => setIsFormOpen(false);
-
-  const acceptForm = (selectedDevice) => {
-    closeForm();
-    setDevice(selectedDevice);
-    onChange(selectedDevice);
-  };
-
-  /**
-   * Renders a button that allows changing selected device
-   *
-   * NOTE: if a device is already selected, its name and size will be used for
-   * the button text. Otherwise, a "No device selected" text will be shown.
-   *
-   * @param {object} props
-   * @param {StorageDevice|undefined} [props.current] - Currently selected device, if any.
-   */
-  const DeviceContent = ({ device }) => {
-    return (
-      <Button variant="link" isInline onClick={openForm}>
-        {device ? deviceLabel(device) : _("No device selected yet")}
-      </Button>
-    );
-  };
-
-  if (isLoading) {
-    return <Skeleton width="25%" />;
-  }
-
-  const description = _("Select the device for installing the system.");
-
-  return (
-    <>
-      <div className="split">
-        <span>{_("Installation device")}</span>
-        <DeviceContent device={device} />
-      </div>
-      <Popup
-        title={_("Installation device")}
-        description={description}
-        isOpen={isFormOpen}
-      >
-        <If
-          condition={devices.length === 0}
-          then={_("No devices found.")}
-          else={
-            <InstallationDeviceForm
-              id="bootDeviceForm"
-              current={device}
-              devices={devices}
-              onSubmit={acceptForm}
-            />
-          }
-        />
-        <Popup.Actions>
-          <Popup.Confirm
-            form="bootDeviceForm"
-            type="submit"
-            isDisabled={devices.length === 0}
-          >
-            {_("Accept")}
-          </Popup.Confirm>
-          <Popup.Cancel onClick={closeForm} />
-        </Popup.Actions>
-      </Popup>
-    </>
-  );
-};
-
 /**
  * Form for configuring the system volume group.
  * @component
@@ -575,11 +439,6 @@ export default function ProposalSettingsSection({
   isLoading = false,
   onChange = noop
 }) {
-  // FIXME: we should work with devices objects ASAP
-  const changeBootDevice = (device) => {
-    onChange({ bootDevice: device.name });
-  };
-
   const changeLVM = ({ lvm, vgDevices }) => {
     const settings = {};
     if (lvm !== undefined) settings.lvm = lvm;
@@ -596,18 +455,11 @@ export default function ProposalSettingsSection({
     onChange({ volumes });
   };
 
-  const { bootDevice } = settings;
   const encryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0;
 
   return (
     <>
       <Section title={_("Settings")} className="flex-stack">
-        <InstallationDeviceField
-          current={bootDevice}
-          devices={availableDevices}
-          isLoading={isLoading && bootDevice === undefined}
-          onChange={changeBootDevice}
-        />
         <LVMField
           settings={settings}
           devices={availableDevices}
diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx
index a3687f6047..7d0a575de8 100644
--- a/web/src/components/storage/ProposalSettingsSection.test.jsx
+++ b/web/src/components/storage/ProposalSettingsSection.test.jsx
@@ -86,116 +86,6 @@ beforeEach(() => {
   props = {};
 });
 
-describe("Installation device field", () => {
-  describe("if it is loading", () => {
-    beforeEach(() => {
-      props.isLoading = true;
-    });
-
-    describe("and there is no selected device yet", () => {
-      beforeEach(() => {
-        props.settings = { bootDevice: "" };
-      });
-
-      it("renders a message indicating that the device is not selected", () => {
-        plainRender(<ProposalSettingsSection {...props} />);
-
-        screen.getByText(/Installation device/);
-        screen.getByText(/No device selected/);
-      });
-    });
-
-    // FIXME: skipping this test example by now.
-    // It will be addressed when reworking the Device section
-    // as part of https://github.com/openSUSE/agama/pull/1031
-    describe.skip("and there is a selected device", () => {
-      beforeEach(() => {
-        props.settings = { bootDevice: "/dev/vda" };
-      });
-
-      it("renders the selected device", () => {
-        plainRender(<ProposalSettingsSection {...props} />);
-
-        screen.getByText(/Installation device/);
-        screen.getByText("/dev/vda");
-      });
-    });
-  });
-
-  describe("if there is no selected device yet", () => {
-    beforeEach(() => {
-      props.settings = { bootDevice: "" };
-    });
-
-    it("renders a message indicating that the device is not selected", () => {
-      plainRender(<ProposalSettingsSection {...props} />);
-
-      screen.getByText(/Installation device/);
-      screen.getByText(/No device selected/);
-    });
-  });
-
-  // FIXME: skipping this test example by now.
-  // It will be addressed when reworking the Device section
-  // as part of https://github.com/openSUSE/agama/pull/1031
-  describe.skip("if there is a selected device", () => {
-    beforeEach(() => {
-      props.settings = { bootDevice: "/dev/vda" };
-    });
-
-    it("renders the selected device", () => {
-      plainRender(<ProposalSettingsSection {...props} />);
-
-      screen.getByText(/Installation device/);
-      screen.getByText("/dev/vda");
-    });
-  });
-
-  it("allows selecting a device when clicking on the device name", async () => {
-    props = {
-      availableDevices: [vda],
-      settings: { bootDevice: "/dev/vda" },
-      onChange: jest.fn()
-    };
-
-    const { user } = plainRender(<ProposalSettingsSection {...props} />);
-
-    const button = screen.getByRole("button", { name: "/dev/vda, 1 KiB" });
-    await user.click(button);
-
-    const popup = await screen.findByRole("dialog");
-    within(popup).getByText("Installation device");
-
-    const accept = within(popup).getByRole("button", { name: "Accept" });
-    await user.click(accept);
-
-    expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
-    expect(props.onChange).toHaveBeenCalled();
-  });
-
-  it("allows canceling the selection of the device", async () => {
-    props = {
-      availableDevices: [vda],
-      settings: { bootDevice: "/dev/vda" },
-      onChange: jest.fn()
-    };
-
-    const { user } = plainRender(<ProposalSettingsSection {...props} />);
-
-    const button = screen.getByRole("button", { name: "/dev/vda, 1 KiB" });
-    await user.click(button);
-
-    const popup = await screen.findByRole("dialog");
-    within(popup).getByText("Installation device");
-
-    const cancel = within(popup).getByRole("button", { name: "Cancel" });
-    await user.click(cancel);
-
-    expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
-    expect(props.onChange).not.toHaveBeenCalled();
-  });
-});
-
 describe("LVM field", () => {
   describe("if LVM setting is not set yet", () => {
     beforeEach(() => {
diff --git a/web/src/components/storage/index.js b/web/src/components/storage/index.js
index 14717598a8..5cccae8494 100644
--- a/web/src/components/storage/index.js
+++ b/web/src/components/storage/index.js
@@ -23,6 +23,7 @@ export { default as ProposalPage } from "./ProposalPage";
 export { default as ProposalPageMenu } from "./ProposalPageMenu";
 export { default as ProposalSettingsSection } from "./ProposalSettingsSection";
 export { default as ProposalSpacePolicySection } from "./ProposalSpacePolicySection";
+export { default as ProposalDeviceSection } from "./ProposalDeviceSection";
 export { default as ProposalActionsSection } from "./ProposalActionsSection";
 export { default as ProposalVolumes } from "./ProposalVolumes";
 export { default as DASDPage } from "./DASDPage";

From f39c9af6cff8a72a8397be06ff869a1d6a5ba53c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Wed, 21 Feb 2024 12:06:31 +0000
Subject: [PATCH 02/11] [web] Create a section for file systems

And mount there the storage/ProposalVolumes component instead of doing
so in the settings section.

Note that, most probably, the code of ProposalFileSystemsSection and
ProposalVolumes could be merged in a single component.
---
 .../storage/ProposalFileSystemsSection.jsx    | 80 +++++++++++++++++++
 .../ProposalFileSystemsSection.test.jsx       | 55 +++++++++++++
 web/src/components/storage/ProposalPage.jsx   | 17 ++--
 .../storage/ProposalSettingsSection.jsx       | 17 +---
 web/src/components/storage/index.js           |  1 +
 5 files changed, 144 insertions(+), 26 deletions(-)
 create mode 100644 web/src/components/storage/ProposalFileSystemsSection.jsx
 create mode 100644 web/src/components/storage/ProposalFileSystemsSection.test.jsx

diff --git a/web/src/components/storage/ProposalFileSystemsSection.jsx b/web/src/components/storage/ProposalFileSystemsSection.jsx
new file mode 100644
index 0000000000..0b0e5f632d
--- /dev/null
+++ b/web/src/components/storage/ProposalFileSystemsSection.jsx
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) [2024] SUSE LLC
+ *
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, contact SUSE LLC.
+ *
+ * To contact SUSE LLC about this file by physical or electronic mail, you may
+ * find current contact information at www.suse.com.
+ */
+
+import React from "react";
+import { _ } from "~/i18n";
+import { Section } from "~/components/core";
+import { ProposalVolumes } from "~/components/storage";
+import { noop } from "~/utils";
+
+/**
+ * @typedef {import ("~/client/storage").ProposalManager.ProposalSettings} ProposalSettings
+ * @typedef {import ("~/client/storage").ProposalManager.Volume} Volume
+ */
+
+/**
+ * Section for editing the proposal file systems
+ * @component
+ *
+ * @callback onChangeFn
+ * @param {object} settings
+ *
+ * @param {object} props
+ * @param {ProposalSettings} props.settings
+ * @param {Volume[]} [props.volumeTemplates=[]]
+ * @param {boolean} [props.isLoading=false]
+ * @param {onChangeFn} [props.onChange=noop]
+ *
+ */
+export default function ProposalFileSystemsSection({
+  settings,
+  volumeTemplates = [],
+  isLoading = false,
+  onChange = noop
+}) {
+  const { volumes = [] } = settings;
+
+  const changeVolumes = (volumes) => {
+    onChange({ volumes });
+  };
+
+  // Templates for already existing mount points are filtered out
+  const usefulTemplates = () => {
+    const mountPaths = volumes.map(v => v.mountPath);
+    return volumeTemplates.filter(t => (
+      t.mountPath.length > 0 && !mountPaths.includes(t.mountPath)
+    ));
+  };
+
+  const encryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0;
+
+  return (
+    <Section title={_("File systems")}>
+      <ProposalVolumes
+        volumes={volumes}
+        templates={usefulTemplates()}
+        options={{ lvm: settings.lvm, encryption }}
+        isLoading={isLoading && settings.volumes === undefined}
+        onChange={changeVolumes}
+      />
+    </Section>
+  );
+}
diff --git a/web/src/components/storage/ProposalFileSystemsSection.test.jsx b/web/src/components/storage/ProposalFileSystemsSection.test.jsx
new file mode 100644
index 0000000000..0b1493a5ff
--- /dev/null
+++ b/web/src/components/storage/ProposalFileSystemsSection.test.jsx
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) [2024] SUSE LLC
+ *
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, contact SUSE LLC.
+ *
+ * To contact SUSE LLC about this file by physical or electronic mail, you may
+ * find current contact information at www.suse.com.
+ */
+
+import React from "react";
+import { screen, within } from "@testing-library/react";
+import { plainRender } from "~/test-utils";
+import { ProposalFileSystemsSection } from "~/components/storage";
+
+const props = {
+  settings: {},
+  isLoading: false,
+  onChange: jest.fn()
+};
+
+describe("ProposalFileSystemsSection", () => {
+  it("renders a section holding file systems related stuff", () => {
+    plainRender(<ProposalFileSystemsSection {...props} />);
+    screen.getByRole("region", { name: "File systems" });
+    screen.getByRole("grid", { name: /mount points/ });
+  });
+
+  it("requests a volume change when onChange callback is triggered", async () => {
+    const { user } = plainRender(<ProposalFileSystemsSection {...props} />);
+    const button = screen.getByRole("button", { name: "Actions" });
+
+    await user.click(button);
+
+    const menu = screen.getByRole("menu");
+    const reset = within(menu).getByRole("menuitem", { name: /Reset/ });
+
+    await user.click(reset);
+
+    expect(props.onChange).toHaveBeenCalledWith(
+      { volumes: expect.any(Array) }
+    );
+  });
+});
diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx
index faeb34f240..38a5193af7 100644
--- a/web/src/components/storage/ProposalPage.jsx
+++ b/web/src/components/storage/ProposalPage.jsx
@@ -33,6 +33,7 @@ import {
   ProposalSettingsSection,
   ProposalSpacePolicySection,
   ProposalDeviceSection,
+  ProposalFileSystemsSection
 } from "~/components/storage";
 import { IDLE } from "~/client/status";
 
@@ -200,15 +201,6 @@ export default function ProposalPage() {
   };
 
   const PageContent = () => {
-    // Templates for already existing mount points are filtered out
-    const usefulTemplates = () => {
-      const volumes = state.settings.volumes || [];
-      const mountPaths = volumes.map(v => v.mountPath);
-      return state.volumeTemplates.filter(t => (
-        t.mountPath.length > 0 && !mountPaths.includes(t.mountPath)
-      ));
-    };
-
     return (
       <>
         <Alert
@@ -224,12 +216,17 @@ export default function ProposalPage() {
         />
         <ProposalSettingsSection
           availableDevices={state.availableDevices}
-          volumeTemplates={usefulTemplates()}
           encryptionMethods={state.encryptionMethods}
           settings={state.settings}
           onChange={changeSettings}
           isLoading={state.loading}
         />
+        <ProposalFileSystemsSection
+          settings={state.settings}
+          volumeTemplates={state.volumeTemplates}
+          onChange={changeSettings}
+          isLoading={state.loading}
+        />
         <ProposalSpacePolicySection
           settings={state.settings}
           onChange={changeSettings}
diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx
index 74b8dfdabf..7084e3a09b 100644
--- a/web/src/components/storage/ProposalSettingsSection.jsx
+++ b/web/src/components/storage/ProposalSettingsSection.jsx
@@ -28,7 +28,7 @@ import {
 
 import { _ } from "~/i18n";
 import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core";
-import { DeviceList, DeviceSelector, ProposalVolumes } from "~/components/storage";
+import { DeviceList, DeviceSelector } from "~/components/storage";
 import { Icon } from "~/components/layout";
 import { noop } from "~/utils";
 
@@ -423,9 +423,7 @@ const EncryptionField = ({
  * @param {object} props
  * @param {ProposalSettings} props.settings
  * @param {StorageDevice[]} [props.availableDevices=[]]
- * @param {Volume[]} [props.volumeTemplates=[]]
  * @param {String[]} [props.encryptionMethods=[]]
- * @param {boolean} [isLoading=false]
  * @param {onChangeFn} [props.onChange=noop]
  *
  * @callback onChangeFn
@@ -434,9 +432,7 @@ const EncryptionField = ({
 export default function ProposalSettingsSection({
   settings,
   availableDevices = [],
-  volumeTemplates = [],
   encryptionMethods = [],
-  isLoading = false,
   onChange = noop
 }) {
   const changeLVM = ({ lvm, vgDevices }) => {
@@ -451,10 +447,6 @@ export default function ProposalSettingsSection({
     onChange({ encryptionPassword: password, encryptionMethod: method });
   };
 
-  const changeVolumes = (volumes) => {
-    onChange({ volumes });
-  };
-
   const encryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0;
 
   return (
@@ -475,13 +467,6 @@ export default function ProposalSettingsSection({
           isLoading={settings.encryptionPassword === undefined}
           onChange={changeEncryption}
         />
-        <ProposalVolumes
-          volumes={settings.volumes || []}
-          templates={volumeTemplates}
-          options={{ lvm: settings.lvm, encryption }}
-          isLoading={isLoading && settings.volumes === undefined}
-          onChange={changeVolumes}
-        />
       </Section>
     </>
   );
diff --git a/web/src/components/storage/index.js b/web/src/components/storage/index.js
index 5cccae8494..6c518c51a0 100644
--- a/web/src/components/storage/index.js
+++ b/web/src/components/storage/index.js
@@ -24,6 +24,7 @@ export { default as ProposalPageMenu } from "./ProposalPageMenu";
 export { default as ProposalSettingsSection } from "./ProposalSettingsSection";
 export { default as ProposalSpacePolicySection } from "./ProposalSpacePolicySection";
 export { default as ProposalDeviceSection } from "./ProposalDeviceSection";
+export { default as ProposalFileSystemsSection } from "./ProposalFileSystemsSection";
 export { default as ProposalActionsSection } from "./ProposalActionsSection";
 export { default as ProposalVolumes } from "./ProposalVolumes";
 export { default as DASDPage } from "./DASDPage";

From e708f47070e31e2e4711c2ffbfbbbc7372be41bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Wed, 21 Feb 2024 17:18:14 +0000
Subject: [PATCH 03/11] [web] Allow sections to have a description

Intended for adding, when needed, a short explanation about its usefulness.
---
 web/src/assets/styles/blocks.scss        | 27 +++++++++++++++++-------
 web/src/assets/styles/layout.scss        |  2 +-
 web/src/assets/styles/variables.scss     |  1 +
 web/src/components/core/Section.jsx      |  8 ++++++-
 web/src/components/core/Section.test.jsx | 26 ++++++++++++++++++++---
 5 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss
index a6d42c2e68..52a3926d6a 100644
--- a/web/src/assets/styles/blocks.scss
+++ b/web/src/assets/styles/blocks.scss
@@ -18,24 +18,35 @@
     margin-block-end: var(--spacer-medium);
   }
 
-  > h2 {
+  > header {
     display: grid;
     grid-area: header;
     grid-template-columns: subgrid;
     grid-column: bleed / content-end;
 
-    svg {
-      block-size: var(--section-icon-size);
-      inline-size: var(--section-icon-size);
-      grid-column: bleed / content;
+    h2 {
+      display: grid;
+      grid-template-columns: subgrid;
+      grid-column: bleed / content-end;
+
+      svg {
+        block-size: var(--section-icon-size);
+        inline-size: var(--section-icon-size);
+        grid-column: bleed / content;
+      }
+
+      :not(svg) {
+        grid-column: content
+      }
     }
 
-    :not(svg) {
-      grid-column: content
+    p {
+      grid-column: content;
+      color: var(--color-gray-dimmest);
     }
   }
 
-  > :not(h2) {
+  > :not(header) {
     grid-area: content;
     grid-column: content;
   }
diff --git a/web/src/assets/styles/layout.scss b/web/src/assets/styles/layout.scss
index 6e879528f1..d8a7034ad5 100644
--- a/web/src/assets/styles/layout.scss
+++ b/web/src/assets/styles/layout.scss
@@ -20,7 +20,7 @@
     padding: var(--spacer-small);
   }
 
-  header {
+  > header {
     @extend .bottom-shadow;
     grid-area: header;
     display: flex;
diff --git a/web/src/assets/styles/variables.scss b/web/src/assets/styles/variables.scss
index fb96b3058f..b25bfcbad3 100644
--- a/web/src/assets/styles/variables.scss
+++ b/web/src/assets/styles/variables.scss
@@ -52,6 +52,7 @@
   --color-gray-dark: #efefef; // Fog
   --color-gray-darker: #999;
   --color-gray-dimmed: #888;
+  --color-gray-dimmest: #666;
 
   --color-link: #0c322c;
   --color-link-hover: #30ba78;
diff --git a/web/src/components/core/Section.jsx b/web/src/components/core/Section.jsx
index 0a6b755740..c6323e6b54 100644
--- a/web/src/components/core/Section.jsx
+++ b/web/src/components/core/Section.jsx
@@ -48,6 +48,7 @@ import { ValidationErrors } from "~/components/core";
  * @typedef { Object } SectionProps
  * @property {string} [icon] - Name of the section icon. Not rendered if title is not provided.
  * @property {string} [title] - The section title. If not given, aria-label must be provided.
+ * @property {string} [description] - A section description. Use only if really needed.
  * @property {string} [name] - The section name. Used to build the header id.
  * @property {string} [path] - Path where the section links to.
  *  when user clicks on the title, used for opening a dialog.
@@ -63,6 +64,7 @@ import { ValidationErrors } from "~/components/core";
 export default function Section({
   icon,
   title,
+  description,
   name,
   path,
   loading,
@@ -86,7 +88,11 @@ export default function Section({
     const headerText = !path?.trim() ? title : <Link to={path}>{title}</Link>;
 
     return (
-      <h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
+      <header>
+        <h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
+        <p>{description}</p>
+      </header>
+
     );
   };
 
diff --git a/web/src/components/core/Section.test.jsx b/web/src/components/core/Section.test.jsx
index d7d5db2a81..f132a637a6 100644
--- a/web/src/components/core/Section.test.jsx
+++ b/web/src/components/core/Section.test.jsx
@@ -36,7 +36,13 @@ describe("Section", () => {
   describe("when title is given", () => {
     it("renders the section header", () => {
       plainRender(<Section title="Settings" />);
-      screen.getByRole("heading", { name: "Settings" });
+      screen.getByRole("banner");
+    });
+
+    it("renders given title as section heading", () => {
+      plainRender(<Section title="Settings" />);
+      const header = screen.getByRole("banner");
+      within(header).getByRole("heading", { name: "Settings" });
     });
 
     it("renders an icon if valid icon name is given", () => {
@@ -58,15 +64,29 @@ describe("Section", () => {
       const icon = container.querySelector("svg");
       expect(icon).toBeNull();
     });
+
+    it("renders given description as part of the header", () => {
+      plainRender(
+        <Section title="Settings" description="Short explanation to help the user, if needed" />
+      );
+      const header = screen.getByRole("banner");
+      within(header).getByText(/Short explanation/);
+    });
   });
 
   describe("when title is not given", () => {
     it("does not render the section header", async () => {
-      plainRender(<Section />);
-      const header = await screen.queryByRole("heading");
+      plainRender(<Section description="Does not matter" />);
+      const header = await screen.queryByRole("banner");
       expect(header).not.toBeInTheDocument();
     });
 
+    it("does not render a section heading", async () => {
+      plainRender(<Section description="Does not matter" />);
+      const heading = await screen.queryByRole("heading");
+      expect(heading).not.toBeInTheDocument();
+    });
+
     it("does not render the section icon", () => {
       const { container } = plainRender(<Section icon="settings" />);
       const icon = container.querySelector("svg");

From 8c61f03f762eedeab6ac907dc94254cf3c8ddb0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Thu, 22 Feb 2024 12:39:16 +0000
Subject: [PATCH 04/11] [web] Add descriptions to some storage sections

Based on @ancorgs's comment at https://github.com/openSUSE/agama/pull/1045#issuecomment-1959066761
---
 .../storage/ProposalActionsSection.jsx          | 17 ++++++++++-------
 .../storage/ProposalActionsSection.test.jsx     |  3 ++-
 .../storage/ProposalDeviceSection.jsx           | 16 +++++++++++++++-
 .../storage/ProposalSpacePolicySection.jsx      |  7 ++++++-
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/web/src/components/storage/ProposalActionsSection.jsx b/web/src/components/storage/ProposalActionsSection.jsx
index e6a272b8e5..6310ad9fc1 100644
--- a/web/src/components/storage/ProposalActionsSection.jsx
+++ b/web/src/components/storage/ProposalActionsSection.jsx
@@ -25,7 +25,6 @@ import {
   ListItem,
   ExpandableSection,
   Skeleton,
-  Text
 } from "@patternfly/react-core";
 import { sprintf } from "sprintf-js";
 
@@ -74,9 +73,6 @@ const ProposalActions = ({ actions = [] }) => {
 
   return (
     <>
-      <Text>
-        {_("Actions to create the file systems and to ensure the system boots.")}
-      </Text>
       <ActionsList actions={generalActions} />
       {subvolActions.length > 0 && (
         <ExpandableSection
@@ -121,9 +117,16 @@ export default function ProposalActionsSection({ actions = [], errors = [], isLo
   if (isLoading) errors = [];
 
   return (
-    // TRANSLATORS: section title, list of planned actions for the selected device,
-    // e.g. "delete partition A", "create partition B with filesystem C", ...
-    <Section title={_("Planned Actions")} id="storage-actions" errors={errors}>
+    <Section
+      // TRANSLATORS: The storage "Planned Actions" section's title. The
+      // section shows a list of planned actions for the selected device, e.g.
+      // "delete partition A", "create partition B with filesystem C", ...
+      title={_("Planned Actions")}
+      // TRANSLATORS: The storage "Planned Actions" section's description
+      description={_("Actions to create the file systems and to ensure the new system boots")}
+      id="storage-actions"
+      errors={errors}
+    >
       <If
         condition={isLoading}
         then={<ActionsSkeleton />}
diff --git a/web/src/components/storage/ProposalActionsSection.test.jsx b/web/src/components/storage/ProposalActionsSection.test.jsx
index 46d9bf4964..9864391d0d 100644
--- a/web/src/components/storage/ProposalActionsSection.test.jsx
+++ b/web/src/components/storage/ProposalActionsSection.test.jsx
@@ -65,8 +65,9 @@ it("renders skeleton while loading", () => {
 it("renders nothing when there is no actions", () => {
   plainRender(<ProposalActionsSection actions={[]} />);
 
-  expect(screen.queryByText(/Actions to create/)).toBeNull();
+  expect(screen.queryAllByText(/Delete/)).toEqual([]);
   expect(screen.queryAllByText(/Create/)).toEqual([]);
+  expect(screen.queryAllByText(/Show/)).toEqual([]);
 });
 
 describe("when there are actions", () => {
diff --git a/web/src/components/storage/ProposalDeviceSection.jsx b/web/src/components/storage/ProposalDeviceSection.jsx
index 8c2f3dc61c..1b8a42e349 100644
--- a/web/src/components/storage/ProposalDeviceSection.jsx
+++ b/web/src/components/storage/ProposalDeviceSection.jsx
@@ -195,8 +195,22 @@ export default function ProposalDeviceSection({
     }
   };
 
+  const Description = () => (
+    <span
+      dangerouslySetInnerHTML={{
+        // TRANSLATORS: The storage "Device" sections's description. Do not
+        // translate 'abbr' and 'title', they are part of the HTML markup.
+        __html: _("Select the main disk or <abbr title='Logical Volume Manager'>LVM</abbr> Volume Group for installation")
+      }}
+    />
+  );
+
   return (
-    <Section title={_("Device")}>
+    <Section
+      // TRANSLATORS: The storage "Device" section's title
+      title={_("Device")}
+      description={<Description />}
+    >
       <InstallationDeviceField
         current={bootDevice}
         devices={availableDevices}
diff --git a/web/src/components/storage/ProposalSpacePolicySection.jsx b/web/src/components/storage/ProposalSpacePolicySection.jsx
index 9c2d65fc45..e5735412df 100644
--- a/web/src/components/storage/ProposalSpacePolicySection.jsx
+++ b/web/src/components/storage/ProposalSpacePolicySection.jsx
@@ -447,7 +447,12 @@ export default function ProposalSpacePolicySection({
   const currentPolicy = SPACE_POLICIES.find(p => p.id === settings.spacePolicy) || SPACE_POLICIES[0];
 
   return (
-    <Section title={_("Find Space")}>
+    <Section
+      // TRANSLATORS: The storage "Find Space" section's title
+      title={_("Find Space")}
+      // TRANSLATORS: The storage "Find space" sections's description
+      description={_("Allocating the file systems might need to find free space in the devices listed below. Choose how to do it")}
+    >
       <If
         condition={isLoading && settings.spacePolicy === undefined}
         then={<SectionSkeleton numRows={4} />}

From 1d34f4c34abc0f64f4bcbb022ee86eb74da5a69f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Thu, 22 Feb 2024 12:40:13 +0000
Subject: [PATCH 05/11] [web] Drop redundant text in the file systems section

We've agreed that section title states almost the same.
---
 web/src/components/storage/ProposalVolumes.jsx | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/web/src/components/storage/ProposalVolumes.jsx b/web/src/components/storage/ProposalVolumes.jsx
index d9f16334c0..8b6398b43a 100644
--- a/web/src/components/storage/ProposalVolumes.jsx
+++ b/web/src/components/storage/ProposalVolumes.jsx
@@ -404,9 +404,6 @@ export default function ProposalVolumes({
     <>
       <Toolbar>
         <ToolbarContent>
-          <ToolbarItem>
-            {_("File systems to create in your system")}
-          </ToolbarItem>
           <ToolbarItem align={{ default: "alignRight" }}>
             <GeneralActions
               templates={templates}

From e8896a5fdb604a2769508a3f617975f9ee268f95 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Thu, 22 Feb 2024 12:43:21 +0000
Subject: [PATCH 06/11] [web] Fine tuning styles

---
 web/src/assets/styles/blocks.scss                      | 1 +
 web/src/assets/styles/patternfly-overrides.scss        | 1 -
 web/src/components/storage/ProposalSettingsSection.jsx | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss
index 52a3926d6a..30e95e4aa4 100644
--- a/web/src/assets/styles/blocks.scss
+++ b/web/src/assets/styles/blocks.scss
@@ -43,6 +43,7 @@
     p {
       grid-column: content;
       color: var(--color-gray-dimmest);
+      margin-block-end: var(--spacer-smaller);
     }
   }
 
diff --git a/web/src/assets/styles/patternfly-overrides.scss b/web/src/assets/styles/patternfly-overrides.scss
index 90fa1bb699..9127b55a5b 100644
--- a/web/src/assets/styles/patternfly-overrides.scss
+++ b/web/src/assets/styles/patternfly-overrides.scss
@@ -127,7 +127,6 @@ table td > .pf-v5-c-empty-state {
   --stack-gutter: 0;
   --pf-v5-c-toolbar--PaddingTop: 0;
   --pf-v5-c-toolbar--PaddingBottom: 0;
-  border-block-end: 1px solid var(--color-gray-light);
 }
 
 .pf-v5-c-toolbar__content {
diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx
index 7084e3a09b..9dedef6fd0 100644
--- a/web/src/components/storage/ProposalSettingsSection.jsx
+++ b/web/src/components/storage/ProposalSettingsSection.jsx
@@ -451,7 +451,7 @@ export default function ProposalSettingsSection({
 
   return (
     <>
-      <Section title={_("Settings")} className="flex-stack">
+      <Section title={_("Settings")}>
         <LVMField
           settings={settings}
           devices={availableDevices}

From 93c4d21daf7b02ae36866ffde14a68ecb83ba90f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Thu, 22 Feb 2024 12:48:23 +0000
Subject: [PATCH 07/11] [web] Render section title conditionally

To avoid having empty <p> HTML nodes when description is not given.
---
 web/src/components/core/Section.jsx | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/web/src/components/core/Section.jsx b/web/src/components/core/Section.jsx
index c6323e6b54..c1954b7d4e 100644
--- a/web/src/components/core/Section.jsx
+++ b/web/src/components/core/Section.jsx
@@ -24,7 +24,7 @@
 import React from "react";
 import { Link } from "react-router-dom";
 import { Icon } from '~/components/layout';
-import { ValidationErrors } from "~/components/core";
+import { If, ValidationErrors } from "~/components/core";
 
 /**
  * Renders children into an HTML section
@@ -48,7 +48,7 @@ import { ValidationErrors } from "~/components/core";
  * @typedef { Object } SectionProps
  * @property {string} [icon] - Name of the section icon. Not rendered if title is not provided.
  * @property {string} [title] - The section title. If not given, aria-label must be provided.
- * @property {string} [description] - A section description. Use only if really needed.
+ * @property {string|React.ReactElement} [description] - A section description. Use only if really needed.
  * @property {string} [name] - The section name. Used to build the header id.
  * @property {string} [path] - Path where the section links to.
  *  when user clicks on the title, used for opening a dialog.
@@ -86,13 +86,13 @@ export default function Section({
     const iconName = loading ? "loading" : icon;
     const headerIcon = iconName ? <Icon name={iconName} /> : null;
     const headerText = !path?.trim() ? title : <Link to={path}>{title}</Link>;
+    const renderDescription = React.isValidElement(description) || description?.length > 0;
 
     return (
       <header>
         <h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
-        <p>{description}</p>
+        <If condition={renderDescription} then={<p>{description}</p>} />
       </header>
-
     );
   };
 

From ba0114deede31dd1915918ba4412eaf4a7fabf74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Thu, 22 Feb 2024 14:01:06 +0000
Subject: [PATCH 08/11] [web] Move LVM setting to storage device section

Since, using @ancorgs's words, it is more

  > consistent with the future we want to go (in which LVM is part of
  > the device selection)
---
 .../storage/ProposalDeviceSection.jsx         | 222 +++++++++++-
 .../storage/ProposalDeviceSection.test.jsx    | 342 ++++++++++++++----
 .../storage/ProposalSettingsSection.jsx       | 219 +----------
 .../storage/ProposalSettingsSection.test.jsx  | 213 -----------
 4 files changed, 499 insertions(+), 497 deletions(-)

diff --git a/web/src/components/storage/ProposalDeviceSection.jsx b/web/src/components/storage/ProposalDeviceSection.jsx
index 1b8a42e349..6b9ed801f5 100644
--- a/web/src/components/storage/ProposalDeviceSection.jsx
+++ b/web/src/components/storage/ProposalDeviceSection.jsx
@@ -20,11 +20,19 @@
  */
 
 import React, { useState } from "react";
-import { Button, Form, Skeleton } from "@patternfly/react-core";
+import {
+  Button,
+  Form,
+  Skeleton,
+  Switch,
+  ToggleGroup, ToggleGroupItem,
+  Tooltip
+} from "@patternfly/react-core";
 
 import { _ } from "~/i18n";
+import { Icon } from "~/components/layout";
 import { If, Section, Popup } from "~/components/core";
-import { DeviceSelector } from "~/components/storage";
+import { DeviceList, DeviceSelector } from "~/components/storage";
 import { deviceLabel } from '~/components/storage/utils';
 import { noop } from "~/utils";
 
@@ -167,6 +175,201 @@ const InstallationDeviceField = ({
   );
 };
 
+/**
+ * Form for configuring the system volume group.
+ * @component
+ *
+ * @param {object} props
+ * @param {string} props.id - Form ID.
+ * @param {ProposalSettings} props.settings - Settings used for calculating a proposal.
+ * @param {StorageDevice[]} [props.devices=[]] - Available storage devices.
+ * @param {onSubmitFn} [props.onSubmit=noop] - On submit callback.
+ * @param {onValidateFn} [props.onValidate=noop] - On validate callback.
+ *
+ * @callback onSubmitFn
+ * @param {string[]} devices - Name of the selected devices.
+ *
+ * @callback onValidateFn
+ * @param {boolean} valid
+ */
+const LVMSettingsForm = ({
+  id,
+  settings,
+  devices = [],
+  onSubmit: onSubmitProp = noop,
+  onValidate = noop
+}) => {
+  const [vgDevices, setVgDevices] = useState(settings.systemVGDevices);
+  const [isBootDeviceSelected, setIsBootDeviceSelected] = useState(settings.systemVGDevices.length === 0);
+  const [editedDevices, setEditedDevices] = useState(false);
+
+  const selectBootDevice = () => {
+    setIsBootDeviceSelected(true);
+    onValidate(true);
+  };
+
+  const selectCustomDevices = () => {
+    setIsBootDeviceSelected(false);
+    const { bootDevice } = settings;
+    const customDevices = (vgDevices.length === 0 && !editedDevices) ? [bootDevice] : vgDevices;
+    setVgDevices(customDevices);
+    onValidate(customDevices.length > 0);
+  };
+
+  const onChangeDevices = (selection) => {
+    const selectedDevices = devices.filter(d => selection.includes(d.sid)).map(d => d.name);
+    setVgDevices(selectedDevices);
+    setEditedDevices(true);
+    onValidate(devices.length > 0);
+  };
+
+  const onSubmit = (e) => {
+    e.preventDefault();
+    const customDevices = isBootDeviceSelected ? [] : vgDevices;
+    onSubmitProp(customDevices);
+  };
+
+  const BootDevice = () => {
+    const bootDevice = devices.find(d => d.name === settings.bootDevice);
+
+    // FIXME: In this case, should be a "readOnly" selector.
+    return <DeviceList devices={[bootDevice]} />;
+  };
+
+  return (
+    <Form id={id} onSubmit={onSubmit}>
+      <div className="split">
+        <span>{_("Devices for creating the volume group")}</span>
+        <ToggleGroup isCompact>
+          <ToggleGroupItem
+            text={_("Installation device")}
+            buttonId="bootDevice"
+            isSelected={isBootDeviceSelected}
+            onClick={selectBootDevice}
+          />
+          <ToggleGroupItem
+            text={_("Custom devices")}
+            buttonId="customDevices"
+            isSelected={!isBootDeviceSelected}
+            onClick={selectCustomDevices}
+          />
+        </ToggleGroup>
+      </div>
+      <If
+        condition={isBootDeviceSelected}
+        then={<BootDevice />}
+        else={
+          <DeviceSelector
+            isMultiple
+            selected={devices.filter(d => vgDevices?.includes(d.name))}
+            devices={devices}
+            onChange={onChangeDevices}
+          />
+        }
+      />
+    </Form>
+  );
+};
+
+/**
+ * Allows to select LVM and configure the system volume group.
+ * @component
+ *
+ * @param {object} props
+ * @param {ProposalSettings} props.settings - Settings used for calculating a proposal.
+ * @param {StorageDevice[]} [props.devices=[]] - Available storage devices.
+ * @param {boolean} [props.isChecked=false] - Whether LVM is selected.
+ * @param {boolean} [props.isLoading=false] - Whether to show the selector as loading.
+ * @param {onChangeFn} [props.onChange=noop] - On change callback.
+ *
+ * @callback onChangeFn
+ * @param {boolean} lvm
+ */
+const LVMField = ({
+  settings,
+  devices = [],
+  isChecked: isCheckedProp = false,
+  isLoading = false,
+  onChange: onChangeProp = noop
+}) => {
+  const [isChecked, setIsChecked] = useState(isCheckedProp);
+  const [isFormOpen, setIsFormOpen] = useState(false);
+  const [isFormValid, setIsFormValid] = useState(true);
+
+  const onChange = (_, value) => {
+    setIsChecked(value);
+    onChangeProp({ lvm: value, vgDevices: [] });
+  };
+
+  const openForm = () => setIsFormOpen(true);
+
+  const closeForm = () => setIsFormOpen(false);
+
+  const onValidateForm = (valid) => setIsFormValid(valid);
+
+  const onSubmitForm = (vgDevices) => {
+    closeForm();
+    onChangeProp({ vgDevices });
+  };
+
+  const description = _("Configuration of the system volume group. All the file systems will be \
+created in a logical volume of the system volume group.");
+
+  const LVMSettingsButton = () => {
+    return (
+      <Tooltip
+        content={_("Configure the LVM settings")}
+        entryDelay={400}
+        exitDelay={50}
+        position="right"
+      >
+        <button aria-label={_("LVM settings")} className="plain-control" onClick={openForm}>
+          <Icon name="tune" size="s" />
+        </button>
+      </Tooltip>
+    );
+  };
+
+  if (isLoading) return <Skeleton width="25%" />;
+
+  return (
+    <div className="split">
+      <Switch
+        id="lvm"
+        label={_("Use logical volume management (LVM)")}
+        isReversed
+        isChecked={isChecked}
+        onChange={onChange}
+      />
+      <If condition={isChecked} then={<LVMSettingsButton />} />
+      <Popup
+        aria-label={_("LVM settings")}
+        title={_("System Volume Group")}
+        description={description}
+        isOpen={isFormOpen}
+      >
+        <LVMSettingsForm
+          id="lvmSettingsForm"
+          devices={devices}
+          settings={settings}
+          onSubmit={onSubmitForm}
+          onValidate={onValidateForm}
+        />
+        <Popup.Actions>
+          <Popup.Confirm
+            form="lvmSettingsForm"
+            type="submit"
+            isDisabled={!isFormValid}
+          >
+            {_("Accept")}
+          </Popup.Confirm>
+          <Popup.Cancel onClick={closeForm} />
+        </Popup.Actions>
+      </Popup>
+    </div>
+  );
+};
+
 /**
  * Section for editing the selected device
  * @component
@@ -195,6 +398,14 @@ export default function ProposalDeviceSection({
     }
   };
 
+  const changeLVM = ({ lvm, vgDevices }) => {
+    const settings = {};
+    if (lvm !== undefined) settings.lvm = lvm;
+    if (vgDevices !== undefined) settings.systemVGDevices = vgDevices;
+
+    onChange(settings);
+  };
+
   const Description = () => (
     <span
       dangerouslySetInnerHTML={{
@@ -217,6 +428,13 @@ export default function ProposalDeviceSection({
         isLoading={isLoading && bootDevice === undefined}
         onChange={changeBootDevice}
       />
+      <LVMField
+        settings={settings}
+        devices={availableDevices}
+        isChecked={settings.lvm === true}
+        isLoading={settings.lvm === undefined}
+        onChange={changeLVM}
+      />
     </Section>
   );
 }
diff --git a/web/src/components/storage/ProposalDeviceSection.test.jsx b/web/src/components/storage/ProposalDeviceSection.test.jsx
index e90db6a054..27bb058910 100644
--- a/web/src/components/storage/ProposalDeviceSection.test.jsx
+++ b/web/src/components/storage/ProposalDeviceSection.test.jsx
@@ -66,6 +66,53 @@ const sdb = {
   udevPaths: ["pci-0000:00-19"]
 };
 
+const vda = {
+  sid: "59",
+  type: "disk",
+  vendor: "Micron",
+  model: "Micron 1100 SATA",
+  driver: ["ahci", "mmcblk"],
+  bus: "IDE",
+  transport: "usb",
+  dellBOSS: false,
+  sdCard: true,
+  active: true,
+  name: "/dev/vda",
+  size: 1024,
+  systems: ["Windows", "openSUSE Leap 15.2"],
+  udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"],
+  udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"],
+  partitionTable: { type: "gpt", partitions: [] }
+};
+
+const md0 = {
+  sid: "62",
+  type: "md",
+  level: "raid0",
+  uuid: "12345:abcde",
+  members: ["/dev/vdb"],
+  active: true,
+  name: "/dev/md0",
+  size: 2048,
+  systems: [],
+  udevIds: [],
+  udevPaths: []
+};
+
+const md1 = {
+  sid: "63",
+  type: "md",
+  level: "raid0",
+  uuid: "12345:abcde",
+  members: ["/dev/vdc"],
+  active: true,
+  name: "/dev/md1",
+  size: 4096,
+  systems: [],
+  udevIds: [],
+  udevPaths: []
+};
+
 const props = {
   settings: {
     bootDevice: "/dev/sda",
@@ -76,107 +123,274 @@ const props = {
 };
 
 describe("ProposalDeviceSection", () => {
-  describe("when set as loading", () => {
-    beforeEach(() => {
-      props.isLoading = true;
-    });
+  describe("Installation device field", () => {
+    describe("when set as loading", () => {
+      beforeEach(() => {
+        props.isLoading = true;
+      });
 
-    describe("and selected device is not defined yet", () => {
+      describe("and selected device is not defined yet", () => {
+        beforeEach(() => {
+          props.settings = { bootDevice: undefined };
+        });
+
+        it("renders a loading hint", () => {
+          plainRender(<ProposalDeviceSection {...props} />);
+          screen.getByText("Loading selected device");
+        });
+      });
+    });
+    describe("when installation device is not selected yet", () => {
       beforeEach(() => {
-        props.settings = { bootDevice: undefined };
+        props.settings = { bootDevice: "" };
       });
 
-      it("renders a loading hint", () => {
-        plainRender(<ProposalDeviceSection {...props} />);
-        screen.getByText("Loading selected device");
+      it("uses a 'No device selected yet' text for the selection button", async () => {
+        const { user } = plainRender(<ProposalDeviceSection {...props} />);
+        const button = screen.getByRole("button", { name: "No device selected yet" });
+
+        await user.click(button);
+
+        screen.getByRole("dialog", { name: "Installation device" });
       });
     });
-  });
 
-  describe("when installation device is not selected yet", () => {
-    beforeEach(() => {
-      props.settings = { bootDevice: "" };
+    describe("when installation device is selected", () => {
+      beforeEach(() => {
+        props.settings = { bootDevice: "/dev/sda" };
+      });
+
+      it("uses its name as part of the text for the selection button", async () => {
+        const { user } = plainRender(<ProposalDeviceSection {...props} />);
+        const button = screen.getByRole("button", { name: /\/dev\/sda/ });
+
+        await user.click(button);
+
+        screen.getByRole("dialog", { name: "Installation device" });
+      });
     });
 
-    it("uses a 'No device selected yet' text for the selection button", async () => {
+    it("allows changing the selected device", async () => {
       const { user } = plainRender(<ProposalDeviceSection {...props} />);
-      const button = screen.getByRole("button", { name: "No device selected yet" });
+      const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
 
       await user.click(button);
 
-      screen.getByRole("dialog", { name: "Installation device" });
+      const selector = await screen.findByRole("dialog", { name: "Installation device" });
+      const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
+      const accept = within(selector).getByRole("button", { name: "Accept" });
+
+      await user.click(sdbOption);
+      await user.click(accept);
+
+      expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
+      expect(props.onChange).toHaveBeenCalledWith({ bootDevice: sdb.name });
     });
-  });
 
-  describe("when installation device is selected", () => {
-    beforeEach(() => {
-      props.settings = { bootDevice: "/dev/sda" };
+    it("allows canceling a device selection", async () => {
+      const { user } = plainRender(<ProposalDeviceSection {...props} />);
+      const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
+
+      await user.click(button);
+
+      const selector = await screen.findByRole("dialog", { name: "Installation device" });
+      const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
+      const cancel = within(selector).getByRole("button", { name: "Cancel" });
+
+      await user.click(sdbOption);
+      await user.click(cancel);
+
+      expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
+      expect(props.onChange).not.toHaveBeenCalled();
     });
 
-    it("uses its name as part of the text for the selection button", async () => {
+    it("does not trigger the onChange callback when selection actually did not change", async () => {
       const { user } = plainRender(<ProposalDeviceSection {...props} />);
-      const button = screen.getByRole("button", { name: /\/dev\/sda/ });
+      const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
 
       await user.click(button);
 
-      screen.getByRole("dialog", { name: "Installation device" });
+      const selector = await screen.findByRole("dialog", { name: "Installation device" });
+      const sdaOption = within(selector).getByRole("radio", { name: /sda/ });
+      const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
+      const accept = within(selector).getByRole("button", { name: "Accept" });
+
+      // User selects a different device
+      await user.click(sdbOption);
+      // but then goes back to the selected device
+      await user.click(sdaOption);
+      // and clicks on Accept button
+      await user.click(accept);
+
+      expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
+      // There is no reason for triggering the onChange callback
+      expect(props.onChange).not.toHaveBeenCalled();
     });
   });
 
-  it("allows changing the selected device", async () => {
-    const { user } = plainRender(<ProposalDeviceSection {...props} />);
-    const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
+  describe("LVM field", () => {
+    describe("if LVM setting is not set yet", () => {
+      beforeEach(() => {
+        props.settings = {};
+      });
 
-    await user.click(button);
+      it("does not render the LVM switch", () => {
+        plainRender(<ProposalDeviceSection {...props} />);
 
-    const selector = await screen.findByRole("dialog", { name: "Installation device" });
-    const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
-    const accept = within(selector).getByRole("button", { name: "Accept" });
+        expect(screen.queryByLabelText(/Use logical volume/)).toBeNull();
+      });
+    });
 
-    await user.click(sdbOption);
-    await user.click(accept);
+    describe("if LVM setting is set", () => {
+      beforeEach(() => {
+        props.settings = { lvm: false };
+      });
 
-    expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
-    expect(props.onChange).toHaveBeenCalledWith({ bootDevice: sdb.name });
-  });
+      it("renders the LVM switch", () => {
+        plainRender(<ProposalDeviceSection {...props} />);
+
+        screen.getByRole("checkbox", { name: /Use logical volume/ });
+      });
+    });
 
-  it("allows canceling a device selection", async () => {
-    const { user } = plainRender(<ProposalDeviceSection {...props} />);
-    const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
+    describe("if LVM is set to true", () => {
+      beforeEach(() => {
+        props.availableDevices = [vda, md0, md1];
+        props.settings = { bootDevice: "/dev/vda", lvm: true, systemVGDevices: [] };
+        props.onChange = jest.fn();
+      });
 
-    await user.click(button);
+      it("renders the LVM switch as selected", () => {
+        plainRender(<ProposalDeviceSection {...props} />);
 
-    const selector = await screen.findByRole("dialog", { name: "Installation device" });
-    const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
-    const cancel = within(selector).getByRole("button", { name: "Cancel" });
+        const checkbox = screen.getByRole("checkbox", { name: /Use logical volume/ });
+        expect(checkbox).toBeChecked();
+      });
 
-    await user.click(sdbOption);
-    await user.click(cancel);
+      it("renders a button for changing the LVM settings", () => {
+        plainRender(<ProposalDeviceSection {...props} />);
 
-    expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
-    expect(props.onChange).not.toHaveBeenCalled();
-  });
+        screen.getByRole("button", { name: /LVM settings/ });
+      });
+
+      it("changes the selection on click", async () => {
+        const { user } = plainRender(<ProposalDeviceSection {...props} />);
+
+        const checkbox = screen.getByRole("checkbox", { name: /Use logical volume/ });
+        await user.click(checkbox);
+
+        expect(checkbox).not.toBeChecked();
+        expect(props.onChange).toHaveBeenCalled();
+      });
+
+      describe("and user clicks on LVM settings", () => {
+        it("opens the LVM settings dialog", async () => {
+          const { user } = plainRender(<ProposalDeviceSection {...props} />);
+          const settingsButton = screen.getByRole("button", { name: /LVM settings/ });
+
+          await user.click(settingsButton);
+
+          const popup = await screen.findByRole("dialog");
+          within(popup).getByText("System Volume Group");
+        });
+
+        it("allows selecting either installation device or custom devices", async () => {
+          const { user } = plainRender(<ProposalDeviceSection {...props} />);
+          const settingsButton = screen.getByRole("button", { name: /LVM settings/ });
+
+          await user.click(settingsButton);
+
+          const popup = await screen.findByRole("dialog");
+          screen.getByText("System Volume Group");
+
+          within(popup).getByRole("button", { name: "Installation device" });
+          within(popup).getByRole("button", { name: "Custom devices" });
+        });
+
+        it("allows to set the installation device as system volume group", async () => {
+          const { user } = plainRender(<ProposalDeviceSection {...props} />);
+          const settingsButton = screen.getByRole("button", { name: /LVM settings/ });
+
+          await user.click(settingsButton);
 
-  it("does not trigger the onChange callback when selection actually did not change", async () => {
-    const { user } = plainRender(<ProposalDeviceSection {...props} />);
-    const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" });
+          const popup = await screen.findByRole("dialog");
+          screen.getByText("System Volume Group");
 
-    await user.click(button);
+          const bootDeviceButton = within(popup).getByRole("button", { name: "Installation device" });
+          const customDevicesButton = within(popup).getByRole("button", { name: "Custom devices" });
+          const acceptButton = within(popup).getByRole("button", { name: "Accept" });
 
-    const selector = await screen.findByRole("dialog", { name: "Installation device" });
-    const sdaOption = within(selector).getByRole("radio", { name: /sda/ });
-    const sdbOption = within(selector).getByRole("radio", { name: /sdb/ });
-    const accept = within(selector).getByRole("button", { name: "Accept" });
+          await user.click(customDevicesButton);
+          await user.click(bootDeviceButton);
+          await user.click(acceptButton);
 
-    // User selects a different device
-    await user.click(sdbOption);
-    // but then goes back to the selected device
-    await user.click(sdaOption);
-    // and clicks on Accept button
-    await user.click(accept);
+          expect(props.onChange).toHaveBeenCalledWith(
+            expect.objectContaining({ systemVGDevices: [] })
+          );
+        });
 
-    expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
-    // There is no reason for triggering the onChange callback
-    expect(props.onChange).not.toHaveBeenCalled();
+        it("allows customize the system volume group", async () => {
+          const { user } = plainRender(<ProposalDeviceSection {...props} />);
+          const settingsButton = screen.getByRole("button", { name: /LVM settings/ });
+
+          await user.click(settingsButton);
+
+          const popup = await screen.findByRole("dialog");
+          screen.getByText("System Volume Group");
+
+          const customDevicesButton = within(popup).getByRole("button", { name: "Custom devices" });
+          const acceptButton = within(popup).getByRole("button", { name: "Accept" });
+
+          await user.click(customDevicesButton);
+
+          const vdaOption = within(popup).getByRole("row", { name: /vda/ });
+          const md0Option = within(popup).getByRole("row", { name: /md0/ });
+          const md1Option = within(popup).getByRole("row", { name: /md1/ });
+
+          // unselect the boot devices
+          await user.click(vdaOption);
+
+          await user.click(md0Option);
+          await user.click(md1Option);
+
+          await user.click(acceptButton);
+
+          expect(props.onChange).toHaveBeenCalledWith(
+            expect.objectContaining({ systemVGDevices: ["/dev/md0", "/dev/md1"] })
+          );
+        });
+      });
+    });
+
+    describe("if LVM is set to false", () => {
+      beforeEach(() => {
+        props.settings = { lvm: false };
+        props.onChange = jest.fn();
+      });
+
+      it("renders the LVM switch as not selected", () => {
+        plainRender(<ProposalDeviceSection {...props} />);
+
+        const checkbox = screen.getByRole("checkbox", { name: /Use logical volume/ });
+        expect(checkbox).not.toBeChecked();
+      });
+
+      it("does not render a button for changing the LVM settings", () => {
+        plainRender(<ProposalDeviceSection {...props} />);
+
+        const button = screen.queryByRole("button", { name: /LVM settings/ });
+        expect(button).toBeNull();
+      });
+
+      it("changes the selection on click", async () => {
+        const { user } = plainRender(<ProposalDeviceSection {...props} />);
+
+        const checkbox = screen.getByRole("checkbox", { name: /Use logical volume/ });
+        await user.click(checkbox);
+
+        expect(checkbox).toBeChecked();
+        expect(props.onChange).toHaveBeenCalled();
+      });
+    });
   });
 });
diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx
index 9dedef6fd0..1a8812d949 100644
--- a/web/src/components/storage/ProposalSettingsSection.jsx
+++ b/web/src/components/storage/ProposalSettingsSection.jsx
@@ -20,15 +20,10 @@
  */
 
 import React, { useEffect, useState } from "react";
-import {
-  Form, Skeleton, Switch, Checkbox,
-  ToggleGroup, ToggleGroupItem,
-  Tooltip
-} from "@patternfly/react-core";
+import { Checkbox, Form, Skeleton, Switch, Tooltip } from "@patternfly/react-core";
 
 import { _ } from "~/i18n";
 import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core";
-import { DeviceList, DeviceSelector } from "~/components/storage";
 import { Icon } from "~/components/layout";
 import { noop } from "~/utils";
 
@@ -38,201 +33,6 @@ import { noop } from "~/utils";
  * @typedef {import ("~/client/storage").ProposalManager.Volume} Volume
  */
 
-/**
- * Form for configuring the system volume group.
- * @component
- *
- * @param {object} props
- * @param {string} props.id - Form ID.
- * @param {ProposalSettings} props.settings - Settings used for calculating a proposal.
- * @param {StorageDevice[]} [props.devices=[]] - Available storage devices.
- * @param {onSubmitFn} [props.onSubmit=noop] - On submit callback.
- * @param {onValidateFn} [props.onValidate=noop] - On validate callback.
- *
- * @callback onSubmitFn
- * @param {string[]} devices - Name of the selected devices.
- *
- * @callback onValidateFn
- * @param {boolean} valid
- */
-const LVMSettingsForm = ({
-  id,
-  settings,
-  devices = [],
-  onSubmit: onSubmitProp = noop,
-  onValidate = noop
-}) => {
-  const [vgDevices, setVgDevices] = useState(settings.systemVGDevices);
-  const [isBootDeviceSelected, setIsBootDeviceSelected] = useState(settings.systemVGDevices.length === 0);
-  const [editedDevices, setEditedDevices] = useState(false);
-
-  const selectBootDevice = () => {
-    setIsBootDeviceSelected(true);
-    onValidate(true);
-  };
-
-  const selectCustomDevices = () => {
-    setIsBootDeviceSelected(false);
-    const { bootDevice } = settings;
-    const customDevices = (vgDevices.length === 0 && !editedDevices) ? [bootDevice] : vgDevices;
-    setVgDevices(customDevices);
-    onValidate(customDevices.length > 0);
-  };
-
-  const onChangeDevices = (selection) => {
-    const selectedDevices = devices.filter(d => selection.includes(d.sid)).map(d => d.name);
-    setVgDevices(selectedDevices);
-    setEditedDevices(true);
-    onValidate(devices.length > 0);
-  };
-
-  const onSubmit = (e) => {
-    e.preventDefault();
-    const customDevices = isBootDeviceSelected ? [] : vgDevices;
-    onSubmitProp(customDevices);
-  };
-
-  const BootDevice = () => {
-    const bootDevice = devices.find(d => d.name === settings.bootDevice);
-
-    // FIXME: In this case, should be a "readOnly" selector.
-    return <DeviceList devices={[bootDevice]} />;
-  };
-
-  return (
-    <Form id={id} onSubmit={onSubmit}>
-      <div className="split">
-        <span>{_("Devices for creating the volume group")}</span>
-        <ToggleGroup isCompact>
-          <ToggleGroupItem
-            text={_("Installation device")}
-            buttonId="bootDevice"
-            isSelected={isBootDeviceSelected}
-            onClick={selectBootDevice}
-          />
-          <ToggleGroupItem
-            text={_("Custom devices")}
-            buttonId="customDevices"
-            isSelected={!isBootDeviceSelected}
-            onClick={selectCustomDevices}
-          />
-        </ToggleGroup>
-      </div>
-      <If
-        condition={isBootDeviceSelected}
-        then={<BootDevice />}
-        else={
-          <DeviceSelector
-            isMultiple
-            selected={devices.filter(d => vgDevices?.includes(d.name))}
-            devices={devices}
-            onChange={onChangeDevices}
-          />
-        }
-      />
-    </Form>
-  );
-};
-
-/**
- * Allows to select LVM and configure the system volume group.
- * @component
- *
- * @param {object} props
- * @param {ProposalSettings} props.settings - Settings used for calculating a proposal.
- * @param {StorageDevice[]} [props.devices=[]] - Available storage devices.
- * @param {boolean} [props.isChecked=false] - Whether LVM is selected.
- * @param {boolean} [props.isLoading=false] - Whether to show the selector as loading.
- * @param {onChangeFn} [props.onChange=noop] - On change callback.
- *
- * @callback onChangeFn
- * @param {boolean} lvm
- */
-const LVMField = ({
-  settings,
-  devices = [],
-  isChecked: isCheckedProp = false,
-  isLoading = false,
-  onChange: onChangeProp = noop
-}) => {
-  const [isChecked, setIsChecked] = useState(isCheckedProp);
-  const [isFormOpen, setIsFormOpen] = useState(false);
-  const [isFormValid, setIsFormValid] = useState(true);
-
-  const onChange = (_, value) => {
-    setIsChecked(value);
-    onChangeProp({ lvm: value, vgDevices: [] });
-  };
-
-  const openForm = () => setIsFormOpen(true);
-
-  const closeForm = () => setIsFormOpen(false);
-
-  const onValidateForm = (valid) => setIsFormValid(valid);
-
-  const onSubmitForm = (vgDevices) => {
-    closeForm();
-    onChangeProp({ vgDevices });
-  };
-
-  const description = _("Configuration of the system volume group. All the file systems will be \
-created in a logical volume of the system volume group.");
-
-  const LVMSettingsButton = () => {
-    return (
-      <Tooltip
-        content={_("Configure the LVM settings")}
-        entryDelay={400}
-        exitDelay={50}
-        position="right"
-      >
-        <button aria-label={_("LVM settings")} className="plain-control" onClick={openForm}>
-          <Icon name="tune" size="s" />
-        </button>
-      </Tooltip>
-    );
-  };
-
-  if (isLoading) return <Skeleton width="25%" />;
-
-  return (
-    <div className="split">
-      <Switch
-        id="lvm"
-        label={_("Use logical volume management (LVM)")}
-        isReversed
-        isChecked={isChecked}
-        onChange={onChange}
-      />
-      <If condition={isChecked} then={<LVMSettingsButton />} />
-      <Popup
-        aria-label={_("LVM settings")}
-        title={_("System Volume Group")}
-        description={description}
-        isOpen={isFormOpen}
-      >
-        <LVMSettingsForm
-          id="lvmSettingsForm"
-          devices={devices}
-          settings={settings}
-          onSubmit={onSubmitForm}
-          onValidate={onValidateForm}
-        />
-        <Popup.Actions>
-          <Popup.Confirm
-            form="lvmSettingsForm"
-            type="submit"
-            isDisabled={!isFormValid}
-          >
-            {_("Accept")}
-          </Popup.Confirm>
-          <Popup.Cancel onClick={closeForm} />
-        </Popup.Actions>
-      </Popup>
-    </div>
-  );
-};
-
 /**
  * Form for configuring the encryption password.
  * @component
@@ -422,7 +222,6 @@ const EncryptionField = ({
  *
  * @param {object} props
  * @param {ProposalSettings} props.settings
- * @param {StorageDevice[]} [props.availableDevices=[]]
  * @param {String[]} [props.encryptionMethods=[]]
  * @param {onChangeFn} [props.onChange=noop]
  *
@@ -431,18 +230,9 @@ const EncryptionField = ({
  */
 export default function ProposalSettingsSection({
   settings,
-  availableDevices = [],
   encryptionMethods = [],
   onChange = noop
 }) {
-  const changeLVM = ({ lvm, vgDevices }) => {
-    const settings = {};
-    if (lvm !== undefined) settings.lvm = lvm;
-    if (vgDevices !== undefined) settings.systemVGDevices = vgDevices;
-
-    onChange(settings);
-  };
-
   const changeEncryption = ({ password, method }) => {
     onChange({ encryptionPassword: password, encryptionMethod: method });
   };
@@ -452,13 +242,6 @@ export default function ProposalSettingsSection({
   return (
     <>
       <Section title={_("Settings")}>
-        <LVMField
-          settings={settings}
-          devices={availableDevices}
-          isChecked={settings.lvm === true}
-          isLoading={settings.lvm === undefined}
-          onChange={changeLVM}
-        />
         <EncryptionField
           password={settings.encryptionPassword || ""}
           method={settings.encryptionMethod}
diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx
index 7d0a575de8..50de9dac00 100644
--- a/web/src/components/storage/ProposalSettingsSection.test.jsx
+++ b/web/src/components/storage/ProposalSettingsSection.test.jsx
@@ -35,223 +35,10 @@ jest.mock("@patternfly/react-core", () => {
 
 let props;
 
-const vda = {
-  sid: "59",
-  type: "disk",
-  vendor: "Micron",
-  model: "Micron 1100 SATA",
-  driver: ["ahci", "mmcblk"],
-  bus: "IDE",
-  transport: "usb",
-  dellBOSS: false,
-  sdCard: true,
-  active: true,
-  name: "/dev/vda",
-  size: 1024,
-  systems: ["Windows", "openSUSE Leap 15.2"],
-  udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"],
-  udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"],
-  partitionTable: { type: "gpt", partitions: [] }
-};
-
-const md0 = {
-  sid: "62",
-  type: "md",
-  level: "raid0",
-  uuid: "12345:abcde",
-  members: ["/dev/vdb"],
-  active: true,
-  name: "/dev/md0",
-  size: 2048,
-  systems: [],
-  udevIds: [],
-  udevPaths: []
-};
-
-const md1 = {
-  sid: "63",
-  type: "md",
-  level: "raid0",
-  uuid: "12345:abcde",
-  members: ["/dev/vdc"],
-  active: true,
-  name: "/dev/md1",
-  size: 4096,
-  systems: [],
-  udevIds: [],
-  udevPaths: []
-};
-
 beforeEach(() => {
   props = {};
 });
 
-describe("LVM field", () => {
-  describe("if LVM setting is not set yet", () => {
-    beforeEach(() => {
-      props.settings = {};
-    });
-
-    it("does not render the LVM switch", () => {
-      plainRender(<ProposalSettingsSection {...props} />);
-
-      expect(screen.queryByLabelText(/Use logical volume/)).toBeNull();
-    });
-  });
-
-  describe("if LVM setting is set", () => {
-    beforeEach(() => {
-      props.settings = { lvm: false };
-    });
-
-    it("renders the LVM switch", () => {
-      plainRender(<ProposalSettingsSection {...props} />);
-
-      screen.getByRole("checkbox", { name: /Use logical volume/ });
-    });
-  });
-
-  describe("if LVM is set to true", () => {
-    beforeEach(() => {
-      props.availableDevices = [vda, md0, md1];
-      props.settings = { bootDevice: "/dev/vda", lvm: true, systemVGDevices: [] };
-      props.onChange = jest.fn();
-    });
-
-    it("renders the LVM switch as selected", () => {
-      plainRender(<ProposalSettingsSection {...props} />);
-
-      const checkbox = screen.getByRole("checkbox", { name: /Use logical volume/ });
-      expect(checkbox).toBeChecked();
-    });
-
-    it("renders a button for changing the LVM settings", () => {
-      plainRender(<ProposalSettingsSection {...props} />);
-
-      screen.getByRole("button", { name: /LVM settings/ });
-    });
-
-    it("changes the selection on click", async () => {
-      const { user } = plainRender(<ProposalSettingsSection {...props} />);
-
-      const checkbox = screen.getByRole("checkbox", { name: /Use logical volume/ });
-      await user.click(checkbox);
-
-      expect(checkbox).not.toBeChecked();
-      expect(props.onChange).toHaveBeenCalled();
-    });
-
-    describe("and user clicks on LVM settings", () => {
-      it("opens the LVM settings dialog", async () => {
-        const { user } = plainRender(<ProposalSettingsSection {...props} />);
-        const settingsButton = screen.getByRole("button", { name: /LVM settings/ });
-
-        await user.click(settingsButton);
-
-        const popup = await screen.findByRole("dialog");
-        within(popup).getByText("System Volume Group");
-      });
-
-      it("allows selecting either installation device or custom devices", async () => {
-        const { user } = plainRender(<ProposalSettingsSection {...props} />);
-        const settingsButton = screen.getByRole("button", { name: /LVM settings/ });
-
-        await user.click(settingsButton);
-
-        const popup = await screen.findByRole("dialog");
-        screen.getByText("System Volume Group");
-
-        within(popup).getByRole("button", { name: "Installation device" });
-        within(popup).getByRole("button", { name: "Custom devices" });
-      });
-
-      it("allows to set the installation device as system volume group", async () => {
-        const { user } = plainRender(<ProposalSettingsSection {...props} />);
-        const settingsButton = screen.getByRole("button", { name: /LVM settings/ });
-
-        await user.click(settingsButton);
-
-        const popup = await screen.findByRole("dialog");
-        screen.getByText("System Volume Group");
-
-        const bootDeviceButton = within(popup).getByRole("button", { name: "Installation device" });
-        const customDevicesButton = within(popup).getByRole("button", { name: "Custom devices" });
-        const acceptButton = within(popup).getByRole("button", { name: "Accept" });
-
-        await user.click(customDevicesButton);
-        await user.click(bootDeviceButton);
-        await user.click(acceptButton);
-
-        expect(props.onChange).toHaveBeenCalledWith(
-          expect.objectContaining({ systemVGDevices: [] })
-        );
-      });
-
-      it("allows customize the system volume group", async () => {
-        const { user } = plainRender(<ProposalSettingsSection {...props} />);
-        const settingsButton = screen.getByRole("button", { name: /LVM settings/ });
-
-        await user.click(settingsButton);
-
-        const popup = await screen.findByRole("dialog");
-        screen.getByText("System Volume Group");
-
-        const customDevicesButton = within(popup).getByRole("button", { name: "Custom devices" });
-        const acceptButton = within(popup).getByRole("button", { name: "Accept" });
-
-        await user.click(customDevicesButton);
-
-        const vdaOption = within(popup).getByRole("row", { name: /vda/ });
-        const md0Option = within(popup).getByRole("row", { name: /md0/ });
-        const md1Option = within(popup).getByRole("row", { name: /md1/ });
-
-        // unselect the boot devices
-        await user.click(vdaOption);
-
-        await user.click(md0Option);
-        await user.click(md1Option);
-
-        await user.click(acceptButton);
-
-        expect(props.onChange).toHaveBeenCalledWith(
-          expect.objectContaining({ systemVGDevices: ["/dev/md0", "/dev/md1"] })
-        );
-      });
-    });
-  });
-
-  describe("if LVM is set to false", () => {
-    beforeEach(() => {
-      props.settings = { lvm: false };
-      props.onChange = jest.fn();
-    });
-
-    it("renders the LVM switch as not selected", () => {
-      plainRender(<ProposalSettingsSection {...props} />);
-
-      const checkbox = screen.getByRole("checkbox", { name: /Use logical volume/ });
-      expect(checkbox).not.toBeChecked();
-    });
-
-    it("does not render a button for changing the LVM settings", () => {
-      plainRender(<ProposalSettingsSection {...props} />);
-
-      const button = screen.queryByRole("button", { name: /LVM settings/ });
-      expect(button).toBeNull();
-    });
-
-    it("changes the selection on click", async () => {
-      const { user } = plainRender(<ProposalSettingsSection {...props} />);
-
-      const checkbox = screen.getByRole("checkbox", { name: /Use logical volume/ });
-      await user.click(checkbox);
-
-      expect(checkbox).toBeChecked();
-      expect(props.onChange).toHaveBeenCalled();
-    });
-  });
-});
-
 describe("Encryption field", () => {
   describe("if encryption password setting is not set yet", () => {
     beforeEach(() => {

From 0c3419b0b05ffc5b3a6603be1bae576b75fe8a0d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Thu, 22 Feb 2024 14:05:15 +0000
Subject: [PATCH 09/11] [web] Improve accessible texts used for skeletons

According to https://github.com/openSUSE/agama/pull/1045#discussion_r1498924560
---
 web/src/components/storage/ProposalDeviceSection.jsx      | 4 ++--
 web/src/components/storage/ProposalDeviceSection.test.jsx | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/web/src/components/storage/ProposalDeviceSection.jsx b/web/src/components/storage/ProposalDeviceSection.jsx
index 6b9ed801f5..1a066f4070 100644
--- a/web/src/components/storage/ProposalDeviceSection.jsx
+++ b/web/src/components/storage/ProposalDeviceSection.jsx
@@ -132,7 +132,7 @@ const InstallationDeviceField = ({
   };
 
   if (isLoading) {
-    return <Skeleton screenreaderText={_("Loading selected device")} width="25%" />;
+    return <Skeleton screenreaderText={_("Waiting for information about selected device")} width="25%" />;
   }
 
   const description = _("Select the device for installing the system.");
@@ -330,7 +330,7 @@ created in a logical volume of the system volume group.");
     );
   };
 
-  if (isLoading) return <Skeleton width="25%" />;
+  if (isLoading) return <Skeleton screenreaderText={_("Waiting for information about LVM")} width="25%" />;
 
   return (
     <div className="split">
diff --git a/web/src/components/storage/ProposalDeviceSection.test.jsx b/web/src/components/storage/ProposalDeviceSection.test.jsx
index 27bb058910..73cd460835 100644
--- a/web/src/components/storage/ProposalDeviceSection.test.jsx
+++ b/web/src/components/storage/ProposalDeviceSection.test.jsx
@@ -136,7 +136,7 @@ describe("ProposalDeviceSection", () => {
 
         it("renders a loading hint", () => {
           plainRender(<ProposalDeviceSection {...props} />);
-          screen.getByText("Loading selected device");
+          screen.getByText("Waiting for information about selected device");
         });
       });
     });

From eb8814a114a58a89663254d59b0e80f0167caee0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Thu, 22 Feb 2024 14:08:34 +0000
Subject: [PATCH 10/11] [web] Update changelog file

---
 web/package/cockpit-agama.changes | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes
index 30ae533f8b..a28b65bab7 100644
--- a/web/package/cockpit-agama.changes
+++ b/web/package/cockpit-agama.changes
@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Thu Feb 22 14:05:56 UTC 2024 - David Diaz <dgonzalez@suse.com>
+
+- Break storage settings in multiple sections to improve the UX
+  (gh#openSUSE/agama#1045).
+
 -------------------------------------------------------------------
 Wed Feb 21 17:40:01 UTC 2024 - David Diaz <dgonzalez@suse.com>
 

From 75bd8aa3bf2b43ad2b4fbc4b2c547f1c10a49c83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= <dgonzalez@suse.de>
Date: Thu, 22 Feb 2024 15:56:14 +0000
Subject: [PATCH 11/11] [web] Minor adjustments from code review

---
 web/src/components/storage/ProposalActionsSection.jsx     | 2 +-
 web/src/components/storage/ProposalDeviceSection.jsx      | 5 +++--
 web/src/components/storage/ProposalSpacePolicySection.jsx | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/web/src/components/storage/ProposalActionsSection.jsx b/web/src/components/storage/ProposalActionsSection.jsx
index 6310ad9fc1..8d35563afd 100644
--- a/web/src/components/storage/ProposalActionsSection.jsx
+++ b/web/src/components/storage/ProposalActionsSection.jsx
@@ -123,7 +123,7 @@ export default function ProposalActionsSection({ actions = [], errors = [], isLo
       // "delete partition A", "create partition B with filesystem C", ...
       title={_("Planned Actions")}
       // TRANSLATORS: The storage "Planned Actions" section's description
-      description={_("Actions to create the file systems and to ensure the new system boots")}
+      description={_("Actions to create the file systems and to ensure the new system boots.")}
       id="storage-actions"
       errors={errors}
     >
diff --git a/web/src/components/storage/ProposalDeviceSection.jsx b/web/src/components/storage/ProposalDeviceSection.jsx
index 1a066f4070..df37eaf304 100644
--- a/web/src/components/storage/ProposalDeviceSection.jsx
+++ b/web/src/components/storage/ProposalDeviceSection.jsx
@@ -47,7 +47,7 @@ import { noop } from "~/utils";
  *
  * @param {object} props
  * @param {string} props.id - Form ID.
- * @param {StorageDevice|undefined} [props.current] - Currently selected device, if any.
+ * @param {StorageDevice} [props.current] - Currently selected device, if any.
  * @param {StorageDevice[]} [props.devices=[]] - Available devices for the selection.
  * @param {onSubmitFn} [props.onSubmit=noop] - On submit callback.
  *
@@ -411,7 +411,8 @@ export default function ProposalDeviceSection({
       dangerouslySetInnerHTML={{
         // TRANSLATORS: The storage "Device" sections's description. Do not
         // translate 'abbr' and 'title', they are part of the HTML markup.
-        __html: _("Select the main disk or <abbr title='Logical Volume Manager'>LVM</abbr> Volume Group for installation")
+        __html: _("Select the main disk or <abbr title='Logical Volume Manager'>LVM</abbr> \
+Volume Group for installation.")
       }}
     />
   );
diff --git a/web/src/components/storage/ProposalSpacePolicySection.jsx b/web/src/components/storage/ProposalSpacePolicySection.jsx
index e5735412df..c08135c0e5 100644
--- a/web/src/components/storage/ProposalSpacePolicySection.jsx
+++ b/web/src/components/storage/ProposalSpacePolicySection.jsx
@@ -451,7 +451,8 @@ export default function ProposalSpacePolicySection({
       // TRANSLATORS: The storage "Find Space" section's title
       title={_("Find Space")}
       // TRANSLATORS: The storage "Find space" sections's description
-      description={_("Allocating the file systems might need to find free space in the devices listed below. Choose how to do it")}
+      description={_("Allocating the file systems might need to find free space \
+in the devices listed below. Choose how to do it.")}
     >
       <If
         condition={isLoading && settings.spacePolicy === undefined}