diff --git a/web/src/components/core/Popup.jsx b/web/src/components/core/Popup.jsx index df0b49abe2..ff547d63dc 100644 --- a/web/src/components/core/Popup.jsx +++ b/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 & 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 ? : content} ); }; diff --git a/web/src/components/core/Popup.test.jsx b/web/src/components/core/Popup.test.jsx index 2184396eb8..db924b33ad 100644 --- a/web/src/components/core/Popup.test.jsx +++ b/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) => {

The Popup Content

@@ -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(); + + const dialog = await screen.findByRole("dialog"); + + expect(within(dialog).queryByText(loadingText)).toBeNull(); + }); + it("renders the popup actions inside a PF/Modal footer", async () => { installerRender(); @@ -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(); + + const dialog = await screen.findByRole("dialog"); + + expect(within(dialog).queryByText("The Popup Content")).toBeNull(); + within(dialog).getByText(loadingText); + }); + }); }); describe("Popup.PrimaryAction", () => { diff --git a/web/src/components/storage/BootConfigField.jsx b/web/src/components/storage/BootConfigField.jsx index b62ccf14fb..34f5b23422 100644 --- a/web/src/components/storage/BootConfigField.jsx +++ b/web/src/components/storage/BootConfigField.jsx @@ -87,8 +87,8 @@ export default function BootConfigField({ onChange({ configureBoot, bootDevice }); }; - if (isLoading) { - return ; + if (isLoading && configureBoot === undefined) { + return ; } let value; diff --git a/web/src/components/storage/DeviceSelectionDialog.jsx b/web/src/components/storage/DeviceSelectionDialog.jsx index 5efdeeb019..403f0a88e7 100644 --- a/web/src/components/storage/DeviceSelectionDialog.jsx +++ b/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(/[[\]]/); + diff --git a/web/src/components/storage/EncryptionField.jsx b/web/src/components/storage/EncryptionField.jsx index 208b2afcd6..0b2bbc7c40 100644 --- a/web/src/components/storage/EncryptionField.jsx +++ b/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} /> diff --git a/web/src/components/storage/EncryptionSettingsDialog.jsx b/web/src/components/storage/EncryptionSettingsDialog.jsx index fb37b4f598..f5b111456e 100644 --- a/web/src/components/storage/EncryptionSettingsDialog.jsx +++ b/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 ( - + ; + value = ; else value = targetValue(target, targetDevice, targetPVDevices); @@ -127,6 +127,7 @@ export default function InstallationDeviceField({ then={ { + const original = jest.requireActual("@patternfly/react-core"); + + return { + ...original, + Skeleton: () =>
PF-Skeleton
+ }; +}); + /** * @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(); - screen.getByText("Waiting for information about selected device"); + + // a PF skeleton is displayed + screen.getByText("PF-Skeleton"); }); }); diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index b2c4169566..16262c5fa6 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/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} /> { const ResultSkeleton = () => { return ( <> - + @@ -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 = [], diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 37bf0669ac..6e79c68633 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -26,6 +26,7 @@ import { _ } from "~/i18n"; import { compact } from "~/utils"; import { Section } from "~/components/core"; import { SPACE_POLICIES } from "~/components/storage/utils"; +import { CHANGING, NOT_AFFECTED } from "~/components/storage/ProposalPage"; import EncryptionField from "~/components/storage/EncryptionField"; import InstallationDeviceField from "~/components/storage/InstallationDeviceField"; import PartitionsField from "~/components/storage/PartitionsField"; @@ -40,6 +41,18 @@ import SpacePolicyField from "~/components/storage/SpacePolicyField"; * @typedef {import ("~/client/storage").Volume} Volume */ +/** + * A helper function to decide whether to show the progress skeletons or not + * for the specified component + * @param {boolean} loading loading status + * @param {string} component name of the component + * @param {symbol} changing the item which is being changed + * @returns {boolean} true if the skeleton should be displayed, false otherwise + */ +const showSkeleton = (loading, component, changing) => { + return loading && !NOT_AFFECTED[component].includes(changing); +}; + /** * Section for editing the proposal settings * @component @@ -50,7 +63,8 @@ import SpacePolicyField from "~/components/storage/SpacePolicyField"; * @property {String[]} encryptionMethods * @property {Volume[]} volumeTemplates * @property {boolean} [isLoading=false] - * @property {(settings: object) => void} onChange + * @property {symbol} [changing=undefined] which part of the configuration is being changed by user + * @property {(changing: symbol, settings: object) => void} onChange * * @param {ProposalSettingsSectionProps} props */ @@ -60,41 +74,51 @@ export default function ProposalSettingsSection({ encryptionMethods, volumeTemplates, isLoading = false, + changing = undefined, onChange }) { /** @param {import("~/components/storage/InstallationDeviceField").TargetConfig} targetConfig */ const changeTarget = ({ target, targetDevice, targetPVDevices }) => { - onChange({ - target, - targetDevice: targetDevice?.name, - targetPVDevices: targetPVDevices.map(d => d.name) - }); + onChange( + CHANGING.TARGET, + { + target, + targetDevice: targetDevice?.name, + targetPVDevices: targetPVDevices.map(d => d.name) + } + ); }; /** @param {import("~/components/storage/EncryptionField").EncryptionConfig} encryptionConfig */ const changeEncryption = ({ password, method }) => { - onChange({ encryptionPassword: password, encryptionMethod: method }); + onChange(CHANGING.ENCRYPTION, { encryptionPassword: password, encryptionMethod: method }); }; /** @param {Volume[]} volumes */ const changeVolumes = (volumes) => { - onChange({ volumes }); + onChange(CHANGING.VOLUMES, { volumes }); }; /** @param {import("~/components/storage/SpacePolicyField").SpacePolicyConfig} spacePolicyConfig */ const changeSpacePolicy = ({ spacePolicy, spaceActions }) => { - onChange({ - spacePolicy: spacePolicy.id, - spaceActions - }); + onChange( + CHANGING.POLICY, + { + spacePolicy: spacePolicy.id, + spaceActions + } + ); }; /** @param {import("~/components/storage/PartitionsField").BootConfig} bootConfig */ const changeBoot = ({ configureBoot, bootDevice }) => { - onChange({ - configureBoot, - bootDevice: bootDevice?.name - }); + onChange( + CHANGING.BOOT, + { + configureBoot, + bootDevice: bootDevice?.name + } + ); }; /** @@ -120,7 +144,7 @@ export default function ProposalSettingsSection({ targetDevice={targetDevice} targetPVDevices={targetPVDevices} devices={availableDevices} - isLoading={isLoading} + isLoading={showSkeleton(isLoading, "InstallationDeviceField", changing)} onChange={changeTarget} /> @@ -147,7 +171,7 @@ export default function ProposalSettingsSection({ policy={spacePolicy} actions={spaceActions} devices={installationDevices} - isLoading={isLoading} + isLoading={showSkeleton(isLoading, "SpacePolicyField", changing)} onChange={changeSpacePolicy} /> diff --git a/web/src/components/storage/SpacePolicyDialog.jsx b/web/src/components/storage/SpacePolicyDialog.jsx index e0f5162d04..77c1753214 100644 --- a/web/src/components/storage/SpacePolicyDialog.jsx +++ b/web/src/components/storage/SpacePolicyDialog.jsx @@ -73,6 +73,7 @@ const SpacePolicyPicker = ({ currentPolicy, onChange = noop }) => { * @param {SpaceAction[]} props.actions * @param {StorageDevice[]} props.devices * @param {boolean} [props.isOpen=false] + * @param {boolean} [props.isLoading] * @param {() => void} [props.onCancel=noop] * @param {(spaceConfig: SpaceConfig) => void} [props.onAccept=noop] * @@ -85,6 +86,7 @@ export default function SpacePolicyDialog({ actions: defaultActions, devices, isOpen, + isLoading, onCancel = noop, onAccept = noop, ...props @@ -94,8 +96,11 @@ export default function SpacePolicyDialog({ const [customUsed, setCustomUsed] = useState(false); const [expandedDevices, setExpandedDevices] = useState([]); + if (!policy && defaultPolicy) { setPolicy(defaultPolicy) } + if (!actions && defaultActions) { setActions(defaultActions) } + useEffect(() => { - if (policy.id === "custom") setExpandedDevices(devices); + if (policy && policy.id === "custom") setExpandedDevices(devices); }, [devices, policy, setExpandedDevices]); // The selectors for the space action have to be initialized always to the same value @@ -104,12 +109,12 @@ export default function SpacePolicyDialog({ // Stores whether the custom policy has been used. useEffect(() => { - if (policy.id === "custom" && !customUsed) setCustomUsed(true); + if (policy && policy.id === "custom" && !customUsed) setCustomUsed(true); }, [policy, customUsed, setCustomUsed]); // Resets actions (i.e., sets everything to "keep") if the custom policy has not been used yet. useEffect(() => { - if (policy.id !== "custom" && !customUsed) setActions([]); + if (policy && policy.id !== "custom" && !customUsed) setActions([]); }, [policy, customUsed, setActions]); // Generates the action value according to the policy. @@ -142,6 +147,7 @@ in the devices listed below. Choose how to do it."); title={_("Find space")} description={description} isOpen={isOpen} + isLoading={isLoading} blockSize="large" inlineSize="large" {...props} @@ -155,7 +161,7 @@ in the devices listed below. Choose how to do it."); devices={devices} expandedDevices={expandedDevices} deviceAction={deviceAction} - isActionDisabled={policy.id !== "custom"} + isActionDisabled={policy?.id !== "custom"} onActionChange={changeActions} /> } diff --git a/web/src/components/storage/SpacePolicyField.jsx b/web/src/components/storage/SpacePolicyField.jsx index 7858df0fc7..db1cb0d4ff 100644 --- a/web/src/components/storage/SpacePolicyField.jsx +++ b/web/src/components/storage/SpacePolicyField.jsx @@ -72,7 +72,7 @@ export default function SpacePolicyField({ let value; if (isLoading || !policy) { - value = ; + value = ; } else if (policy.summaryLabels.length === 1) { // eslint-disable-next-line agama-i18n/string-literals value = _(policy.summaryLabels[0]); @@ -95,6 +95,7 @@ in the installation device(s).")} then={