web: Improve find space (#1128)

Improvements in the dialog to find space: * Includes the unused space. * Show devices and slots sorted by position. * The colums *Device* and *Details show similar information to the Result table. * Remove *Size details* in favor of *Shrinkable* column. Code improvements: * Remove `ProposalSpacePolicyField` in favor of `SpacePolicyDialog` component, following the same apprach that the dialogs for boot and device configuration. * Enable type checking in several components. * Add more options to `TreeTable` (expandable rows, `expadedItems` props). ## Screenshots ![localhost_8080_ (34)](https://github.com/openSUSE/agama/assets/1112304/646848d9-1915-4527-914e-27fbf4b78c95)
agama-project · Apr 8, 2024 · 05ea8cd · 05ea8cd
2 parents 2fb60f8 + 816ae4d
commit 05ea8cd
Show file tree

Hide file tree

Showing 20 changed files with 1,033 additions and 1,010 deletions.
diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes
@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Mon Apr  8 14:17:45 UTC 2024 - José Iván López González <jlopez@suse.com>
+
+- Improve the representation of the devices in the dialog for
+  finding space (gh#openSUSE/agama#1128).
+
 -------------------------------------------------------------------
 Thu Apr  4 15:59:37 UTC 2024 - Ancor Gonzalez Sosa <ancor@suse.com>
 

diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss
@@ -526,9 +526,7 @@ table[data-type="agama/tree-table"] {
   }
 }
 
-// FIXME: use a single selector
-table.devices-table,
-table.proposal-result {
+table.devices-table {
   tr.dimmed-row {
     background-color: #fff;
     opacity: 0.8;
@@ -538,8 +536,11 @@ table.proposal-result {
       color: var(--color-gray-dimmed);
       padding-block: 0;
     }
-
   }
+}
+
+table.proposal-result {
+  @extend .devices-table;
 
   /**
    * Temporary hack because the collapse/expand callback was not given to the

diff --git a/web/src/components/core/OptionsPicker.jsx b/web/src/components/core/OptionsPicker.jsx
@@ -19,8 +19,12 @@
  * find current contact information at www.suse.com.
  */
 
+// @ts-check
+
 import React from "react";
 
+import { noop } from "~/utils";
+
 /**
  * Wrapper for OptionsPicker options
  * @component
@@ -29,12 +33,14 @@ import React from "react";
  * @param {string} [props.title] - Text to be used as option title
  * @param {string} [props.body] - Text to be used as option body
  * @param {boolean} [props.isSelected=false] - Whether the option should be set as select of not
+ * @param {() => void} [props.onClick=noop]
  * @param {object} [props.props] - Other props sent to div#option node
  */
-const Option = ({ title, body, isSelected = false, ...props }) => {
+const Option = ({ title, body, isSelected = false, onClick = noop, ...props }) => {
   return (
     <div
       {...props}
+      onClick={onClick}
       role="option"
       aria-selected={isSelected}
     >
@@ -50,10 +56,10 @@ const Option = ({ title, body, isSelected = false, ...props }) => {
  *
  * @param {object} props
  * @param {string} [props.ariaLabel] - Text to be used as accessible label
- * @param {Array<Option>} props.children - A collection of Option
+ * @param {React.ReactNode} props.children - A collection of Option
  * @param {object} [props.props] - Other props sent to div#listbox node
  */
-const OptionsPicker = ({ "aria-label": ariaLabel, children, ...props }) => {
+const OptionsPicker = ({ ariaLabel, children, ...props }) => {
   return (
     <div
       {...props}

diff --git a/web/src/components/core/Page.test.jsx b/web/src/components/core/Page.test.jsx
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2023] SUSE LLC
+ * Copyright (c) [2023-2024] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -23,12 +23,35 @@ import React from "react";
 import { screen, within } from "@testing-library/react";
 import { installerRender, plainRender, mockNavigateFn } from "~/test-utils";
 import { Page } from "~/components/core";
+import { createClient } from "~/client";
+
+jest.mock("~/client");
+
+const l10nClientMock = {
+  getUILocale: jest.fn().mockResolvedValue("en_US"),
+  keymaps: jest.fn().mockResolvedValue([]),
+  getKeymap: jest.fn().mockResolvedValue(undefined),
+  timezones: jest.fn().mockResolvedValue([]),
+  getTimezone: jest.fn().mockResolvedValue(undefined),
+  onLocalesChange: jest.fn(),
+  onKeymapChange: jest.fn(),
+  onTimezoneChange: jest.fn()
+};
 
 describe("Page", () => {
   beforeAll(() => {
     jest.spyOn(console, "error").mockImplementation();
   });
 
+  beforeEach(() => {
+    // if defined outside, the mock is cleared automatically
+    createClient.mockImplementation(() => {
+      return {
+        l10n: l10nClientMock
+      };
+    });
+  });
+
   afterAll(() => {
     console.error.mockRestore();
   });

diff --git a/web/src/components/core/PasswordAndConfirmationInput.jsx b/web/src/components/core/PasswordAndConfirmationInput.jsx
@@ -24,7 +24,7 @@ import { FormGroup } from "@patternfly/react-core";
 import { FormValidationError, PasswordInput } from "~/components/core";
 import { _ } from "~/i18n";
 
-const PasswordAndConfirmationInput = ({ value, onChange, onValidation, isDisabled }) => {
+const PasswordAndConfirmationInput = ({ value, onChange, onValidation, isDisabled = false }) => {
   const [confirmation, setConfirmation] = useState(value || "");
   const [error, setError] = useState("");
 

diff --git a/web/src/components/core/Popup.jsx b/web/src/components/core/Popup.jsx
@@ -19,12 +19,19 @@
  * find current contact information at www.suse.com.
  */
 
+// @ts-check
+
 import React from "react";
 import { Button, Modal } from "@patternfly/react-core";
-
 import { _ } from "~/i18n";
 import { partition } from "~/utils";
 
+/**
+ * @typedef {import("@patternfly/react-core").ModalProps} ModalProps
+ * @typedef {import("@patternfly/react-core").ButtonProps} ButtonProps
+ * @typedef {Omit<ButtonProps, 'variant'>} ButtonWithoutVariantProps
+ */
+
 /**
  * Wrapper component for holding Popup actions
  *
@@ -33,6 +40,7 @@ import { partition } from "~/utils";
  *
  * @see Popup examples.
  *
+ * @param {object} props
  * @param {React.ReactNode} [props.children] - a collection of Action components
  */
 const Actions = ({ children }) => <>{children}</>;
@@ -42,13 +50,10 @@ const Actions = ({ children }) => <>{children}</>;
  *
  * Built on top of {@link https://www.patternfly.org/components/button PF/Button}
  *
- * @see Popup examples.
- *
- * @param {React.ReactNode} props.children - content of the action
- * @param {object} [props] - PF/Button props, see {@link https://www.patternfly.org/components/button#props}
+ * @param {ButtonProps} props
  */
-const Action = ({ children, ...props }) => (
-  <Button { ...props }>
+const Action = ({ children, ...buttonProps }) => (
+  <Button { ...buttonProps }>
     {children}
   </Button>
 );
@@ -68,11 +73,10 @@ const Action = ({ children, ...props }) => (
  *     <Text>Upload</Text>
  *   </PrimaryAction>
  *
- * @param {React.ReactNode} props.children - content of the action
- * @param {object} [props] - {@link Action} props
+ * @param {ButtonWithoutVariantProps} props
  */
-const PrimaryAction = ({ children, ...props }) => (
-  <Action { ...props } variant="primary">{ children }</Action>
+const PrimaryAction = ({ children, ...actionProps }) => (
+  <Action { ...actionProps } variant="primary">{ children }</Action>
 );
 
 /**
@@ -84,11 +88,10 @@ const PrimaryAction = ({ children, ...props }) => (
  * @example <caption>Using it with a custom text</caption>
  *   <Confirm onClick={accept}>Accept</Confirm>
  *
- * @param {React.ReactNode} [props.children="confirm"] - content of the action
- * @param {object} [props] - {@link Action} props
+ * @param {ButtonWithoutVariantProps} props
  */
-const Confirm = ({ children = _("Confirm"), ...props }) => (
-  <PrimaryAction key="confirm" { ...props }>{ children }</PrimaryAction>
+const Confirm = ({ children = _("Confirm"), ...actionProps }) => (
+  <PrimaryAction key="confirm" { ...actionProps }>{ children }</PrimaryAction>
 );
 
 /**
@@ -106,11 +109,10 @@ const Confirm = ({ children = _("Confirm"), ...props }) => (
  *     <Text>Dismiss</Text>
  *   </SecondaryAction>
  *
- * @param {React.ReactNode} props.children - content of the action
- * @param {object} [props] - {@link Action} props
+ * @param {ButtonWithoutVariantProps} props
  */
-const SecondaryAction = ({ children, ...props }) => (
-  <Action { ...props } variant="secondary">{ children }</Action>
+const SecondaryAction = ({ children, ...actionProps }) => (
+  <Action { ...actionProps } variant="secondary">{ children }</Action>
 );
 
 /**
@@ -122,11 +124,10 @@ const SecondaryAction = ({ children, ...props }) => (
  * @example <caption>Using it with a custom text</caption>
  *   <Cancel onClick={dismiss}>Dismiss</Confirm>
  *
- * @param {React.ReactNode} [props.children="Cancel"] - content of the action
- * @param {object} [props] - {@link Action} props
+ * @param {ButtonWithoutVariantProps} props
  */
-const Cancel = ({ children = _("Cancel"), ...props }) => (
-  <SecondaryAction key="cancel" { ...props }>{ children }</SecondaryAction>
+const Cancel = ({ children = _("Cancel"), ...actionProps }) => (
+  <SecondaryAction key="cancel" { ...actionProps }>{ children }</SecondaryAction>
 );
 
 /**
@@ -144,11 +145,10 @@ const Cancel = ({ children = _("Cancel"), ...props }) => (
  *     <Text>Do not set</Text>
  *   </AncillaryAction>
  *
- * @param {React.ReactNode} props.children - content of the action
- * @param {object} [props] - {@link Action} props
+ * @param {ButtonWithoutVariantProps} props
  */
-const AncillaryAction = ({ children, ...props }) => (
-  <Action { ...props } variant="link">{ children }</Action>
+const AncillaryAction = ({ children, ...actionsProps }) => (
+  <Action { ...actionsProps } variant="link">{ children }</Action>
 );
 
 /**
@@ -187,13 +187,7 @@ const AncillaryAction = ({ children, ...props }) => (
  *     </Popup.Actions>
  *   </Popup>
  *
- * @param {object} props - component props
- * @param {boolean} [props.isOpen=false] - whether the popup is displayed or not
- * @param {boolean} [props.showClose=false] - whether the popup should include a "X" action for closing it
- * @param {string} [props.variant="small"] - the popup size, based on PF/Modal `variant` prop
- * @param {React.ReactNode} props.children - the popup content and actions
- * @param {object} [pfModalProps] - PF/Modal props, See {@link https://www.patternfly.org/components/modal#props}
- *
+ * @param {ModalProps} props
  */
 const Popup = ({
   isOpen = false,
@@ -205,6 +199,7 @@ const Popup = ({
   const [actions, content] = partition(React.Children.toArray(children), child => child.type === Actions);
 
   return (
+    /** @ts-ignore */
     <Modal
       { ...pfModalProps }
       isOpen={isOpen}

diff --git a/web/src/components/core/Section.jsx b/web/src/components/core/Section.jsx
@@ -56,7 +56,7 @@ import { If, ValidationErrors } from "~/components/core";
  * @property {string} [className] - Class name for section html tag.
  * @property {string} [id] - Id of the section ("software", "product", "storage", "storage-actions", ...)
  * @property {import("~/client/mixins").ValidationError[]} [props.errors] - Validation errors to be shown before the title.
- * @property {React.ReactElement} [children] - The section content.
+ * @property {React.ReactNode} [children] - The section content.
  * @property {string} [aria-label] - aria-label attribute, required if title if not given
  *
  * @param { SectionProps } props

diff --git a/web/src/components/core/TreeTable.jsx b/web/src/components/core/TreeTable.jsx
@@ -21,7 +21,7 @@
 
 // @ts-check
 
-import React from "react";
+import React, { useEffect, useState } from "react";
 import { Table, Thead, Tr, Th, Tbody, Td, TreeRowWrapper } from '@patternfly/react-table';
 
 /**
@@ -39,6 +39,7 @@ import { Table, Thead, Tr, Th, Tbody, Td, TreeRowWrapper } from '@patternfly/rea
  * @typedef {object} TreeTableBaseProps
  * @property {TreeTableColumn[]} columns=[]
  * @property {object[]} items=[]
+ * @property {object[]} [expandedItems=[]]
  * @property {(any) => array} [itemChildren]
  * @property {(any) => string} [rowClassNames]
  */
@@ -58,9 +59,26 @@ export default function TreeTable({
   columns = [],
   items = [],
   itemChildren = () => [],
+  expandedItems = [],
   rowClassNames = () => "",
   ...tableProps
 }) {
+  const [expanded, setExpanded] = useState(expandedItems);
+
+  useEffect(() => {
+    setExpanded(expandedItems);
+  }, [expandedItems, setExpanded]);
+
+  const isExpanded = (item) => expanded.includes(item);
+
+  const toggle = (item) => {
+    if (isExpanded(item)) {
+      setExpanded(expanded.filter(d => d !== item));
+    } else {
+      setExpanded([...expanded, item]);
+    }
+  };
+
   const renderColumns = (item, treeRow) => {
     return columns.map((c, cIdx) => {
       const props = {
@@ -76,17 +94,20 @@ export default function TreeTable({
     });
   };
 
-  const renderRows = (items, level) => {
+  const renderRows = (items, level, hidden = false) => {
     if (items?.length <= 0) return;
 
     return (
       items.map((item, itemIdx) => {
         const children = itemChildren(item);
+        const expanded = isExpanded(item);
 
         const treeRow = {
+          onCollapse: () => toggle(item),
           props: {
-            isExpanded: true,
+            isExpanded: expanded,
             isDetailsExpanded: true,
+            isHidden: hidden,
             "aria-level": level,
             "aria-posinset": itemIdx + 1,
             "aria-setsize": children?.length || 0
@@ -101,7 +122,7 @@ export default function TreeTable({
         return (
           <React.Fragment key={itemIdx}>
             <TreeRowWrapper {...rowProps}>{renderColumns(item, treeRow)}</TreeRowWrapper>
-            { renderRows(children, level + 1)}
+            { renderRows(children, level + 1, !expanded)}
           </React.Fragment>
         );
       })

diff --git a/web/src/components/storage/BootSelectionDialog.jsx b/web/src/components/storage/BootSelectionDialog.jsx
@@ -28,6 +28,8 @@ import { Popup } from "~/components/core";
 import { deviceLabel } from "~/components/storage/utils";
 import { sprintf } from "sprintf-js";
 
+// @ts-check
+
 /**
  * @typedef {import ("~/client/storage").StorageDevice} StorageDevice
  */

diff --git a/web/src/components/storage/DeviceSelectionDialog.jsx b/web/src/components/storage/DeviceSelectionDialog.jsx
@@ -19,6 +19,8 @@
  * find current contact information at www.suse.com.
  */
 
+// @ts-check
+
 import React, { useState } from "react";
 import { Form } from "@patternfly/react-core";
 
@@ -53,7 +55,7 @@ const Html = ({ children, ...props }) => (
  * @param {StorageDevice[]} props.targetPVDevices
  * @param {StorageDevice[]} props.devices - The actions to perform in the system.
  * @param {boolean} [props.isOpen=false] - Whether the dialog is visible or not.
- * @param {function} [props.onCancel=noop]
+ * @param {() => void} [props.onCancel=noop]
  * @param {(target: Target) => void} [props.onAccept=noop]
  *
  * @typedef {object} Target