Space policy UI

.gitignore

@@ -1,2 +1,3 @@
 .log/
 .yardoc/
+.vscode/tasks.json

web/package/cockpit-agama.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Thu Nov 30 15:19:38 UTC 2023 - José Iván López González <jlopez@suse.com>
+
+- Allow selecting the storage policy to make free space
+  (gh#openSUSE/agama#883).
+
 -------------------------------------------------------------------
 Wed Nov 29 14:15:16 UTC 2023 - José Iván López González <jlopez@suse.com>
 

web/src/assets/styles/blocks.scss

@@ -272,81 +272,110 @@ span.notification-mark[data-variant="sidebar"] {
   }
 }
 
-.item-list {
-  [role="option"] {
-    padding: var(--spacer-normal);
+ul[data-type="agama/list"] {
+  li {
     border: 2px solid var(--color-gray-dark);
-    background: var(--color-gray-light);
-
     border-radius: 5px;
-    transition: all 0.2s ease-in-out;
-    display: grid;
-
-    gap: var(--spacer-small);
-
-    grid-template-columns: 1fr;
-    grid-template-areas: "content";
+    padding: var(--spacer-normal);
     text-align: start;
+    background: var(--color-gray-light);
+    margin-block-end: var(--spacer-small);
+  }
 
-    > [data-type="details"] {
-      font-size: 80%;
+  // FIXME: see if it's semantically correct to mark an li as aria-selected when
+  // not belongs to a listbox or grid list ul.
+  li[aria-selected] {
+    border: 2px solid var(--color-primary);
+    box-shadow: 0 2px 5px 0 var(--color-gray-dark);
+    background: var(--color-primary);
+    color: white;
+    font-weight: 700;
+
+    svg {
+      fill: white;
     }
+  }
+}
+
+// These attributes together means that UI is rendering a selector
+ul[data-type="agama/list"][role="listbox"] {
+  li[role="option"] {
+    transition: all 0.2s ease-in-out;
 
     &:hover {
       &:not([aria-selected]) {
         background: var(--color-gray-dark);
       }
     }
+  }
+}
 
-    &:is([data-type="locale"]) {
-      grid-template-columns: 1fr 2fr;
-      grid-template-areas:
-        "language territory"
-        "details details";
+// Each kind of list/selector has its way of laying out their items
+ul[data-of="agama/storage-devices"] {
+  li {
+    display: grid;
+    gap: var(--spacer-small);
+    grid-template-columns: 1fr 2fr 2fr;
+    grid-template-areas: "type-and-size drive-info drive-content";
 
-      > [data-type="details"] {
-        grid-area: details;
-      }
+    svg {
+      vertical-align: inherit;
     }
 
-    &:is([data-type="timezone"]) {
-      grid-template-columns: 1fr 2fr 1fr;
-      grid-template-areas:
-        "part1 rest-parts time"
-        "details details .";
+    > :first-child {
+      align-self: center;
+      text-align: center;
+      justify-self: start;
+    }
+  }
+}
 
-      > [data-type="details"] {
-        grid-area: details;
-      }
+ul[data-of="agama/space-policies"] {
+  li *:not(:last-child) {
+    margin-block-end: var(--spacer-small);
+  }
+}
 
-      > [data-type="time"] {
-        color: var(--color-gray-dimmed);
-        grid-area: time;
-        text-align: end;
-      }
-    }
+ul[data-of="agama/locales"] {
+  li {
+    display: grid;
+    grid-template-columns: 1fr 2fr;
+    grid-template-areas:
+      "language territory"
+      "details details";
 
-    &:is([data-type="storage-device"]) {
-      grid-template-columns: 1fr 2fr 2fr;
-      grid-template-areas: "type-and-size drive-info drive-content";
+    > [data-type="details"] {
+      grid-area: details;
+      font-size: 80%;
+    }
+  }
+}
 
-      > [data-type="type-and-size"] {
-        align-self: center;
-        text-align: center;
-        justify-self: start;
-      }
+ul[data-of="agama/keymaps"] {
+  li {
+    > [data-type="details"] {
+      font-size: 80%;
     }
   }
+}
 
-  [aria-selected] {
-    border: 2px solid var(--color-primary);
-    box-shadow: 0 2px 5px 0 var(--color-gray-dark);
-    background: var(--color-primary);
-    color: white;
-    font-weight: 700;
+ul[data-of="agama/timezones"] {
+  li {
+    display: grid;
+    grid-template-columns: 1fr 2fr 1fr;
+    grid-template-areas:
+      "part1 rest-parts time"
+      "details details .";
 
-    svg {
-      fill: white;
+    > [data-type="details"] {
+      grid-area: details;
+      font-size: 80%;
+    }
+
+    > [data-type="time"] {
+      grid-area: time;
+      color: var(--color-gray-dimmed);
+      text-align: end;
     }
   }
 }

web/src/assets/styles/global.scss

@@ -56,6 +56,16 @@ th {
   text-align: start;
 }
 
+li {
+  svg {
+    vertical-align: middle;
+  }
+
+  span {
+    margin-inline-start: var(--spacer-small);
+  }
+}
+
 // Style focus making use of :focus-visible
 *:focus {
   outline: none;

web/src/assets/styles/patternfly-overrides.scss

@@ -190,3 +190,38 @@ table td > .pf-v5-c-empty-state {
   --pf-v5-c-toggle-group__button--m-selected--BackgroundColor: var(--color-primary);
   --pf-v5-c-toggle-group__button--Color: var(--color-gray-light);
 }
+
+// Reduce padding of PF/Hint because it looks like an option of current Agama
+// select
+.pf-v5-c-hint {
+  --pf-v5-c-hint--PaddingTop: var(--spacer-small);
+  --pf-v5-c-hint--PaddingRight: var(--spacer-small);
+  --pf-v5-c-hint--PaddingBottom: var(--spacer-small);
+  --pf-v5-c-hint--PaddingLeft: var(--spacer-small);
+}
+
+// Do not reserve space for PF/Hint actions when there are none
+.pf-v5-c-hint__actions:empty {
+  display: none;
+}
+
+// Make PF/ExpandableSection looks a bit different when wrapped in a PF/Hint
+.pf-v5-c-hint {
+  .pf-v5-c-expandable-section {
+    --pf-v5-c-expandable-section__toggle--Color: var(--color-primary);
+  }
+
+  .pf-v5-c-expandable-section__toggle,
+  .pf-v5-c-expandable-section__toggle:hover {
+    // NOTE. would be nice to being able to use darker variant of primary color
+    // when hovering the link, but we aren't ready yet. We should switch to hsla
+    // colors or so.
+    --pf-v5-c-expandable-section__toggle--Color: var(--color-primary);
+    text-decoration: underline;
+  }
+
+  .pf-v5-c-expandable-section__content {
+    --pf-v5-c-expandable-section__content--PaddingRight: var(--spacer-normal);
+    --pf-v5-c-expandable-section__content--PaddingLeft: var(--spacer-normal);
+  }
+}

web/src/assets/styles/utilities.scss

@@ -25,6 +25,11 @@
   text-align: center;
 }
 
+.title {
+  font-size: var(--fs-large);
+  font-weight: var(--fw-bold);
+}
+
 .bold {
   font-weight: bold;
 }

web/src/assets/styles/variables.scss

@@ -9,6 +9,7 @@
   --fw-bold: 700;
 
   --fs-base: 14px;
+  --fs-large: 1rem;
   --fs-h1: 1.5rem;
   --fs-h2: 1.2rem;
 

web/src/client/storage.js

@@ -228,8 +228,10 @@ class ProposalManager {
    * @property {string} bootDevice
    * @property {string} encryptionPassword
    * @property {boolean} lvm
+   * @property {string} spacePolicy
    * @property {string[]} systemVGDevices
    * @property {Volume[]} volumes
+   * @property {StorageDevice[]} installationDevices
    *
    * @typedef {object} Volume
    * @property {string} mountPath
@@ -311,6 +313,8 @@ class ProposalManager {
 
     if (!proxy) return undefined;
 
+    const systemDevices = await this.system.getDevices();
+
     const buildResult = (proxy) => {
       const buildAction = dbusAction => {
         return {
@@ -320,13 +324,32 @@ class ProposalManager {
         };
       };
 
+      const buildInstallationDevices = (proxy, devices) => {
+        const findDevice = (devices, name) => {
+          const device = devices.find(d => d.name === name);
+
+          if (device === undefined) console.log("D-Bus object not found: ", name);
+
+          return device;
+        };
+
+        const names = proxy.SystemVGDevices.filter(n => n !== proxy.BootDevice).concat([proxy.BootDevice]);
+        return names.map(dev => findDevice(devices, dev)).filter(Boolean);
+      };
+
       return {
         settings: {
           bootDevice: proxy.BootDevice,
           lvm: proxy.LVM,
+          spacePolicy: proxy.SpacePolicy,
           systemVGDevices: proxy.SystemVGDevices,
           encryptionPassword: proxy.EncryptionPassword,
           volumes: proxy.Volumes.map(this.buildVolume),
+          // NOTE: strictly speaking, installation devices does not belong to the settings. It
+          // should be a separate method instead of an attribute in the settings object.
+          // Nevertheless, it was added here for simplicity and to avoid passing more props in some
+          // react components. Please, do not use settings as a jumble.
+          installationDevices: buildInstallationDevices(proxy, systemDevices)
         },
         actions: proxy.Actions.map(buildAction)
       };
@@ -341,7 +364,7 @@ class ProposalManager {
    * @param {ProposalSettings} settings
    * @returns {Promise<number>} 0 on success, 1 on failure
    */
-  async calculate({ bootDevice, encryptionPassword, lvm, systemVGDevices, volumes }) {
+  async calculate({ bootDevice, encryptionPassword, lvm, spacePolicy, systemVGDevices, volumes }) {
     const dbusVolume = (volume) => {
       return removeUndefinedCockpitProperties({
         MountPath: { t: "s", v: volume.mountPath },
@@ -358,6 +381,7 @@ class ProposalManager {
       BootDevice: { t: "s", v: bootDevice },
       EncryptionPassword: { t: "s", v: encryptionPassword },
       LVM: { t: "b", v: lvm },
+      SpacePolicy: { t: "s", v: spacePolicy },
       SystemVGDevices: { t: "as", v: systemVGDevices },
       Volumes: { t: "aa{sv}", v: volumes?.map(dbusVolume) }
     });

web/src/client/storage.test.js

@@ -155,6 +155,7 @@ const contexts = {
       LVM: true,
       SystemVGDevices: ["/dev/sda", "/dev/sdb"],
       EncryptionPassword: "00000",
+      SpacePolicy: "delete",
       Volumes: [
         {
           MountPath: { t: "s", v: "/" },
@@ -811,18 +812,20 @@ describe("#proposal", () => {
 
     describe("if there is a proposal", () => {
       beforeEach(() => {
+        contexts.withSystemDevices();
         contexts.withProposal();
         client = new StorageClient();
       });
 
       it("returns the proposal settings and actions", async () => {
         const { settings, actions } = await client.proposal.getResult();
 
-        expect(settings).toStrictEqual({
+        expect(settings).toMatchObject({
           bootDevice: "/dev/sda",
           lvm: true,
           systemVGDevices: ["/dev/sda", "/dev/sdb"],
           encryptionPassword: "00000",
+          spacePolicy: "delete",
           volumes: [
             {
               mountPath: "/",
@@ -861,6 +864,10 @@ describe("#proposal", () => {
           ]
         });
 
+        expect(settings.installationDevices.map(d => d.name).sort()).toStrictEqual(
+          ["/dev/sda", "/dev/sdb"].sort()
+        );
+
         expect(actions).toStrictEqual([
           { text: "Mount /dev/sdb1 as root", subvol: false, delete: false }
         ]);

web/src/components/l10n/KeymapSelector.jsx

@@ -29,7 +29,11 @@ import { noop } from "~/utils";
  * @typedef {import ("~/clients/l10n").Keymap} Keymap
  */
 
-const ListBox = ({ children, ...props }) => <ul role="listbox" {...props}>{children}</ul>;
+const ListBox = ({ children, ...props }) => {
+  return (
+    <ul data-type="agama/list" data-of="agama/keymaps" {...props}>{children}</ul>
+  );
+};
 
 const ListBoxItem = ({ isSelected, children, onClick, ...props }) => {
   if (isSelected) props['aria-selected'] = true;
@@ -82,7 +86,7 @@ export default function KeymapSelector({ value, keymaps = [], onChange = noop })
       <div className="sticky-top-0">
         <ListSearch placeholder={helpSearch} elements={keymaps} onChange={setFilteredKeymaps} />
       </div>
-      <ListBox aria-label={_("Available keymaps")} className="stack item-list">
+      <ListBox aria-label={_("Available keymaps")} role="listbox">
         { filteredKeymaps.map((keymap, index) => (
           <ListBoxItem
             key={`keymap-${index}`}