[web] Storage page: break settings into multiple sections (#1045)

Break the storage settings section to accomplish the organization
defined in the [storage
UI](https://github.com/openSUSE/agama/blob/master/doc/storage_ui.md)
document. Thus, proposed changes make the page to have _Device_,
_Settings_, and _File systems_ sections instead of only _Settings_.

Also, it includes a last minute change about moving the LVM setting to
the new _Device_ section, because it is more

> consistent with the future we want to go (in which LVM is part of the
device selection)
> 
>  — @ancorgs at #yast IRC channel

---

Related to https://trello.com/c/SK4uBRnw (internal link)

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>
 

web/src/assets/styles/blocks.scss

@@ -18,24 +18,36 @@
     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);
+      margin-block-end: var(--spacer-smaller);
     }
   }
 
-  > :not(h2) {
+  > :not(header) {
     grid-area: content;
     grid-column: content;
   }

web/src/assets/styles/layout.scss

@@ -20,7 +20,7 @@
     padding: var(--spacer-small);
   }
 
-  header {
+  > header {
     @extend .bottom-shadow;
     grid-area: header;
     display: flex;

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 {

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;

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,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|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.
@@ -63,6 +64,7 @@ import { ValidationErrors } from "~/components/core";
 export default function Section({
   icon,
   title,
+  description,
   name,
   path,
   loading,
@@ -84,9 +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 (
-      <h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
+      <header>
+        <h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
+        <If condition={renderDescription} then={<p>{description}</p>} />
+      </header>
     );
   };
 

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");

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 />}

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", () => {