Storage: show skeletons only when needed

web/src/components/core/Popup.jsx

@@ -25,6 +25,7 @@ import React from "react";
 import { Button, Modal } from "@patternfly/react-core";
 import { _ } from "~/i18n";
 import { partition } from "~/utils";
+import { Loading } from "~/components/layout";
 
 /**
  * @typedef {import("@patternfly/react-core").ModalProps} ModalProps
@@ -190,12 +191,17 @@ const AncillaryAction = ({ children, ...actionsProps }) => (
  * @typedef {object} PopupBaseProps
  * @property {"auto" | "small" | "medium" | "large"} [blockSize="auto"] - The block/height size for the dialog. Default is "auto".
  * @property {"auto" | "small" | "medium" | "large"} [inlineSize="medium"] - The inline/width size for the dialog. Default is "medium".
+ * @property {boolean} [isLoading=false] - Whether the data is loading, if yes it displays a loading indicator instead of the requested content
+ * @property {string} [loadingText="Loading data..."] - Text displayed when `isLoading` is set to `true`
  * @typedef {Omit<ModalProps, "variant" | "size"> & PopupBaseProps} PopupProps
  *
  * @param {PopupProps} props
  */
 const Popup = ({
   isOpen = false,
+  isLoading = false,
+  // TRANSLATORS: progress message
+  loadingText = _("Loading data..."),
   showClose = false,
   inlineSize = "medium",
   blockSize = "auto",
@@ -214,7 +220,7 @@ const Popup = ({
       actions={actions}
       className={`${className} block-size-${blockSize} inline-size-${inlineSize}`.trim()}
     >
-      {content}
+      {isLoading ? <Loading text={loadingText} /> : content}
     </Modal>
   );
 };

web/src/components/core/Popup.test.jsx

@@ -27,8 +27,10 @@ import { installerRender } from "~/test-utils";
 import { Popup } from "~/components/core";
 
 let isOpen;
+let isLoading;
 const confirmFn = jest.fn();
 const cancelFn = jest.fn();
+const loadingText = "Loading text";
 
 const TestingPopup = (props) => {
   const [isMounted, setIsMounted] = useState(true);
@@ -39,6 +41,8 @@ const TestingPopup = (props) => {
     <Popup
       title="Testing Popup component"
       isOpen={isOpen}
+      isLoading={isLoading}
+      loadingText={loadingText}
       {...props}
     >
       <p>The Popup Content</p>
@@ -55,6 +59,7 @@ describe("Popup", () => {
   describe("when it is not open", () => {
     beforeEach(() => {
       isOpen = false;
+      isLoading = false;
     });
 
     it("renders nothing", async () => {
@@ -65,9 +70,10 @@ describe("Popup", () => {
     });
   });
 
-  describe("when it is open", () => {
+  describe("when it is open and not loading", () => {
     beforeEach(() => {
       isOpen = true;
+      isLoading = false;
     });
 
     it("renders the popup content inside a PF/Modal", async () => {
@@ -79,6 +85,14 @@ describe("Popup", () => {
       within(dialog).getByText("The Popup Content");
     });
 
+    it("does not display a progress message", async () => {
+      installerRender(<TestingPopup />);
+
+      const dialog = await screen.findByRole("dialog");
+
+      expect(within(dialog).queryByText(loadingText)).toBeNull();
+    });
+
     it("renders the popup actions inside a PF/Modal footer", async () => {
       installerRender(<TestingPopup />);
 
@@ -92,6 +106,22 @@ describe("Popup", () => {
       within(footer).getByText("Cancel");
     });
   });
+
+  describe("when it is open and loading", () => {
+    beforeEach(() => {
+      isOpen = true;
+      isLoading = true;
+    });
+
+    it("displays progress message instead of the content", async () => {
+      installerRender(<TestingPopup />);
+
+      const dialog = await screen.findByRole("dialog");
+
+      expect(within(dialog).queryByText("The Popup Content")).toBeNull();
+      within(dialog).getByText(loadingText);
+    });
+  });
 });
 
 describe("Popup.PrimaryAction", () => {

web/src/components/storage/BootConfigField.jsx

@@ -87,8 +87,8 @@ export default function BootConfigField({
     onChange({ configureBoot, bootDevice });
   };
 
-  if (isLoading) {
-    return <Skeleton screenreaderText={_("Waiting for information about boot config")} width="75%" />;
+  if (isLoading && configureBoot === undefined) {
+    return <Skeleton width="75%" />;
   }
 
   let value;

web/src/components/storage/DeviceSelectionDialog.jsx

@@ -52,6 +52,7 @@ const OPTIONS_NAME = "selection-mode";
  * @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 {boolean} [props.isLoading=false] - Whether loading the data is in progress
  * @param {() => void} [props.onCancel=noop]
  * @param {(target: Target) => void} [props.onAccept=noop]
  *
@@ -67,6 +68,7 @@ export default function DeviceSelectionDialog({
   targetPVDevices: defaultPVDevices,
   devices,
   isOpen,
+  isLoading,
   onCancel = noop,
   onAccept = noop,
   ...props
@@ -95,6 +97,11 @@ export default function DeviceSelectionDialog({
     return true;
   };
 
+  // change the initial `undefined` state when receiving the real data
+  if (!target && defaultTarget) { setTarget(defaultTarget) }
+  if (!targetDevice && defaultTargetDevice) { setTargetDevice(defaultTargetDevice) }
+  if (!targetPVDevices && defaultPVDevices) { setTargetPVDevices(defaultPVDevices) }
+
   const isDeviceSelectable = (device) => device.isDrive || device.type === "md";
 
   // TRANSLATORS: description for using plain partitions for installing the
@@ -114,6 +121,7 @@ devices.").split(/[[\]]/);
     <Popup
       title={_("Device for installing the system")}
       isOpen={isOpen}
+      isLoading={isLoading}
       blockSize="large"
       inlineSize="large"
       {...props}
@@ -175,6 +183,7 @@ devices.").split(/[[\]]/);
           </Panels.Panel>
         </Panels>
       </Form>
+
       <Popup.Actions>
         <Popup.Confirm form="target-form" type="submit" isDisabled={isAcceptDisabled()} />
         <Popup.Cancel onClick={onCancel} />

web/src/components/storage/EncryptionField.jsx

@@ -106,6 +106,7 @@ export default function EncryptionField({
             method={method}
             methods={methods}
             isOpen={isDialogOpen}
+            isLoading={isLoading}
             onCancel={closeDialog}
             onAccept={onAccept}
           />

web/src/components/storage/EncryptionSettingsDialog.jsx

@@ -53,6 +53,7 @@ directly on its first run.");
  * @property {string} method - Encryption method.
  * @property {string[]} methods - Possible encryption methods.
  * @property {boolean} [isOpen=false] - Whether the dialog is visible or not.
+ * @property {boolean} [isLoading=false] - Whether the data is loading
  * @property {() => void} onCancel - Callback to trigger when on cancel action.
  * @property {(settings: EncryptionSetting) => void} onAccept - Callback to trigger on accept action.
  *
@@ -63,6 +64,7 @@ export default function EncryptionSettingsDialog({
   method: methodProp,
   methods,
   isOpen = false,
+  isLoading = false,
   onCancel,
   onAccept
 }) {
@@ -71,8 +73,21 @@ export default function EncryptionSettingsDialog({
   const [method, setMethod] = useState(methodProp);
   const [passwordsMatch, setPasswordsMatch] = useState(true);
   const [validSettings, setValidSettings] = useState(true);
+  const [wasLoading, setWasLoading] = useState(isLoading);
   const formId = "encryptionSettingsForm";
 
+  // reset the settings only after loading is finished
+  if (isLoading && !wasLoading) { setWasLoading(true) }
+  if (!isLoading && wasLoading) {
+    setWasLoading(false);
+    // refresh the state when the real values are loaded
+    if (method !== methodProp) { setMethod(methodProp) }
+    if (password !== passwordProp) {
+      setPassword(passwordProp);
+      setIsEnabled(passwordProp?.length > 0);
+    }
+  }
+
   useEffect(() => {
     setValidSettings(!isEnabled || (password.length > 0 && passwordsMatch));
   }, [isEnabled, password, passwordsMatch]);
@@ -91,7 +106,7 @@ export default function EncryptionSettingsDialog({
   };
 
   return (
-    <Popup title={DIALOG_TITLE} description={DIALOG_DESCRIPTION} isOpen={isOpen}>
+    <Popup title={DIALOG_TITLE} description={DIALOG_DESCRIPTION} isOpen={isOpen} isLoading={isLoading}>
       <SwitchField
         highlightContent
         isChecked={isEnabled}

web/src/components/storage/InstallationDeviceField.jsx

@@ -109,7 +109,7 @@ export default function InstallationDeviceField({
 
   let value;
   if (isLoading || !target)
-    value = <Skeleton screenreaderText={_("Waiting for information about selected device")} width="25%" />;
+    value = <Skeleton width="25%" />;
   else
     value = targetValue(target, targetDevice, targetPVDevices);
 
@@ -127,6 +127,7 @@ export default function InstallationDeviceField({
         then={
           <DeviceSelectionDialog
             isOpen
+            isLoading={isLoading}
             target={target}
             targetDevice={targetDevice}
             targetPVDevices={targetPVDevices}

web/src/components/storage/InstallationDeviceField.test.jsx

@@ -26,6 +26,15 @@ import { screen, within } from "@testing-library/react";
 import { installerRender } from "~/test-utils";
 import InstallationDeviceField from "~/components/storage/InstallationDeviceField";
 
+jest.mock("@patternfly/react-core", () => {
+  const original = jest.requireActual("@patternfly/react-core");
+
+  return {
+    ...original,
+    Skeleton: () => <div>PF-Skeleton</div>
+  };
+});
+
 /**
  * @typedef {import ("~/components/storage/InstallationDeviceField").InstallationDeviceFieldProps} InstallationDeviceFieldProps
  * @typedef {import ("~/client/storage").StorageDevice} StorageDevice
@@ -98,7 +107,9 @@ describe("when set as loading", () => {
 
   it("renders a loading hint", () => {
     installerRender(<InstallationDeviceField {...props} />);
-    screen.getByText("Waiting for information about selected device");
+
+    // a PF skeleton is displayed
+    screen.getByText("PF-Skeleton");
   });
 });
 

web/src/components/storage/ProposalPage.jsx

@@ -35,6 +35,8 @@ import { IDLE } from "~/client/status";
 
 const initialState = {
   loading: true,
+  // which UI item is being changed by user
+  changing: undefined,
   availableDevices: [],
   volumeTemplates: [],
   encryptionMethods: [],
@@ -52,7 +54,8 @@ const reducer = (state, action) => {
     }
 
     case "STOP_LOADING" : {
-      return { ...state, loading: false };
+      // reset the changing value after the refresh is finished
+      return { ...state, loading: false, changing: undefined };
     }
 
     case "UPDATE_AVAILABLE_DEVICES": {
@@ -76,8 +79,8 @@ const reducer = (state, action) => {
     }
 
     case "UPDATE_SETTINGS": {
-      const { settings } = action.payload;
-      return { ...state, settings };
+      const { settings, changing } = action.payload;
+      return { ...state, settings, changing };
     }
 
     case "UPDATE_DEVICES": {
@@ -96,6 +99,30 @@ const reducer = (state, action) => {
   }
 };
 
+/**
+ * Which UI item is being changed by user
+ */
+export const CHANGING = Object.freeze({
+  ENCRYPTION: Symbol("encryption"),
+  TARGET: Symbol("target"),
+  VOLUMES: Symbol("volumes"),
+  POLICY: Symbol("policy"),
+  BOOT: Symbol("boot"),
+});
+
+// mapping of not affected values for settings components
+// key:   component name
+// value: list of items which can be changed without affecting
+//        the state of the component
+export const NOT_AFFECTED = {
+  // the EncryptionField shows the skeleton only during initial load,
+  // it does not depend on any changed item and does not show skeleton later.
+  // the ProposalResultSection is refreshed always
+  InstallationDeviceField: [CHANGING.ENCRYPTION, CHANGING.BOOT, CHANGING.POLICY, CHANGING.VOLUMES],
+  PartitionsField: [CHANGING.ENCRYPTION, CHANGING.POLICY],
+  SpacePolicyField: [CHANGING.ENCRYPTION, CHANGING.TARGET],
+};
+
 export default function ProposalPage() {
   const { storage: client } = useInstallerClient();
   const { cancellablePromise } = useCancellablePromise();
@@ -208,10 +235,10 @@ export default function ProposalPage() {
     }
   }, [client, load, state.settings]);
 
-  const changeSettings = async (settings) => {
+  const changeSettings = async (changing, settings) => {
     const newSettings = { ...state.settings, ...settings };
 
-    dispatch({ type: "UPDATE_SETTINGS", payload: { settings: newSettings } });
+    dispatch({ type: "UPDATE_SETTINGS", payload: { settings: newSettings, changing } });
     calculate(newSettings).catch(console.error);
   };
 
@@ -236,6 +263,7 @@ export default function ProposalPage() {
         settings={state.settings}
         onChange={changeSettings}
         isLoading={state.loading}
+        changing={state.changing}
       />
       <ProposalResultSection
         system={state.system}

web/src/components/storage/ProposalResultSection.jsx

@@ -207,7 +207,7 @@ const DevicesTreeTable = ({ devicesManager }) => {
 const ResultSkeleton = () => {
   return (
     <>
-      <Skeleton width="80%" />
+      <Skeleton screenreaderText={_("Waiting for information about storage configuration")} width="80%" />
       <Skeleton width="65%" />
       <Skeleton width="70%" />
     </>
@@ -251,6 +251,7 @@ const SectionContent = ({ system, staging, actions, errors }) => {
  * @param {Action[]} [props.actions=[]]
  * @param {ValidationError[]} [props.errors=[]] - Validation errors
  * @param {boolean} [props.isLoading=false] - Whether the section content should be rendered as loading
+ * @param {symbol} [props.changing=undefined] - Which part of the configuration is being changed by user
  */
 export default function ProposalResultSection({
   system = [],