Skip to content

Commit

Permalink
Storage: show skeletons only when needed
Browse files Browse the repository at this point in the history 
Also display progress for loading the installation device
  • Loading branch information
@lslezak
lslezak committed Apr 26, 2024
1 parent 493706b commit 837a880
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 85 deletions.
130 changes: 72 additions & 58 deletions web/src/components/storage/DeviceSelectionDialog.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { Form } from "@patternfly/react-core";

import { _ } from "~/i18n";
import { deviceChildren } from "~/components/storage/utils";
import { ControlledPanels as Panels, Popup } from "~/components/core";
import { ControlledPanels as Panels, If, Popup, SectionSkeleton } from "~/components/core";
import { DeviceSelectorTable } from "~/components/storage";
import { noop } from "~/utils";

Expand All @@ -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]
*
Expand All @@ -67,6 +68,7 @@ export default function DeviceSelectionDialog({
targetPVDevices: defaultPVDevices,
devices,
isOpen,
isLoading,
onCancel = noop,
onAccept = noop,
...props
Expand Down Expand Up @@ -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
Expand All @@ -118,63 +125,70 @@ devices.").split(/[[\]]/);
inlineSize="large"
{...props}
>
<Form id="target-form" onSubmit={onSubmit}>
<Panels className="stack">
<Panels.Options data-variant="buttons">
<Panels.Option
id={SELECT_DISK_ID}
name={OPTIONS_NAME}
isSelected={isTargetDisk}
onChange={selectTargetDisk}
controls={SELECT_DISK_PANEL_ID}
>
{_("Select a disk")}
</Panels.Option>
<Panels.Option
id={CREATE_LVM_ID}
name={OPTIONS_NAME}
isSelected={isTargetNewLvmVg}
onChange={selectTargetNewLvmVG}
controls={CREATE_LVM_PANEL_ID}
>
{_("Create an LVM Volume Group")}
</Panels.Option>
</Panels.Options>

<Panels.Panel id={SELECT_DISK_PANEL_ID} isExpanded={isTargetDisk}>
{msgStart1}
<b>{msgBold1}</b>
{msgEnd1}

<DeviceSelectorTable
aria-label={_("Device selector for target disk")}
devices={devices}
selected={[targetDevice]}
itemChildren={deviceChildren}
itemSelectable={isDeviceSelectable}
onSelectionChange={selectTargetDevice}
variant="compact"
/>
</Panels.Panel>

<Panels.Panel id={CREATE_LVM_PANEL_ID} isExpanded={isTargetNewLvmVg} className="stack">
{msgStart2}
<b>{msgBold2}</b>
{msgEnd2}

<DeviceSelectorTable
aria-label={_("Device selector for new LVM volume group")}
isMultiple
devices={devices}
selected={targetPVDevices}
itemChildren={deviceChildren}
itemSelectable={isDeviceSelectable}
onSelectionChange={setTargetPVDevices}
variant="compact"
/>
</Panels.Panel>
</Panels>
</Form>
<If
condition={isLoading}
then={ <SectionSkeleton numRows={6} /> }
else={
<Form id="target-form" onSubmit={onSubmit}>
<Panels className="stack">
<Panels.Options data-variant="buttons">
<Panels.Option
id={SELECT_DISK_ID}
name={OPTIONS_NAME}
isSelected={isTargetDisk}
onChange={selectTargetDisk}
controls={SELECT_DISK_PANEL_ID}
>
{_("Select a disk")}
</Panels.Option>
<Panels.Option
id={CREATE_LVM_ID}
name={OPTIONS_NAME}
isSelected={isTargetNewLvmVg}
onChange={selectTargetNewLvmVG}
controls={CREATE_LVM_PANEL_ID}
>
{_("Create an LVM Volume Group")}
</Panels.Option>
</Panels.Options>

<Panels.Panel id={SELECT_DISK_PANEL_ID} isExpanded={isTargetDisk}>
{msgStart1}
<b>{msgBold1}</b>
{msgEnd1}

<DeviceSelectorTable
aria-label={_("Device selector for target disk")}
devices={devices}
selected={[targetDevice]}
itemChildren={deviceChildren}
itemSelectable={isDeviceSelectable}
onSelectionChange={selectTargetDevice}
variant="compact"
/>
</Panels.Panel>

<Panels.Panel id={CREATE_LVM_PANEL_ID} isExpanded={isTargetNewLvmVg} className="stack">
{msgStart2}
<b>{msgBold2}</b>
{msgEnd2}

<DeviceSelectorTable
aria-label={_("Device selector for new LVM volume group")}
isMultiple
devices={devices}
selected={targetPVDevices}
itemChildren={deviceChildren}
itemSelectable={isDeviceSelectable}
onSelectionChange={setTargetPVDevices}
variant="compact"
/>
</Panels.Panel>
</Panels>
</Form>
}
/>

<Popup.Actions>
<Popup.Confirm form="target-form" type="submit" isDisabled={isAcceptDisabled()} />
<Popup.Cancel onClick={onCancel} />
Expand Down
1 change: 1 addition & 0 deletions web/src/components/storage/InstallationDeviceField.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export default function InstallationDeviceField({
then={
<DeviceSelectionDialog
isOpen
isLoading={isLoading}
target={target}
targetDevice={targetDevice}
targetPVDevices={targetPVDevices}
Expand Down
39 changes: 34 additions & 5 deletions web/src/components/storage/ProposalPage.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand All @@ -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": {
Expand All @@ -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": {
Expand All @@ -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
InstallationDeviceField: [CHANGING.ENCRYPTION, CHANGING.BOOT, CHANGING.POLICY, CHANGING.VOLUMES],
PartitionsField: [CHANGING.ENCRYPTION],
SpacePolicyField: [CHANGING.ENCRYPTION, CHANGING.BOOT, CHANGING.VOLUMES, CHANGING.TARGET],
ProposalResultSection: [CHANGING.ENCRYPTION],
};

export default function ProposalPage() {
const { storage: client } = useInstallerClient();
const { cancellablePromise } = useCancellablePromise();
Expand Down Expand Up @@ -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);
};

Expand All @@ -236,13 +263,15 @@ export default function ProposalPage() {
settings={state.settings}
onChange={changeSettings}
isLoading={state.loading}
changing={state.changing}
/>
<ProposalResultSection
system={state.system}
staging={state.staging}
actions={state.actions}
errors={state.errors}
isLoading={state.loading}
changing={state.changing}
/>
</Page>
);
Expand Down
16 changes: 13 additions & 3 deletions web/src/components/storage/ProposalResultSection.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { Button, Skeleton } from "@patternfly/react-core";
import { sprintf } from "sprintf-js";
import { _, n_ } from "~/i18n";
import { deviceChildren, deviceSize } from "~/components/storage/utils";
import { NOT_AFFECTED } from "~/components/storage/ProposalPage";
import DevicesManager from "~/components/storage/DevicesManager";
import { If, Section, Reminder, Tag, TreeTable } from "~/components/core";
import { ProposalActionsDialog, FilesystemLabel } from "~/components/storage";
Expand All @@ -36,6 +37,13 @@ import { ProposalActionsDialog, FilesystemLabel } from "~/components/storage";
* @typedef {import("~/client/mixins").ValidationError} ValidationError
*/

/**
* A helper function to decide whether to show the progress skeletons or not
* @param {boolean} loading
* @param {symbol} changing the item which is being changed
*/
const ShowSkeleton = (loading, changing) => loading && !NOT_AFFECTED.ProposalResultSection.includes(changing);

/**
* Renders information about planned actions, allowing to check all of them and warning with a
* summary about the deletion ones, if any.
Expand Down Expand Up @@ -251,13 +259,15 @@ 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 = [],
staging = [],
actions = [],
errors = [],
isLoading = false
isLoading = false,
changing = undefined
}) {
if (isLoading) errors = [];
const totalActions = actions.length;
Expand All @@ -274,12 +284,12 @@ export default function ProposalResultSection({
<Section
// TRANSLATORS: The storage "Result" section's title
title={_("Result")}
description={!isLoading && errors.length === 0 && description}
description={!ShowSkeleton(isLoading, changing) && errors.length === 0 && description}
id="storage-result"
errors={errors}
>
<If
condition={isLoading}
condition={ShowSkeleton(isLoading, changing)}
then={<ResultSkeleton />}
else={
<SectionContent
Expand Down

0 comments on commit 837a880

Please sign in to comment.