Merge pull request #491 from KKoukiou/spur-ux-fixups-v2

Super picky UI review fixups round #2

src/components/review/StorageReview.scss

@@ -17,8 +17,10 @@ ul.storage-review-device-tree {
 
 td.storage-review-table-column-action-destroy {
   color: var(--pf-v5-global--danger-color--100);
+  font-weight: bold;
 }
 
 td.storage-review-table-column-action-resize {
   color: var(--pf-v5-global--warning-color--100);
+  font-weight: bold;
 }

src/components/storage/HelpAutopartOptions.jsx

@@ -2,7 +2,7 @@ import cockpit from "cockpit";
 
 const _ = cockpit.gettext;
 
-export const helpEraseAll = _("Remove all partitions on the selected devices, including existing operating systems. Make sure you have backed up your data.");
+export const helpEraseAll = _("Remove all partitions on the selected devices, including existing operating systems.");
 
 export const helpUseFreeSpace = _("Keep current disk layout and use available space, to dual-boot with another OS.");
 

src/components/storage/InstallationScenario.jsx

@@ -126,8 +126,8 @@ const checkMountPointMapping = ({ mountPointConstraints, selectedDisks, usablePa
             .filter(device => device.formatData.mountable.v || device.formatData.type.v === "luks").length > 0;
 
     if (!hasFilesystems) {
-        availability.available = false;
-        availability.reason = _("No usable devices on the selected disks.");
+        // No usable devices on the selected disks: hide the scenario to reduce UI clutter
+        availability.hidden = true;
     } else if (missingNMParts.length) {
         availability.available = false;
         availability.reason = cockpit.format(_("Some required partitions are missing: $0"), missingNMParts.join(", "));

src/components/storage/ReclaimSpaceModal.jsx

@@ -40,7 +40,7 @@ import { CompressArrowsAltIcon, HddIcon, LockIcon, TrashIcon, UndoIcon } from "@
 
 import { isDeviceShrinkable, removeDevice, shrinkDevice } from "../../apis/storage_partitioning_automatic_resizable.js";
 
-import { getDeviceAncestors, unitMultiplier } from "../../helpers/storage.js";
+import { getDeviceAncestors, getDeviceTypeInfo, unitMultiplier } from "../../helpers/storage.js";
 
 import { ModalError } from "cockpit-components-inline-notification.jsx";
 import { ListingTable } from "cockpit-components-table.jsx";
@@ -240,10 +240,10 @@ const getDeviceRow = (disk, devices, level = 0, unappliedActions, setUnappliedAc
     const isPartition = device.type.v === "partition";
     const typeLabel = device.attrs?.v["partition-type-name"] || "";
     const diskDescription = (
-        <Flex spaceItems={{ default: "spaceItemsSm" }} alignItems={{ default: "alignItemsCenter" }} flexWrap={{ default: "nowrap" }}>
-            <FlexItem><HddIcon /></FlexItem>
-            <FlexItem>{cockpit.format("$0 ($1)", device.name.v, device.description.v)}</FlexItem>
-        </Flex>
+        <>
+            <HddIcon />
+            {cockpit.format("$0 ($1)", device.name.v, device.description.v)}
+        </>
     );
     const classNames = [
         idPrefix + "-table-row",
@@ -260,7 +260,7 @@ const getDeviceRow = (disk, devices, level = 0, unappliedActions, setUnappliedAc
         }
     }
 
-    let deviceType = isDisk ? device.type.v : device.formatData.type.v;
+    let deviceType = getDeviceTypeInfo(device);
     if (isDeviceLocked({ device })) {
         deviceType = (
             <Flex
@@ -342,7 +342,7 @@ const DeviceActions = ({ device, level, setUnappliedActions, unappliedActions })
     };
 
     return (
-        <Flex spaceItems={{ default: "spaceItemsXs" }}>
+        <Flex spaceItems={{ default: "spaceItemsXs" }} className="reclaim-actions">
             <DeviceActionShrink {...deviceActionProps} />
             <DeviceActionDelete {...deviceActionProps} />
             {hasUnappliedActions && <Button variant="plain" icon={<UndoIcon />} onClick={onUndo} aria-label={_("undo")} />}

src/components/storage/ReclaimSpaceModal.scss

@@ -1,18 +1,11 @@
+@import "global-variables";
+
 #reclaim-space-modal-table {
   tr.reclaim-space-modal-table-row.reclaim-space-modal-device-level-1,
   tr.reclaim-space-modal-table-row.reclaim-space-modal-device-level-2,
   tr.reclaim-space-modal-table-row.reclaim-space-modal-device-level-3 {
     td {
-      padding-top: var(--pf-v5-global--spacer--sm);
-      padding-bottom: var(--pf-v5-global--spacer--sm);
-    }
-  }
-
-  @for $i from 1 through 10 {
-    tr.reclaim-space-modal-table-row.reclaim-space-modal-device-level-#{$i} {
-      td:nth-child(2) {
-        padding-inline-start: calc(var(--pf-v5-c-table--cell--PaddingLeft) + #{$i - 1} * var(--pf-v5-global--spacer--md));
-      }
+      padding-block: var(--pf-v5-global--spacer--sm);
     }
   }
 
@@ -33,6 +26,95 @@
     font-size: var(--pf-v5-global--FontSize--sm);
     color: var(--pf-v5-global--warning-color--100);
   }
+
+  .pf-v5-c-table__td {
+    padding-block: var(--pf-v5-global--spacer--sm);
+    align-items: center;
+  }
+
+  .reclaim-actions {
+    // Actions should not have a gap
+    gap: 0;
+    // Actions should "eat into" TD padding so they're aligned appropriately
+    margin: calc(-1 * var(--pf-v5-global--spacer--xs)) calc(-1 * var(--pf-v5-global--spacer--sm));
+
+    // Fix the button size of actions
+    .pf-v5-c-button {
+      padding: var(--pf-v5-global--spacer--xs) var(--pf-v5-global--spacer--sm);
+    }
+  }
+
+  // Bind indent level variable to classes (start indenting on level 2)
+  @for $i from 1 through 10 {
+    .reclaim-space-modal-device-level-#{$i} {
+      --indent: #{$i - 1};
+    }
+  }
+
+  // Small sizes
+  @media (width < $pf-v5-global--breakpoint--md) {
+    // Indent the left for smaller sizes
+    .reclaim-space-modal-table-row:not(.reclaim-space-modal-device-level-0) {
+      margin-inline-start: calc(
+        var(--indent) * var(--pf-v5-global--spacer--sm)
+      );
+    }
+
+    // Remove labels with no content on small sizes
+    td:empty {
+      display: none;
+    }
+
+    tr {
+      grid-template-columns: auto 1fr;
+    }
+
+    td {
+      grid-template-columns: subgrid;
+      grid-column: 1 / -1;
+    }
+  }
+
+  // Large sizes
+  @media (width >= $pf-v5-global--breakpoint--md) {
+    // Indent the left for larger sizes
+    .reclaim-space-modal-table-row:not(.reclaim-space-modal-device-level-0) td:nth-child(2) {
+      padding-inline-start: calc(var(--pf-v5-c-table--cell--PaddingLeft) + var(--indent) * var(--pf-v5-global--spacer--md));
+    }
+
+    // Align columns to the right on larger screens
+    :is(th, td):nth-last-of-type(-n + 2) {
+      text-align: end;
+    }
+
+    // Also align the actions group to the right
+    .reclaim-actions {
+      justify-content: end;
+    }
+
+    // Set the disk icon size
+    td:first-of-type {
+      --icon-size: 1.5em;
+    }
+
+    // Adjust the disk size and make it align properly
+    .reclaim-space-modal-device-level-0 {
+      td:first-of-type .pf-v5-svg {
+        height: auto;
+        width: var(--icon-size);
+        margin-inline-end: var(--pf-v5-global--spacer--sm);
+        vertical-align: middle;
+      }
+    }
+
+    // Align non-icon text with the text that has an icon
+    .reclaim-space-modal-table-row:not(.reclaim-space-modal-device-level-0) {
+      td:first-of-type {
+        padding-left: calc(var(--icon-size) + var(--pf-v5-global--spacer--sm) + var(--pf-v5-c-table--cell--PaddingLeft));
+      }
+    }
+
+  }
 }
 
 // Make scrolling happen in the list instead of the dialog

src/helpers/storage.js

@@ -151,6 +151,16 @@ export const checkDeviceOnStorageType = (deviceData, device, type) => {
     return deviceData[device].parents.v?.some(parent => checkDeviceOnStorageType(deviceData, parent, type));
 };
 
+export const getDeviceTypeInfo = (device) => {
+    const deviceType = device.type.v;
+    const deviceFormatType = device.formatData.type.v;
+
+    if (deviceType === "partition") {
+        return deviceFormatType;
+    }
+    return deviceType;
+};
+
 /* Match the units to their respective sizes */
 export const unitMultiplier = {
     B: 1,