From 302b335bf5e3bd9cb2821480bc0afb3b7403dde1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 8 Apr 2024 22:16:14 +0100 Subject: [PATCH 01/31] web: Define types for icon name and size --- web/src/components/core/Section.jsx | 8 ++++++-- web/src/components/layout/Icon.jsx | 11 ++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/web/src/components/core/Section.jsx b/web/src/components/core/Section.jsx index 831d5f792d..c1e80cd63b 100644 --- a/web/src/components/core/Section.jsx +++ b/web/src/components/core/Section.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022-2023] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -26,6 +26,10 @@ import { Link } from "react-router-dom"; import { Icon } from '~/components/layout'; import { If, ValidationErrors } from "~/components/core"; +/** + * @typedef {import("~/components/layout/Icon").IconName} IconName + */ + /** * Renders children into an HTML section * @component @@ -46,7 +50,7 @@ import { If, ValidationErrors } from "~/components/core"; * * * @typedef { Object } SectionProps - * @property {string} [icon] - Name of the section icon. Not rendered if title is not provided. + * @property {IconName} [icon] - Name of the section icon. Not rendered if title is not provided. * @property {string} [title] - The section title. If not given, aria-label must be provided. * @property {string|React.ReactElement} [description] - A section description. Use only if really needed. * @property {string} [name] - The section name. Used to build the header id. diff --git a/web/src/components/layout/Icon.jsx b/web/src/components/layout/Icon.jsx index a14e646a66..f32e8cef90 100644 --- a/web/src/components/layout/Icon.jsx +++ b/web/src/components/layout/Icon.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2022-2023] SUSE LLC + * Copyright (c) [2022-2024] SUSE LLC * * All Rights Reserved. * @@ -79,6 +79,11 @@ import { SiLinux, SiWindows } from "@icons-pack/react-simple-icons"; // Icons from SVG import Loading from "./three-dots-loader-icon.svg?component"; +/** + * @typedef {string|number} IconSize + * @typedef {keyof icons} IconName + */ + const icons = { add_a_photo: AddAPhoto, apps: Apps, @@ -149,9 +154,9 @@ const PREDEFINED_SIZES = [ * * * @param {object} props - Component props - * @param {string} props.name - Name of the desired icon. + * @param {IconName} props.name - Name of the desired icon. * @param {string} [props.className=""] - CSS classes. - * @param {string|number} [props.size] - Size used for both, width and height. + * @param {IconSize} [props.size] - Size used for both, width and height. * It can be a CSS unit or one of PREDEFINED_SIZES. * @param {object} [props.otherProps] Other props sent to SVG icon. Please, note * that width and height will be overwritten by the size value if it was given. From d7fdb3297956f1ef548e2ad5ebe32d97160bd5e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 8 Apr 2024 23:23:06 +0100 Subject: [PATCH 02/31] web: Add initial version of core/Field A component to help laying out interactive information in pages. --- web/src/assets/styles/blocks.scss | 45 ++++++++++ web/src/components/core/Field.jsx | 107 +++++++++++++++++++++++ web/src/components/core/Field.test.jsx | 112 +++++++++++++++++++++++++ web/src/components/core/index.js | 2 + web/src/components/layout/Icon.jsx | 8 ++ 5 files changed, 274 insertions(+) create mode 100644 web/src/components/core/Field.jsx create mode 100644 web/src/components/core/Field.test.jsx diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 7b4bd2874e..d263621fa0 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -743,6 +743,51 @@ section [data-type="agama/reminder"] { } } +[data-type="agama/field"] { + > div:first-child { + font-size: var(--fs-large); + + button { + padding-inline: 0; + } + + button:hover { + color: var(--color-link-hover); + fill: var(--color-link-hover); + } + + button b, button:hover b { + text-decoration: underline; + text-underline-offset: var(--spacer-smaller); + } + } + + > div:nth-child(n+2) { + margin-inline-start: calc(var(--icon-size-s) + 1ch); + } + + > div:nth-child(2) { + color: gray; + font-size: var(--fs-medium); + } + + > div:nth-child(n+3) { + margin-block-start: var(--spacer-small); + } + + &.on { + button { + fill: var(--color-link-hover); + } + } + + hr { + margin-block: var(--spacer-normal); + border: 0; + border-bottom: thin dashed var(--color-gray); + } +} + #boot-form { legend { label { diff --git a/web/src/components/core/Field.jsx b/web/src/components/core/Field.jsx new file mode 100644 index 0000000000..f98fd32b91 --- /dev/null +++ b/web/src/components/core/Field.jsx @@ -0,0 +1,107 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React from "react"; +import { Icon } from "~/components/layout"; +import { If } from "~/components/core"; + +/** + * @typedef {import("~/components/layout/Icon").IconName} IconName + * @typedef {import("~/components/layout/Icon").IconSize} IconSize + */ + +/** + * @typedef {object} FieldProps + * @property {React.ReactNode} label - The field label. + * @property {React.ReactNode} [value] - The field value. + * @property {React.ReactNode} [description] - A field description, useful for providing context to the user. + * @property {IconName} [icon] - The name of the icon for the field. + * @property {IconSize} [iconSize="s"] - The size for the field icon. + * @property {string} [className] - ClassName + * @property {() => {}} [onClick] - Callback + * @property {React.ReactNode} [children] - A content to be rendered as field children + * + * @typedef {Omit} FieldPropsWithoutIcon + */ + +/** + * Component for laying out a page field + * + * @param {FieldProps} props + */ +const Field = ({ + label, + value, + description, + icon, + iconSize = "s", + onClick, + children, + ...props +}) => { + return ( +
+
+ {value} +
+
+ {description} +
+
+ { children } +
+
+ ); +}; + +/** + * @param {Omit} props + */ +const SettingsField = ({ ...props }) => { + return ; +}; + +/** + * @param {Omit & {isChecked: true}} props + */ +const SwitchField = ({ isChecked, ...props }) => { + const iconName = isChecked ? "toggle_on" : "toggle_off"; + const className = isChecked ? "on" : "off"; + + return ; +}; + +/** + * @param {Omit & {isExpanded: boolean}} props + */ +const ExpandableField = ({ isExpanded, ...props }) => { + const iconName = isExpanded ? "collapse_all" : "expand_all"; + const className = isExpanded ? "expanded" : "collapsed"; + + return ; +}; + +export default Field; +export { ExpandableField, SettingsField, SwitchField }; diff --git a/web/src/components/core/Field.test.jsx b/web/src/components/core/Field.test.jsx new file mode 100644 index 0000000000..ffaae06651 --- /dev/null +++ b/web/src/components/core/Field.test.jsx @@ -0,0 +1,112 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { Field, ExpandableField, SettingsField, SwitchField } from "~/components/core"; + +const onClick = jest.fn(); + +describe("Field", () => { + it("renders a button with given icon and label", () => { + const { container } = plainRender( + + ); + screen.getByRole("button", { name: "Theme" }); + const icon = container.querySelector("button > svg"); + expect(icon).toHaveAttribute("data-icon-name", "edit"); + }); + + it("renders value, description, and given children", () => { + plainRender( + +

This is a preview

; +
+ ); + screen.getByText("dark"); + screen.getByText("Choose your preferred color schema."); + screen.getByText("This is a"); + screen.getByText("preview"); + }); + + it("triggers the onClick callback when users clicks the button", async () => { + const { user } = plainRender( + + ); + const button = screen.getByRole("button"); + await user.click(button); + expect(onClick).toHaveBeenCalled(); + }); +}); + +describe("SettingsField", () => { + it("uses the 'settings' icon", () => { + const { container } = plainRender( + // Trying to set other icon, although typechecking should catch it. + + ); + const icon = container.querySelector("button > svg"); + expect(icon).toHaveAttribute("data-icon-name", "settings"); + }); +}); + +describe("SwitchField", () => { + it("uses the 'toggle_on' icon when isChecked", () => { + const { container } = plainRender( + + ); + const icon = container.querySelector("button > svg"); + expect(icon).toHaveAttribute("data-icon-name", "toggle_on"); + }); + + it("uses the 'toggle_off' icon when not isChecked", () => { + const { container } = plainRender( + + ); + const icon = container.querySelector("button > svg"); + expect(icon).toHaveAttribute("data-icon-name", "toggle_off"); + }); +}); + +describe("ExpandableField", () => { + it("uses the 'collapse_all' icon when isExpanded", () => { + const { container } = plainRender( + + ); + const icon = container.querySelector("button > svg"); + expect(icon).toHaveAttribute("data-icon-name", "collapse_all"); + }); + + it("uses the 'expand_all' icon when not isExpanded", () => { + const { container } = plainRender( + + ); + const icon = container.querySelector("button > svg"); + expect(icon).toHaveAttribute("data-icon-name", "expand_all"); + }); +}); diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index 7e0a4837ff..c881137abb 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -62,3 +62,5 @@ export { default as Reminder } from "./Reminder"; export { default as Tag } from "./Tag"; export { default as TreeTable } from "./TreeTable"; export { default as ControlledPanels } from "./ControlledPanels"; +export { default as Field } from "./Field"; +export { ExpandableField, SettingsField, SwitchField } from "./Field"; diff --git a/web/src/components/layout/Icon.jsx b/web/src/components/layout/Icon.jsx index f32e8cef90..09d2a36e92 100644 --- a/web/src/components/layout/Icon.jsx +++ b/web/src/components/layout/Icon.jsx @@ -28,6 +28,7 @@ import Apps from "@icons/apps.svg?component"; import Badge from "@icons/badge.svg?component"; import CheckCircle from "@icons/check_circle.svg?component"; import ChevronRight from "@icons/chevron_right.svg?component"; +import CollapseAll from "@icons/collapse_all.svg?component"; import Delete from "@icons/delete.svg?component"; import Description from "@icons/description.svg?component"; import Download from "@icons/download.svg?component"; @@ -35,6 +36,7 @@ import Downloading from "@icons/downloading.svg?component"; import Edit from "@icons/edit.svg?component"; import EditSquare from "@icons/edit_square.svg?component"; import Error from "@icons/error.svg?component"; +import ExpandAll from "@icons/expand_all.svg?component"; import ExpandMore from "@icons/expand_more.svg?component"; import Folder from "@icons/folder.svg?component"; import FolderOff from "@icons/folder_off.svg?component"; @@ -64,6 +66,8 @@ import Storage from "@icons/storage.svg?component"; import Sync from "@icons/sync.svg?component"; import TaskAlt from "@icons/task_alt.svg?component"; import Terminal from "@icons/terminal.svg?component"; +import ToggleOff from "@icons/toggle_off.svg?component"; +import ToggleOn from "@icons/toggle_on.svg?component"; import Translate from "@icons/translate.svg?component"; import Tune from "@icons/tune.svg?component"; import Warning from "@icons/warning.svg?component"; @@ -90,6 +94,7 @@ const icons = { badge: Badge, check_circle: CheckCircle, chevron_right: ChevronRight, + collapse_all: CollapseAll, delete: Delete, description: Description, download: Download, @@ -97,6 +102,7 @@ const icons = { edit: Edit, edit_square: EditSquare, error: Error, + expand_all: ExpandAll, expand_more: ExpandMore, folder: Folder, folder_off: FolderOff, @@ -127,6 +133,8 @@ const icons = { sync: Sync, task_alt: TaskAlt, terminal: Terminal, + toggle_off: ToggleOff, + toggle_on: ToggleOn, translate: Translate, tune: Tune, visibility: Visibility, From e483ec2e0968d932819326c63db98c8388d02852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 9 Apr 2024 17:33:27 +0100 Subject: [PATCH 03/31] web: core/Field improvements - Add styles for making possible to use PF/Skeleton as field value. - Fix the return type of the onClick callback. --- web/src/assets/styles/blocks.scss | 6 ++++++ web/src/components/core/Field.jsx | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index d263621fa0..f03405fcaa 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -760,6 +760,12 @@ section [data-type="agama/reminder"] { text-decoration: underline; text-underline-offset: var(--spacer-smaller); } + + div.pf-v5-c-skeleton { + display: inline-block; + vertical-align: middle; + height: 1.5ex; + } } > div:nth-child(n+2) { diff --git a/web/src/components/core/Field.jsx b/web/src/components/core/Field.jsx index f98fd32b91..54afbbfaea 100644 --- a/web/src/components/core/Field.jsx +++ b/web/src/components/core/Field.jsx @@ -38,7 +38,7 @@ import { If } from "~/components/core"; * @property {IconName} [icon] - The name of the icon for the field. * @property {IconSize} [iconSize="s"] - The size for the field icon. * @property {string} [className] - ClassName - * @property {() => {}} [onClick] - Callback + * @property {() => void} [onClick] - Callback * @property {React.ReactNode} [children] - A content to be rendered as field children * * @typedef {Omit} FieldPropsWithoutIcon From 4bfd0aa5cc4d73818f7957ab276da6b556c62d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 9 Apr 2024 17:35:27 +0100 Subject: [PATCH 04/31] web: Move storage/SpacePolicyField to its own file And adapts it to use a core/Field/SettingsField button. --- .../storage/ProposalSettingsSection.jsx | 71 +----------- .../storage/ProposalSettingsSection.test.jsx | 42 ------- .../components/storage/SpacePolicyField.jsx | 106 ++++++++++++++++++ .../storage/SpacePolicyField.test.jsx | 71 ++++++++++++ web/src/components/storage/utils.js | 2 +- 5 files changed, 181 insertions(+), 111 deletions(-) create mode 100644 web/src/components/storage/SpacePolicyField.jsx create mode 100644 web/src/components/storage/SpacePolicyField.test.jsx diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 8d9fc4f253..fd808fd348 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -25,8 +25,9 @@ import React, { useEffect, useState } from "react"; import { Button, Checkbox, Form, Skeleton, Switch, Tooltip } from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; -import { _, n_ } from "~/i18n"; -import { BootSelectionDialog, ProposalVolumes, SpacePolicyDialog } from "~/components/storage"; +import { _ } from "~/i18n"; +import { BootSelectionDialog, ProposalVolumes } from "~/components/storage"; +import SpacePolicyField from "~/components/storage/SpacePolicyField"; import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core"; import { Icon } from "~/components/layout"; import { noop } from "~/utils"; @@ -337,72 +338,6 @@ const BootConfigField = ({ ); }; -/** - * Allows to select the space policy. - * @component - * - * @param {object} props - * @param {SpacePolicy|undefined} props.policy - * @param {SpaceAction[]} props.actions - * @param {StorageDevice[]} props.devices - * @param {boolean} props.isLoading - * @param {(config: SpacePolicyConfig) => void} props.onChange - * - * @typedef {object} SpacePolicyConfig - * @property {SpacePolicy} spacePolicy - * @property {SpaceAction[]} spaceActions - */ -const SpacePolicyField = ({ - policy, - actions, - devices, - isLoading, - onChange -}) => { - const [isDialogOpen, setIsDialogOpen] = useState(false); - - const openDialog = () => setIsDialogOpen(true); - - const closeDialog = () => setIsDialogOpen(false); - - const onAccept = ({ spacePolicy, spaceActions }) => { - closeDialog(); - onChange({ spacePolicy, spaceActions }); - }; - - const label = () => { - // eslint-disable-next-line agama-i18n/string-literals - if (policy.summaryLabels.length === 1) return _(policy.summaryLabels[0]); - - // eslint-disable-next-line agama-i18n/string-literals - return sprintf(n_(policy.summaryLabels[0], policy.summaryLabels[1], devices.length), devices.length); - }; - - if (isLoading || !policy) { - return ; - } - - return ( -
- {_("Find space")} - - - } - /> -
- ); -}; - /** * Section for editing the proposal settings * @component diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx index 03e05df65b..27dd592d9b 100644 --- a/web/src/components/storage/ProposalSettingsSection.test.jsx +++ b/web/src/components/storage/ProposalSettingsSection.test.jsx @@ -281,45 +281,3 @@ describe("Encryption field", () => { }); }); }); - -describe("Space policy field", () => { - describe.skip("if there is no space policy", () => { - beforeEach(() => { - // Currently settings cannot be undefined. - props.settings = undefined; - }); - - it("does not render the space policy field", () => { - plainRender(); - - expect(screen.queryByLabelText("Find space")).toBeNull(); - }); - }); - - describe("if there is a space policy", () => { - beforeEach(() => { - props.settings.spacePolicy = "delete"; - }); - - it("renders the button with a text according to given policy", () => { - const { rerender } = plainRender(); - screen.getByRole("button", { name: /deleting/ }); - - props.settings.spacePolicy = "resize"; - rerender(); - screen.getByRole("button", { name: /shrinking/ }); - }); - - it("allows to change the policy", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: /deleting all content/ }); - - await user.click(button); - - const popup = await screen.findByRole("dialog"); - within(popup).getByText("Find space"); - const cancel = within(popup).getByRole("button", { name: "Cancel" }); - await user.click(cancel); - }); - }); -}); diff --git a/web/src/components/storage/SpacePolicyField.jsx b/web/src/components/storage/SpacePolicyField.jsx new file mode 100644 index 0000000000..a1bc41e64c --- /dev/null +++ b/web/src/components/storage/SpacePolicyField.jsx @@ -0,0 +1,106 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React, { useState } from "react"; +import { Skeleton } from "@patternfly/react-core"; + +import { sprintf } from "sprintf-js"; +import { _, n_ } from "~/i18n"; +import { If, SettingsField } from "~/components/core"; +import SpacePolicyDialog from "~/components/storage/SpacePolicyDialog"; + +/** + * @typedef {import ("~/client/storage").SpaceAction} SpaceAction + * @typedef {import ("~/components/storage/utils").SpacePolicy} SpacePolicy + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + * + * @typedef {object} SpacePolicyConfig + * @property {SpacePolicy} spacePolicy + * @property {SpaceAction[]} spaceActions + */ + +/** + * Allows to select the space policy. + * @component + * + * @param {object} props + * @param {SpacePolicy|undefined} props.policy + * @param {SpaceAction[]} props.actions + * @param {StorageDevice[]} props.devices + * @param {boolean} props.isLoading + * @param {(config: SpacePolicyConfig) => void} props.onChange + * + */ +export default function SpacePolicyField({ + policy, + actions, + devices, + isLoading, + onChange +}) { + const [isDialogOpen, setIsDialogOpen] = useState(false); + + const openDialog = () => setIsDialogOpen(true); + + const closeDialog = () => setIsDialogOpen(false); + + const onAccept = ({ spacePolicy, spaceActions }) => { + closeDialog(); + onChange({ spacePolicy, spaceActions }); + }; + + let value; + if (isLoading || !policy) { + value = ; + } else if (policy.summaryLabels.length === 1) { + // eslint-disable-next-line agama-i18n/string-literals + value = _(policy.summaryLabels[0]); + } else { + // eslint-disable-next-line agama-i18n/string-literals + value = sprintf(n_(policy.summaryLabels[0], policy.summaryLabels[1], devices.length), devices.length); + } + + return ( + + + } + /> + + ); +} diff --git a/web/src/components/storage/SpacePolicyField.test.jsx b/web/src/components/storage/SpacePolicyField.test.jsx new file mode 100644 index 0000000000..a8b70c31fe --- /dev/null +++ b/web/src/components/storage/SpacePolicyField.test.jsx @@ -0,0 +1,71 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React from "react"; +import { screen } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { SPACE_POLICIES } from "~/components/storage/utils"; +import SpacePolicyField from "~/components/storage/SpacePolicyField"; + +const sda = { + sid: 59, + description: "A fake disk for testing", + isDrive: true, + type: "disk", + vendor: "Micron", + model: "Micron 1100 SATA", + driver: ["ahci", "mmcblk"], + bus: "IDE", + busId: "", + transport: "usb", + dellBOSS: false, + sdCard: true, + active: true, + name: "/dev/sda", + size: 1024, + recoverableSize: 0, + systems : [], + udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"], + udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], +}; + +const keepPolicy = SPACE_POLICIES.find(p => p.id === "keep"); + +const props = { + devices: [sda], + policy: keepPolicy, + isLoading: false, + onChange: jest.fn(), + actions: [ + { device: "/dev/sda", action: "force_delete" }, + ] +}; + +describe("SpacePolicyField", () => { + it("renders a button for opening the space policy dialog", async () => { + const { user } = plainRender(); + const button = screen.getByRole("button"); + await user.click(button); + screen.getByRole("dialog", { name: "Find space" }); + }); +}); diff --git a/web/src/components/storage/utils.js b/web/src/components/storage/utils.js index 6e45ad190c..1e842c211c 100644 --- a/web/src/components/storage/utils.js +++ b/web/src/components/storage/utils.js @@ -215,7 +215,7 @@ const deviceLabel = (device) => { const deviceChildren = (device) => { const partitionTableChildren = (partitionTable) => { const { partitions, unusedSlots } = partitionTable; - const children = partitions.concat(unusedSlots); + const children = partitions.concat(unusedSlots).filter(i => !!i); return children.sort((a, b) => a.start < b.start ? -1 : 1); }; From 1f79798fcc8ee638efc12b1e89b98b9de6e2c876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 10 Apr 2024 15:07:03 +0100 Subject: [PATCH 05/31] web: Enable type checking in some test files --- web/src/components/core/Section.test.jsx | 20 ++++++++++++++------ web/src/components/layout/Icon.jsx | 10 +++------- web/src/components/layout/Icon.test.jsx | 19 +++++++++++++------ 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/web/src/components/core/Section.test.jsx b/web/src/components/core/Section.test.jsx index f132a637a6..ede96d0118 100644 --- a/web/src/components/core/Section.test.jsx +++ b/web/src/components/core/Section.test.jsx @@ -19,18 +19,23 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; import { screen, within } from "@testing-library/react"; import { plainRender, installerRender } from "~/test-utils"; import { Section } from "~/components/core"; +let consoleErrorSpy; + describe("Section", () => { beforeAll(() => { - jest.spyOn(console, "error").mockImplementation(); + consoleErrorSpy = jest.spyOn(console, "error"); + consoleErrorSpy.mockImplementation(); }); afterAll(() => { - console.error.mockRestore(); + consoleErrorSpy.mockRestore(); }); describe("when title is given", () => { @@ -60,7 +65,8 @@ describe("Section", () => { }); it("does not render an icon if not valid icon name is given", () => { - const { container } = plainRender(
); + // @ts-expect-error: Creating the icon name dynamically is unlikely, but let's be safe. + const { container } = plainRender(
); const icon = container.querySelector("svg"); expect(icon).toBeNull(); }); @@ -124,12 +130,14 @@ describe("Section", () => { it("sets predictable header id if name is given", () => { plainRender(
); - screen.getByRole("heading", { name: "Settings", id: "settings-header-section" }); + const section = screen.getByRole("heading", { name: "Settings" }); + expect(section).toHaveAttribute("id", "settings-section-header"); }); it("sets partially random header id if name is not given", () => { - plainRender(
); - screen.getByRole("heading", { name: "Settings", id: /.*(-header-section)$/ }); + plainRender(
); + const section = screen.getByRole("heading", { name: "Settings" }); + expect(section).toHaveAttribute("id", expect.stringContaining("section-header")); }); it("renders a polite live region", () => { diff --git a/web/src/components/layout/Icon.jsx b/web/src/components/layout/Icon.jsx index 09d2a36e92..33a1f229e0 100644 --- a/web/src/components/layout/Icon.jsx +++ b/web/src/components/layout/Icon.jsx @@ -172,13 +172,9 @@ const PREDEFINED_SIZES = [ * @returns {JSX.Element|null} null if requested icon is not available or given a falsy value as name; JSX block otherwise. */ export default function Icon({ name, size, ...otherProps }) { - if (!name) { - console.error(`Icon called without name. '${name}' given instead. Rendering nothing.`); - return null; - } - - if (!icons[name]) { - console.error(`Icon '${name}' not found!`); + // NOTE: Reaching this is unlikely, but let's be safe. + if (!name || !icons[name]) { + console.error(`Icon '${name}' not found.`); return null; } diff --git a/web/src/components/layout/Icon.test.jsx b/web/src/components/layout/Icon.test.jsx index 2e7a707e88..bf2212b33c 100644 --- a/web/src/components/layout/Icon.test.jsx +++ b/web/src/components/layout/Icon.test.jsx @@ -19,17 +19,22 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React from "react"; import { plainRender } from "~/test-utils"; import { Icon } from "~/components/layout"; +let consoleErrorSpy; + describe("Icon", () => { beforeAll(() => { - jest.spyOn(console, "error").mockImplementation(); + consoleErrorSpy = jest.spyOn(console, "error"); + consoleErrorSpy.mockImplementation(); }); afterAll(() => { - console.error.mockRestore(); + consoleErrorSpy.mockRestore(); }); describe("mounted with a known name", () => { @@ -70,6 +75,7 @@ describe("Icon", () => { describe("mounted with unknown name", () => { it("outputs to console.error", () => { + // @ts-expect-error: It's unlikely to happen, but let's test it anyway plainRender(); expect(console.error).toHaveBeenCalledWith( expect.stringContaining("'apsens' not found") @@ -77,6 +83,7 @@ describe("Icon", () => { }); it("renders nothing", async () => { + // @ts-expect-error: It's unlikely to happen, but let's test it anyway const { container } = plainRender(); expect(container).toBeEmptyDOMElement(); }); @@ -84,19 +91,19 @@ describe("Icon", () => { describe("mounted with a falsy value as name", () => { it("outputs to console.error", () => { + // @ts-expect-error: It's unlikely to happen, but let's test it anyway plainRender(); expect(console.error).toHaveBeenCalledWith( - expect.stringContaining("Rendering nothing") + expect.stringContaining("not found") ); }); it("renders nothing", () => { - const { container: contentWhenNotDefined } = plainRender(); - expect(contentWhenNotDefined).toBeEmptyDOMElement(); - + // @ts-expect-error: It's unlikely to happen, but let's test it anyway const { container: contentWhenEmpty } = plainRender(); expect(contentWhenEmpty).toBeEmptyDOMElement(); + // @ts-expect-error: It's unlikely to happen, but let's test it anyway const { container: contentWhenFalse } = plainRender(); expect(contentWhenFalse).toBeEmptyDOMElement(); From 51d55a8d5f4e6268292819a0f861a98d52246cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 10 Apr 2024 16:51:37 +0100 Subject: [PATCH 06/31] web: Move storage/BootConfigField to its own file And adapts it to looks a bit different according the changes proposed in the context of https://trello.com/c/czpTfm3y (internal link). It enables type checking in the storage/BootSelectionDialog.test.jsx too. --- web/src/assets/styles/utilities.scss | 8 ++ web/src/components/layout/Icon.jsx | 4 + .../components/storage/BootConfigField.jsx | 124 ++++++++++++++++++ .../storage/BootConfigField.test.jsx | 112 ++++++++++++++++ .../storage/ProposalSettingsSection.jsx | 80 +---------- 5 files changed, 252 insertions(+), 76 deletions(-) create mode 100644 web/src/components/storage/BootConfigField.jsx create mode 100644 web/src/components/storage/BootConfigField.test.jsx diff --git a/web/src/assets/styles/utilities.scss b/web/src/assets/styles/utilities.scss index ae12b38719..63d1422b2c 100644 --- a/web/src/assets/styles/utilities.scss +++ b/web/src/assets/styles/utilities.scss @@ -138,6 +138,14 @@ padding: 0; } +.inline-flex-button{ + @extend .plain-button; + display: inline-flex; + align-items: center; + gap: 0.7ch; + text-decoration: underline; +} + .p-0 { padding: 0; } diff --git a/web/src/components/layout/Icon.jsx b/web/src/components/layout/Icon.jsx index 33a1f229e0..848d4859cc 100644 --- a/web/src/components/layout/Icon.jsx +++ b/web/src/components/layout/Icon.jsx @@ -38,6 +38,7 @@ import EditSquare from "@icons/edit_square.svg?component"; import Error from "@icons/error.svg?component"; import ExpandAll from "@icons/expand_all.svg?component"; import ExpandMore from "@icons/expand_more.svg?component"; +import Feedback from "@icons/feedback.svg?component"; import Folder from "@icons/folder.svg?component"; import FolderOff from "@icons/folder_off.svg?component"; import Globe from "@icons/globe.svg?component"; @@ -61,6 +62,7 @@ import Schedule from "@icons/schedule.svg?component"; import SettingsApplications from "@icons/settings_applications.svg?component"; import SettingsEthernet from "@icons/settings_ethernet.svg?component"; import SettingsFill from "@icons/settings-fill.svg?component"; +import Shadow from "@icons/shadow.svg?component"; import SignalCellularAlt from "@icons/signal_cellular_alt.svg?component"; import Storage from "@icons/storage.svg?component"; import Sync from "@icons/sync.svg?component"; @@ -104,6 +106,7 @@ const icons = { error: Error, expand_all: ExpandAll, expand_more: ExpandMore, + feedback: Feedback, folder: Folder, folder_off: FolderOff, globe: Globe, @@ -128,6 +131,7 @@ const icons = { settings: SettingsFill, settings_applications: SettingsApplications, settings_ethernet: SettingsEthernet, + shadow: Shadow, signal_cellular_alt: SignalCellularAlt, storage: Storage, sync: Sync, diff --git a/web/src/components/storage/BootConfigField.jsx b/web/src/components/storage/BootConfigField.jsx new file mode 100644 index 0000000000..483e5d9f18 --- /dev/null +++ b/web/src/components/storage/BootConfigField.jsx @@ -0,0 +1,124 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React, { useState } from "react"; +import { Skeleton } from "@patternfly/react-core"; + +import { _ } from "~/i18n"; +import { sprintf } from "sprintf-js"; +import { deviceLabel } from "~/components/storage/utils"; +import { If } from "~/components/core"; +import { Icon } from "~/components/layout"; +import BootSelectionDialog from "~/components/storage/BootSelectionDialog"; + +/** + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + */ + +/** + * Internal component for building the button that opens the dialog + * + * @param {object} props + * @param {boolean} [props.isBold=false] - Whether text should be wrapped by . + * @param {() => void} props.onClick - Callback to trigger when user clicks. + */ +const Button = ({ isBold = false, onClick }) => { + const text = _("Change boot options"); + + return ( + + ); +}; + +/** + * Allows to select the boot config. + * @component + * + * @param {object} props + * @param {boolean} props.configureBoot + * @param {StorageDevice|undefined} props.bootDevice + * @param {StorageDevice|undefined} props.defaultBootDevice + * @param {StorageDevice[]} props.devices + * @param {boolean} props.isLoading + * @param {(boot: Boot) => void} props.onChange + * + * @typedef {object} Boot + * @property {boolean} configureBoot + * @property {StorageDevice} bootDevice + */ +export default function BootConfigField ({ + configureBoot, + bootDevice, + defaultBootDevice, + devices, + isLoading, + onChange +}) { + const [isDialogOpen, setIsDialogOpen] = useState(false); + + const openDialog = () => setIsDialogOpen(true); + + const closeDialog = () => setIsDialogOpen(false); + + const onAccept = ({ configureBoot, bootDevice }) => { + closeDialog(); + onChange({ configureBoot, bootDevice }); + }; + + if (isLoading) { + return ; + } + + let value; + + if (!configureBoot) { + value = <> {_("Installation will not create boot partitions.")}; + } else if (!bootDevice) { + value = _("Installation might create boot partitions at the installation device."); + } else { + // TRANSLATORS: %s is the disk used to configure the boot-related partitions (eg. "/dev/sda, 80 GiB) + value = sprintf(_("Installation might create boot partitions at %s."), deviceLabel(bootDevice)); + } + + return ( +
+ { value }
+ ); +} diff --git a/web/src/components/storage/BootConfigField.test.jsx b/web/src/components/storage/BootConfigField.test.jsx new file mode 100644 index 0000000000..f4945e46bb --- /dev/null +++ b/web/src/components/storage/BootConfigField.test.jsx @@ -0,0 +1,112 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import BootConfigField from "~/components/storage/BootConfigField"; + +const sda = { + sid: 59, + description: "A fake disk for testing", + isDrive: true, + type: "disk", + vendor: "Micron", + model: "Micron 1100 SATA", + driver: ["ahci", "mmcblk"], + bus: "IDE", + busId: "", + transport: "usb", + dellBOSS: false, + sdCard: true, + active: true, + name: "/dev/sda", + size: 1024, + recoverableSize: 0, + systems : [], + udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"], + udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], +}; + +let props; + +beforeEach(() => { + props = { + configureBoot: false, + bootDevice: undefined, + defaultBootDevice: undefined, + devices: [sda], + isLoading: false, + onChange: jest.fn() + }; +}); + +/** + * Helper function that implicitly test that field provides a button for + * opening the dialog + */ +const openBootConfigDialog = async () => { + const { user } = plainRender(); + const button = screen.getByRole("button"); + await user.click(button); + const dialog = screen.getByRole("dialog", { name: "Partitions for booting" }); + + return { user, dialog }; +}; + +describe("BootConfigField", () => { + it("triggers onChange callback when user confirms the dialog", async () => { + const { user, dialog } = await openBootConfigDialog(); + const button = within(dialog).getByRole("button", { name: "Confirm" }); + await user.click(button); + expect(props.onChange).toHaveBeenCalled(); + }); + + it("does not trigger onChange callback when user cancels the dialog", async () => { + const { user, dialog } = await openBootConfigDialog(); + const button = within(dialog).getByRole("button", { name: "Cancel" }); + await user.click(button); + expect(props.onChange).not.toHaveBeenCalled(); + }); + + describe("when installation is set for not configuring boot", () => { + it("renders a text warning about it", () => { + plainRender(); + screen.getByText(/will not create boot partitions/); + }); + }); + + describe("when installation is set for automatically configuring boot", () => { + it("renders a text reporting about it", () => { + plainRender(); + screen.getByText(/create boot partitions at the installation device/); + }); + }); + + describe("when installation is set for configuring boot at specific device", () => { + it("renders a text reporting about it", () => { + plainRender(); + screen.getByText(/boot partitions at \/dev\/sda/); + }); + }); +}); diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index fd808fd348..cb577fc1fa 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -22,16 +22,16 @@ // @ts-check import React, { useEffect, useState } from "react"; -import { Button, Checkbox, Form, Skeleton, Switch, Tooltip } from "@patternfly/react-core"; +import { Checkbox, Form, Skeleton, Switch, Tooltip } from "@patternfly/react-core"; -import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; -import { BootSelectionDialog, ProposalVolumes } from "~/components/storage"; +import { ProposalVolumes } from "~/components/storage"; import SpacePolicyField from "~/components/storage/SpacePolicyField"; +import BootConfigField from "~/components/storage/BootConfigField"; import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core"; import { Icon } from "~/components/layout"; import { noop } from "~/utils"; -import { hasFS, deviceLabel, SPACE_POLICIES } from "~/components/storage/utils"; +import { hasFS, SPACE_POLICIES } from "~/components/storage/utils"; /** * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings @@ -266,78 +266,6 @@ const EncryptionField = ({ ); }; -/** - * Allows to select the boot config. - * @component - * - * @param {object} props - * @param {boolean} props.configureBoot - * @param {StorageDevice|undefined} props.bootDevice - * @param {StorageDevice|undefined} props.defaultBootDevice - * @param {StorageDevice[]} props.devices - * @param {boolean} props.isLoading - * @param {(boot: Boot) => void} props.onChange - * - * @typedef {object} Boot - * @property {boolean} configureBoot - * @property {StorageDevice} bootDevice - */ -const BootConfigField = ({ - configureBoot, - bootDevice, - defaultBootDevice, - devices, - isLoading, - onChange -}) => { - const [isDialogOpen, setIsDialogOpen] = useState(false); - - const openDialog = () => setIsDialogOpen(true); - - const closeDialog = () => setIsDialogOpen(false); - - const onAccept = ({ configureBoot, bootDevice }) => { - closeDialog(); - onChange({ configureBoot, bootDevice }); - }; - - const label = _("Automatically configure any additional partition to boot the system"); - - const value = () => { - if (!configureBoot) return _("nowhere (manual boot setup)"); - - if (!bootDevice) return _("at the installation device"); - - // TRANSLATORS: %s is the disk used to configure the boot-related partitions (eg. "/dev/sda, 80 GiB) - return sprintf(_("at %s"), deviceLabel(bootDevice)); - }; - - if (isLoading) { - return ; - } - - return ( -
- {label} - - - } - /> -
- ); -}; - /** * Section for editing the proposal settings * @component From 81937012d75df042bbb40d22facb739f2f7b33f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 11 Apr 2024 14:58:05 +0100 Subject: [PATCH 07/31] web: Move storage/SnapshotsField to its own file Adapting it for using the core/Field/SwitchField component. As part of this commit, core/Field has also been improved and made a bit more accessible as well as allowed for different graphical representations based on the wrapper chosen for the button text. --- web/src/assets/styles/utilities.scss | 1 + web/src/components/core/Field.jsx | 27 ++++-- web/src/components/core/Field.test.jsx | 21 ++++- .../storage/ProposalSettingsSection.jsx | 70 +++------------ .../storage/ProposalSettingsSection.test.jsx | 19 ++-- web/src/components/storage/SnapshotsField.jsx | 69 ++++++++++++++ .../storage/SnapshotsField.test.jsx | 90 +++++++++++++++++++ 7 files changed, 223 insertions(+), 74 deletions(-) create mode 100644 web/src/components/storage/SnapshotsField.jsx create mode 100644 web/src/components/storage/SnapshotsField.test.jsx diff --git a/web/src/assets/styles/utilities.scss b/web/src/assets/styles/utilities.scss index 63d1422b2c..f4303c8617 100644 --- a/web/src/assets/styles/utilities.scss +++ b/web/src/assets/styles/utilities.scss @@ -136,6 +136,7 @@ color: inherit; font: inherit; padding: 0; + text-align: start; } .inline-flex-button{ diff --git a/web/src/components/core/Field.jsx b/web/src/components/core/Field.jsx index 54afbbfaea..e05bfc84b5 100644 --- a/web/src/components/core/Field.jsx +++ b/web/src/components/core/Field.jsx @@ -23,9 +23,9 @@ import React from "react"; import { Icon } from "~/components/layout"; -import { If } from "~/components/core"; /** + * @typedef {import("react").ButtonHTMLAttributes} ButtonHTMLAttributes * @typedef {import("~/components/layout/Icon").IconName} IconName * @typedef {import("~/components/layout/Icon").IconSize} IconSize */ @@ -37,6 +37,8 @@ import { If } from "~/components/core"; * @property {React.ReactNode} [description] - A field description, useful for providing context to the user. * @property {IconName} [icon] - The name of the icon for the field. * @property {IconSize} [iconSize="s"] - The size for the field icon. + * @property {("b"|"span")} [textWrapper="b"] - The element used for wrapping the label. + * @property {ButtonHTMLAttributes} [buttonAttrs={}] - The element used for wrapping the label. * @property {string} [className] - ClassName * @property {() => void} [onClick] - Callback * @property {React.ReactNode} [children] - A content to be rendered as field children @@ -57,20 +59,24 @@ const Field = ({ iconSize = "s", onClick, children, + textWrapper = "b", + buttonAttrs = {}, ...props }) => { + const FieldIcon = () => icon?.length > 0 && ; + const TextWrapper = textWrapper; return (
- {value}
{description}
- { children } + {children}
); @@ -84,13 +90,20 @@ const SettingsField = ({ ...props }) => { }; /** - * @param {Omit & {isChecked: true}} props + * @param {Omit & {isChecked: boolean}} props */ -const SwitchField = ({ isChecked, ...props }) => { +const SwitchField = ({ isChecked = false, ...props }) => { const iconName = isChecked ? "toggle_on" : "toggle_off"; const className = isChecked ? "on" : "off"; - return ; + return ( + + ); }; /** diff --git a/web/src/components/core/Field.test.jsx b/web/src/components/core/Field.test.jsx index ffaae06651..d84295d1e4 100644 --- a/web/src/components/core/Field.test.jsx +++ b/web/src/components/core/Field.test.jsx @@ -76,9 +76,26 @@ describe("SettingsField", () => { }); describe("SwitchField", () => { + it("sets button role to switch", () => { + plainRender(); + const switchButton = screen.getByRole("switch", { name: "Zoom" }); + expect(switchButton instanceof HTMLButtonElement).toBe(true); + }); + + it("keeps aria-checked attribute in sync with isChecked prop", () => { + let switchButton; + const { rerender } = plainRender(); + switchButton = screen.getByRole("switch", { name: "Zoom" }); + expect(switchButton).toHaveAttribute("aria-checked", "true"); + + rerender(); + switchButton = screen.getByRole("switch", { name: "Zoom" }); + expect(switchButton).toHaveAttribute("aria-checked", "false"); + }); + it("uses the 'toggle_on' icon when isChecked", () => { const { container } = plainRender( - + ); const icon = container.querySelector("button > svg"); expect(icon).toHaveAttribute("data-icon-name", "toggle_on"); @@ -86,7 +103,7 @@ describe("SwitchField", () => { it("uses the 'toggle_off' icon when not isChecked", () => { const { container } = plainRender( - + ); const icon = container.querySelector("button > svg"); expect(icon).toHaveAttribute("data-icon-name", "toggle_off"); diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index cb577fc1fa..18da47bf4f 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -28,10 +28,11 @@ import { _ } from "~/i18n"; import { ProposalVolumes } from "~/components/storage"; import SpacePolicyField from "~/components/storage/SpacePolicyField"; import BootConfigField from "~/components/storage/BootConfigField"; +import SnapshotsField from "~/components/storage/SnapshotsField"; import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core"; import { Icon } from "~/components/layout"; import { noop } from "~/utils"; -import { hasFS, SPACE_POLICIES } from "~/components/storage/utils"; +import { SPACE_POLICIES } from "~/components/storage/utils"; /** * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings @@ -115,56 +116,6 @@ const EncryptionSettingsForm = ({ ); }; -/** - * Allows to define snapshots enablement - * @component - * - * @param {object} props - * @param {ProposalSettings} props.settings - Settings used for calculating a proposal. - * @param {(config: SnapshotsConfig) => void} [props.onChange=noop] - On change callback - * - * @typedef {object} SnapshotsConfig - * @property {boolean} active - * @property {ProposalSettings} settings - */ -const SnapshotsField = ({ - settings, - onChange = noop -}) => { - const rootVolume = (settings.volumes || []).find((i) => i.mountPath === "/"); - - // no root volume is probably some error or still loading - if (rootVolume === undefined) { - return ; - } - - const isChecked = rootVolume !== undefined && hasFS(rootVolume, "Btrfs") && rootVolume.snapshots; - - const switchState = (_, checked) => { - onChange({ active: checked, settings }); - }; - - if (!rootVolume.outline.snapshotsConfigurable) return; - - const explanation = _("Uses Btrfs for the root file system allowing to boot to a previous \ -version of the system after configuration changes or software upgrades."); - - return ( -
- -
- {explanation} -
-
- ); -}; - /** * Allows to define encryption * @component @@ -246,7 +197,7 @@ const EncryptionField = ({ isChecked={isChecked} onChange={changeSelected} /> - { isChecked && } + {isChecked && } { + const changeBtrfsSnapshots = ({ active }) => { const rootVolume = settings.volumes.find((i) => i.mountPath === "/"); if (active) { @@ -338,12 +289,19 @@ export default function ProposalSettingsSection({ )); }; + const rootVolume = (settings.volumes || []).find((i) => i.mountPath === "/"); + return ( <>
- + } /> { }; }); -describe("if snapshots are configurable", () => { +const rootVolume = { mountPath: "/", fsType: "Btrfs", outline: { snapshotsConfigurable: true } }; + +describe("when snapshots are configurable", () => { beforeEach(() => { props.settings.volumes = [volume]; }); - it("renders the snapshots switch", () => { + it("renders the snapshots field", () => { plainRender(); - - screen.getByRole("checkbox", { name: "Use Btrfs Snapshots" }); + screen.getByRole("switch", { name: /snapshots for the root file system/ }); }); }); -describe("if snapshots are not configurable", () => { +describe("when snapshots are not configurable", () => { beforeEach(() => { volume.outline.snapshotsConfigurable = false; }); - it("does not render the snapshots switch", () => { + it("does not render the snapshots field", () => { plainRender(); - - expect(screen.queryByRole("checkbox", { name: "Use Btrfs Snapshots" })).toBeNull(); + const snapshotsSwitch = screen.queryByRole("switch", { name: /snapshots for the root file system/ }); + expect(snapshotsSwitch).toBeNull(); }); }); @@ -120,7 +121,7 @@ it("renders a section holding file systems related stuff", () => { }); it("requests a volume change when onChange callback is triggered", async () => { - const { user } = plainRender(); + const { user } = plainRender(); const button = screen.getByRole("button", { name: "Actions" }); await user.click(button); diff --git a/web/src/components/storage/SnapshotsField.jsx b/web/src/components/storage/SnapshotsField.jsx new file mode 100644 index 0000000000..d5ba48a25d --- /dev/null +++ b/web/src/components/storage/SnapshotsField.jsx @@ -0,0 +1,69 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React from "react"; + +import { _ } from "~/i18n"; +import { noop } from "~/utils"; +import { hasFS } from "~/components/storage/utils"; +import { SwitchField } from "~/components/core"; + +/** + * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings + * @typedef {import ("~/client/storage").Volume} Volume + */ + +const LABEL = _("Use Btrfs snapshots for the root file system"); +const DESCRIPTION = _("Allows to boot to a previous version of the \ +system after configuration changes or software upgrades."); + +/** + * Allows to define snapshots enablement + * @component + * + * @typedef {object} SnapshotsFieldProps + * @property {Volume} rootVolume + * @property {(config: object) => void} onChange + * + * @param {SnapshotsFieldProps} props + */ +export default function SnapshotsField({ + rootVolume, + onChange = noop +}) { + const isChecked = hasFS(rootVolume, "Btrfs") && rootVolume.snapshots; + + const switchState = () => { + onChange({ active: !isChecked }); + }; + + return ( + + ); +} diff --git a/web/src/components/storage/SnapshotsField.test.jsx b/web/src/components/storage/SnapshotsField.test.jsx new file mode 100644 index 0000000000..a1cb27e12b --- /dev/null +++ b/web/src/components/storage/SnapshotsField.test.jsx @@ -0,0 +1,90 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React from "react"; +import { screen } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import SnapshotsField from "~/components/storage/SnapshotsField"; + +/** + * @typedef {import ("~/client/storage").Volume} Volume + * @typedef {import ("~/components/storage/SnapshotsField").SnapshotsFieldProps} SnapshotsFieldProps + */ + +/** @type {Volume} */ +const rootVolume = { + mountPath: "/", + target: "/dev/sda", + fsType: "Btrfs", + minSize: 1024, + autoSize: true, + snapshots: true, + transactional: false, + outline: { + required: true, + fsTypes: ["ext4", "btrfs"], + supportAutoSize: true, + snapshotsConfigurable: false, + snapshotsAffectSizes: true, + sizeRelevantVolumes: ["/home"] + } +}; + +const onChangeFn = jest.fn(); + +/** @type {SnapshotsFieldProps} */ +let props; + +describe("SnapshotsField", () => { + it("reflects snapshots status", () => { + let button; + + props = { rootVolume: { ...rootVolume, snapshots: true }, onChange: onChangeFn }; + const { rerender } = plainRender(); + button = screen.getByRole("switch"); + expect(button).toHaveAttribute("aria-checked", "true"); + + props = { rootVolume: { ...rootVolume, snapshots: false }, onChange: onChangeFn }; + rerender(); + button = screen.getByRole("switch"); + expect(button).toHaveAttribute("aria-checked", "false"); + }); + + it("allows toggling snapshots status", async () => { + let button; + + props = { rootVolume: { ...rootVolume, snapshots: true }, onChange: onChangeFn }; + const { user, rerender } = plainRender(); + button = screen.getByRole("switch"); + expect(button).toHaveAttribute("aria-checked", "true"); + await user.click(button); + expect(onChangeFn).toHaveBeenCalledWith({ active: false }); + + props = { rootVolume: { ...rootVolume, snapshots: false }, onChange: onChangeFn }; + rerender(); + button = screen.getByRole("switch"); + expect(button).toHaveAttribute("aria-checked", "false"); + await user.click(button); + expect(onChangeFn).toHaveBeenCalledWith({ active: true }); + }); +}); From ace29e0186788a1e04a75853387706067374c5b9 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 12 Apr 2024 11:23:10 +0100 Subject: [PATCH 08/31] [WIP] web: New PartitionsField This groups in one ExpandableSection the content that was before at ProposalVolumes, SnapshotsField and BootConfigField. Tests are not adapted, the test file is simply renamed so far. Type checking and documentation of the methods is still marked as TODO. Ideally, this should contain a separate component VolumesField on its own file instead of incorporating the table directly in the advanced view. That can be done in the future. --- ...roposalVolumes.jsx => PartitionsField.jsx} | 376 +++++++++++++----- ...umes.test.jsx => PartitionsField.test.jsx} | 0 .../storage/ProposalSettingsSection.jsx | 40 +- web/src/components/storage/index.js | 1 - 4 files changed, 288 insertions(+), 129 deletions(-) rename web/src/components/storage/{ProposalVolumes.jsx => PartitionsField.jsx} (56%) rename web/src/components/storage/{ProposalVolumes.test.jsx => PartitionsField.test.jsx} (100%) diff --git a/web/src/components/storage/ProposalVolumes.jsx b/web/src/components/storage/PartitionsField.jsx similarity index 56% rename from web/src/components/storage/ProposalVolumes.jsx rename to web/src/components/storage/PartitionsField.jsx index 52b8205541..ff1a6b5560 100644 --- a/web/src/components/storage/ProposalVolumes.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -22,21 +22,17 @@ // @ts-check import React, { useState } from "react"; -import { - Dropdown, DropdownItem, DropdownList, - List, ListItem, - MenuToggle, - Skeleton, - Toolbar, ToolbarContent, ToolbarItem -} from '@patternfly/react-core'; +import { Button, List, ListItem, Skeleton } from '@patternfly/react-core'; import { Table, Thead, Tr, Th, Tbody, Td } from '@patternfly/react-table'; import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; -import { If, Popup, RowActions, Tip } from '~/components/core'; +import { If, ExpandableField, Popup, RowActions, Tip } from '~/components/core'; import { VolumeForm } from '~/components/storage'; import VolumeLocationDialog from '~/components/storage/VolumeLocationDialog'; import { deviceSize, hasSnapshots, isTransactionalRoot } from '~/components/storage/utils'; +import SnapshotsField from "~/components/storage/SnapshotsField"; +import BootConfigField from "~/components/storage/BootConfigField"; import { noop } from "~/utils"; /** @@ -45,6 +41,119 @@ import { noop } from "~/utils"; * @typedef {import ("~/client/storage").Volume} Volume */ +/** + * TODO: document + */ +const SizeText = (volume) => { + let targetSize; + if (volume.target === "FILESYSTEM" || volume.target === "DEVICE") + targetSize = volume.targetDevice.size; + + const minSize = deviceSize(targetSize || volume.minSize); + const maxSize = targetSize ? deviceSize(targetSize) : volume.maxSize ? deviceSize(volume.maxSize) : undefined; + + if (minSize && maxSize && minSize !== maxSize) return `${minSize} - ${maxSize}`; + // TRANSLATORS: minimum device size, %s is replaced by size string, e.g. "17.5 GiB" + if (maxSize === undefined) return sprintf(_("at least %s"), minSize); + + return `${minSize}`; +}; + +/** + * TODO: document + */ +const BasicVolumeText = (volume, target) => { + const snapshots = hasSnapshots(volume); + const transactional = isTransactionalRoot(volume); + const size = SizeText(volume); + const lvm = (target === "NEW_LVM_VG"); + // When target is "filesystem" or "device" this is irrelevant since the type of device + // is not mentioned + const lv = volume.target === "NEW_VG" || (volume.target === "DEFAULT" && lvm); + + if (transactional) + return (lv) + // TRANSLATORS: "/" is in an LVM logical volume. %s replaced by size string, e.g. "17.5 GiB" + ? sprintf(_("Transactional Btrfs root volume (%s)"), size) + // TRANSLATORS: %s replaced by size string, e.g. "17.5 GiB" + : sprintf(_("Transactional Btrfs root partition (%s)"), size); + + if (snapshots) + return (lv) + // TRANSLATORS: "/" is in an LVM logical volume. %s replaced by size string, e.g. "17.5 GiB" + ? sprintf(_("Btrfs root volume with snapshots (%s)"), size) + // TRANSLATORS: %s replaced by size string, e.g. "17.5 GiB" + : sprintf(_("Btrfs root partition with snapshots (%s)"), size); + + const volTarget = volume.target; + const mount = volume.mountPath; + const device = volume.targetDevice?.name; + + if (volTarget === "FILESYSTEM") + // TRANSLATORS: This results in something like "Mount /dev/sda3 at /home (25 GiB)" since + // %1$s is replaced by the device name, %2$s by the mount point and %3$s by the size + return sprintf(_("Mount %1$s at %2$s (%3$s)"), device, mount, size); + + if (mount === "swap") { + if (volTarget === "DEVICE") + // TRANSLATORS: This results in something like "Swap at /dev/sda3 (2 GiB)" since + // %1$s is replaced by the device name, and %2$s by the size + return sprintf(_("Swap at %1$s (%2$s)"), device, size); + + return (lv) + // TRANSLATORS: Swap is in an LVM logical volume. %s replaced by size string, e.g. "8 GiB" + ? sprintf(_("Swap volume (%s)"), size) + // TRANSLATORS: %s replaced by size string, e.g. "8 GiB" + : sprintf(_("Swap partition (%s)"), size); + } + + const type = volume.fsType; + + if (mount === "/") { + if (volTarget === "DEVICE") + // TRANSLATORS: This results in something like "Btrfs root at /dev/sda3 (20 GiB)" since + // %1$s is replaced by the filesystem type, %2$s by the device name, and %3$s by the size + return sprintf(_("%1$s root at %2$s (%3$s)"), type, device, size); + + return (lv) + // TRANSLATORS: "/" is in an LVM logical volume. + // Results in something like "Btrfs root volume (at least 20 GiB)" since + // $1$s is replaced by filesystem type and %2$s by size description + ? sprintf(_("%1$s root volume (%2$s)"), type, size) + // TRANSLATORS: Results in something like "Btrfs root partition (at least 20 GiB)" since + // $1$s is replaced by filesystem type and %2$s by size description + : sprintf(_("%1$s root partition (%2$s)"), type, size); + } + + if (volTarget === "DEVICE") + // TRANSLATORS: This results in something like "Ext4 /home at /dev/sda3 (20 GiB)" since + // %1$s is replaced by filesystem type, %2$s by mount point, %3$s by device name and %4$s by size + return sprintf(_("%1$s %2$s at %3$s (%4$s)"), type, mount, device, size); + + return (lv) + // TRANSLATORS: The filesystem is in an LVM logical volume. + // Results in something like "Ext4 /home volume (at least 10 GiB)" since + // %1$s is replaced by the filesystem type, %2$s by the mount point and %3$s by the size description + ? sprintf(_("%1$s %2$s volume (%3$s)"), type, mount, size) + // TRANSLATORS: This results in something like "Ext4 /home partition (at least 10 GiB)" since + // %1$s is replaced by the filesystem type, %2$s by the mount point and %3$s by the size description + : sprintf(_("%1$s %2$s partition (%3$s)"), type, mount, size); +}; + +/** + * TODO: document + */ +const BootLabelText = (configure, device) => { + if (!configure) + return _("Do not configure partitions for booting"); + + if (!device) + return _("Boot partitions at installation disk"); + + // TRANSLATORS: %s is the disk used to configure the boot-related partitions (eg. "/dev/sda, 80 GiB) + return sprintf(_("Boot partitions at %s"), device.name); +}; + /** * Generates an hint describing which attributes affect the auto-calculated limits. * If the limits are not affected then it returns `null`. @@ -99,7 +208,6 @@ const AutoCalculatedHint = (volume) => { * @return {void} */ const GeneralActions = ({ templates, onAdd, onReset }) => { - const [isOpen, setIsOpen] = useState(false); const [isFormOpen, setIsFormOpen] = useState(false); const openForm = () => setIsFormOpen(true); @@ -111,45 +219,14 @@ const GeneralActions = ({ templates, onAdd, onReset }) => { onAdd(volume); }; - const toggleActions = () => setIsOpen(!isOpen); - - const closeActions = () => setIsOpen(false); - - const Action = ({ children, ...props }) => ( - {children} - ); - return ( - <> - ( - - {/* TRANSLATORS: dropdown label */} - {_("Actions")} - - )} - > - - - {/* TRANSLATORS: dropdown menu label */} - {_("Reset to defaults")} - - - {/* TRANSLATORS: dropdown menu label */} - {_("Add file system")} - - - +
+ + { - +
+ ); +}; + +/** + * TODO: document + */ +const VolumeLabel = ({ volume, target }) => { + return ( +
+ {BasicVolumeText(volume, target)} +
+ ); +}; + +/** + * TODO: document + */ +const BootLabel = ({ bootDevice, configureBoot }) => { + return ( +
+ {BootLabelText(configureBoot, bootDevice)} +
); }; @@ -212,22 +311,11 @@ const VolumeRow = ({ * @param {Volume} props.volume */ const SizeLimits = ({ volume }) => { - let targetSize; - if (volume.target === "FILESYSTEM" || volume.target === "DEVICE") - targetSize = volume.targetDevice.size; - - const minSize = deviceSize(targetSize || volume.minSize); - const maxSize = targetSize ? deviceSize(targetSize) : volume.maxSize ? deviceSize(volume.maxSize) : undefined; const isAuto = volume.autoSize; - let size = minSize; - if (minSize && maxSize && minSize !== maxSize) size = `${minSize} - ${maxSize}`; - // TRANSLATORS: minimum device size, %s is replaced by size string, e.g. "17.5 GiB" - if (maxSize === undefined) size = sprintf(_("At least %s"), minSize); - return (
- {size} + {SizeText(volume)} {/* TRANSLATORS: device flag, the partition size is automatically computed */} {_("auto")}} />
@@ -312,7 +400,7 @@ const VolumeRow = ({ if (isLoading) { return ( - + ); } @@ -437,62 +525,164 @@ const VolumesTable = ({ volumes, devices, target, targetDevice, isLoading, onVol ); }; +/** + * TODO: document + */ +const Basic = ({ volumes, configureBoot, bootDevice, target, isLoading }) => { + if (isLoading) + return ( +
+ +
+ ); + + return ( +
+ { volumes.map((v, i) => ) } + +
+ ); +}; + +/** + * TODO: document + */ +const Advanced = ({ + volumes, + templates, + devices, + target, + targetDevice, + configureBoot, + bootDevice, + defaultBootDevice, + onVolumesChange, + onBootChange, + isLoading +}) => { + const rootVolume = (volumes || []).find((i) => i.mountPath === "/"); + + const addVolume = (volume) => onVolumesChange([...volumes, volume]); + + const resetVolumes = () => onVolumesChange([]); + + const changeBtrfsSnapshots = ({ active }) => { + // const rootVolume = volumes.find((i) => i.mountPath === "/"); + + if (active) { + rootVolume.fsType = "Btrfs"; + rootVolume.snapshots = true; + } else { + rootVolume.snapshots = false; + } + + onVolumesChange(volumes); + }; + + return ( +
+ } + /> + + +
+ +
+ ); +}; + /** * @todo This component should be restructured to use the same approach as other newer components: * * Create dialog components for the popup forms (e.g., EditVolumeDialog). * * Use a TreeTable, specially if we need to represent subvolumes. * - * Renders information of the volumes and actions to modify them + * Renders information of the volumes and boot-related partitions and actions to modify them * @component * - * @typedef {object} ProposalVolumesProps + * @typedef {object} PartitionsFieldProps * @property {Volume[]} volumes - Volumes to show * @property {Volume[]} templates - Templates to use for new volumes * @property {StorageDevice[]} devices - Devices available for installation * @property {ProposalTarget} target - Installation target * @property {StorageDevice|undefined} targetDevice - Device selected for installation, if target is a disk + * @property {TODO} configureBoot - TODO + * @property {TODO} bootDevice - TODO + * @property {TODO} defaultBootDevice - TODO * @property {boolean} [isLoading=false] - Whether to show the content as loading - * @property {(volumes: Volume[]) => void} onChange - Function to use for changing the volumes + * @property {(volumes: Volume[]) => void} onVolumesChange - Function to use for changing the volumes + * @property {(volumes: Volume[]) => void} onBootChange - Function for changing the boot settings * - * @param {ProposalVolumesProps} props + * @param {PartitionsFieldProps} props */ -export default function ProposalVolumes({ +export default function PartitionsField({ volumes, templates, devices, target, targetDevice, + configureBoot, + bootDevice, + defaultBootDevice, isLoading = false, - onChange = noop + onVolumesChange = noop, + onBootChange = noop }) { - const addVolume = (volume) => onChange([...volumes, volume]); - - const resetVolumes = () => onChange([]); + const [isExpanded, setIsExpanded] = useState(false); return ( - <> - - - - {_("File systems to create")} - - - - - - - setIsExpanded(!isExpanded)} + > + + } + else={ + + } /> - + ); } diff --git a/web/src/components/storage/ProposalVolumes.test.jsx b/web/src/components/storage/PartitionsField.test.jsx similarity index 100% rename from web/src/components/storage/ProposalVolumes.test.jsx rename to web/src/components/storage/PartitionsField.test.jsx diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 18da47bf4f..79bd9caa87 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -25,10 +25,8 @@ import React, { useEffect, useState } from "react"; import { Checkbox, Form, Skeleton, Switch, Tooltip } from "@patternfly/react-core"; import { _ } from "~/i18n"; -import { ProposalVolumes } from "~/components/storage"; import SpacePolicyField from "~/components/storage/SpacePolicyField"; -import BootConfigField from "~/components/storage/BootConfigField"; -import SnapshotsField from "~/components/storage/SnapshotsField"; +import PartitionsField from "~/components/storage/PartitionsField"; import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core"; import { Icon } from "~/components/layout"; import { noop } from "~/utils"; @@ -243,19 +241,6 @@ export default function ProposalSettingsSection({ onChange({ encryptionPassword: password, encryptionMethod: method }); }; - const changeBtrfsSnapshots = ({ active }) => { - const rootVolume = settings.volumes.find((i) => i.mountPath === "/"); - - if (active) { - rootVolume.fsType = "Btrfs"; - rootVolume.snapshots = true; - } else { - rootVolume.snapshots = false; - } - - onChange({ volumes: settings.volumes }); - }; - const changeVolumes = (volumes) => { onChange({ volumes }); }; @@ -289,20 +274,9 @@ export default function ProposalSettingsSection({ )); }; - const rootVolume = (settings.volumes || []).find((i) => i.mountPath === "/"); - return ( <>
- - } - /> - - Date: Fri, 12 Apr 2024 12:41:31 +0100 Subject: [PATCH 09/31] web: Add InstallationDeviceField --- .../storage/InstallationDeviceField.jsx | 146 ++++++++++++ .../storage/InstallationDeviceField.test.jsx | 212 ++++++++++++++++++ .../components/storage/ProposalPage.test.jsx | 2 +- .../storage/ProposalSettingsSection.jsx | 38 +++- .../storage/ProposalSettingsSection.test.jsx | 57 ++++- 5 files changed, 448 insertions(+), 7 deletions(-) create mode 100644 web/src/components/storage/InstallationDeviceField.jsx create mode 100644 web/src/components/storage/InstallationDeviceField.test.jsx diff --git a/web/src/components/storage/InstallationDeviceField.jsx b/web/src/components/storage/InstallationDeviceField.jsx new file mode 100644 index 0000000000..2dddcd7cd1 --- /dev/null +++ b/web/src/components/storage/InstallationDeviceField.jsx @@ -0,0 +1,146 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React, { useState } from "react"; +import { Skeleton } from "@patternfly/react-core"; + +import { _ } from "~/i18n"; +import { DeviceSelectionDialog } from "~/components/storage"; +import { deviceLabel } from '~/components/storage/utils'; +import { If, SettingsField } from "~/components/core"; +import { sprintf } from "sprintf-js"; + +/** + * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + */ + +/** + * Generates the target value. + * @function + * + * @param {ProposalTarget} target + * @param {StorageDevice} targetDevice + * @param {StorageDevice[]} targetPVDevices + * @returns {string} + */ +const targetValue = (target, targetDevice, targetPVDevices) => { + if (target === "DISK" && targetDevice) return deviceLabel(targetDevice); + if (target === "NEW_LVM_VG" && targetPVDevices.length > 0) { + if (targetPVDevices.length > 1) return _("new LVM volume group"); + + if (targetPVDevices.length === 1) { + // TRANSLATORS: %s is the disk used for the LVM physical volumes (eg. "/dev/sda, 80 GiB) + return sprintf(_("new LVM volume group on %s"), deviceLabel(targetPVDevices[0])); + } + } + + return _("No device selected yet"); +}; + +/** + * Field description. + * @function + * + * @returns {React.ReactElement} + */ +const renderDescription = () => ( + LVM \ +Volume Group for installation.") + }} + /> +); + +/** + * Allows to select the installation device. + * @component + * + * @typedef {object} InstallationDeviceFieldProps + * @property {ProposalTarget|undefined} target - Installation target + * @property {StorageDevice|undefined} targetDevice - Target device (for target "DISK"). + * @property {StorageDevice[]} targetPVDevices - Target devices for the LVM volume group (target "NEW_LVM_VG"). + * @property {StorageDevice[]} devices - Available devices for installation. + * @property {boolean} isLoading + * @property {(target: Target) => void} onChange + * + * @typedef {object} Target + * @property {ProposalTarget} target + * @property {StorageDevice|undefined} targetDevice + * @property {StorageDevice[]} targetPVDevices + * + * @param {InstallationDeviceFieldProps} props + */ + +export default function InstallationDeviceField({ + target, + targetDevice, + targetPVDevices, + devices, + isLoading, + onChange +}) { + const [isDialogOpen, setIsDialogOpen] = useState(false); + + const openDialog = () => setIsDialogOpen(true); + + const closeDialog = () => setIsDialogOpen(false); + + const onAccept = ({ target, targetDevice, targetPVDevices }) => { + closeDialog(); + onChange({ target, targetDevice, targetPVDevices }); + }; + + let value; + if (isLoading || !target) + value = ; + else + value = targetValue(target, targetDevice, targetPVDevices); + + return ( + + + } + /> + + ); +} diff --git a/web/src/components/storage/InstallationDeviceField.test.jsx b/web/src/components/storage/InstallationDeviceField.test.jsx new file mode 100644 index 0000000000..41a9932402 --- /dev/null +++ b/web/src/components/storage/InstallationDeviceField.test.jsx @@ -0,0 +1,212 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import InstallationDeviceField from "~/components/storage/InstallationDeviceField"; + +/** + * @typedef {import ("~/components/storage/InstallationDeviceField").InstallationDeviceFieldProps} InstallationDeviceFieldProps + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + */ + +/** @type {StorageDevice} */ +const sda = { + sid: 59, + isDrive: true, + type: "disk", + description: "", + vendor: "Micron", + model: "Micron 1100 SATA", + driver: ["ahci", "mmcblk"], + bus: "IDE", + busId: "", + transport: "usb", + dellBOSS: false, + sdCard: true, + active: true, + name: "/dev/sda", + size: 1024, + recoverableSize: 0, + systems : [], + udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"], + udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], +}; + +/** @type {StorageDevice} */ +const sdb = { + sid: 62, + isDrive: true, + type: "disk", + description: "", + vendor: "Samsung", + model: "Samsung Evo 8 Pro", + driver: ["ahci"], + bus: "IDE", + busId: "", + transport: "", + dellBOSS: false, + sdCard: false, + active: true, + name: "/dev/sdb", + size: 2048, + recoverableSize: 0, + systems : [], + udevIds: [], + udevPaths: ["pci-0000:00-19"] +}; + +/** @type {InstallationDeviceFieldProps} */ +let props; + +beforeEach(() => { + props = { + target: "DISK", + targetDevice: sda, + targetPVDevices: [], + devices: [sda, sdb], + isLoading: false, + onChange: jest.fn() + }; +}); + +describe("when set as loading", () => { + beforeEach(() => { + props.isLoading = true; + }); + + it("renders a loading hint", () => { + plainRender(); + screen.getByText("Waiting for information about selected device"); + }); +}); + +describe("when the target is a disk", () => { + beforeEach(() => { + props.target = "DISK"; + }); + + describe("and installation device is not selected yet", () => { + beforeEach(() => { + props.targetDevice = undefined; + }); + + it("uses a 'No device selected yet' text for the selection button", async () => { + plainRender(); + screen.getByText("No device selected yet"); + }); + }); + + describe("and an installation device is selected", () => { + beforeEach(() => { + props.targetDevice = sda; + }); + + it("uses its name as part of the text for the selection button", async () => { + plainRender(); + screen.getByText(/\/dev\/sda/); + }); + }); +}); + +describe("when the target is a new LVM volume group", () => { + beforeEach(() => { + props.target = "NEW_LVM_VG"; + }); + + describe("and the target devices are not selected yet", () => { + beforeEach(() => { + props.targetPVDevices = []; + }); + + it("uses a 'No device selected yet' text for the selection button", async () => { + plainRender(); + screen.getByText("No device selected yet"); + }); + }); + + describe("and there is a selected device", () => { + beforeEach(() => { + props.targetPVDevices = [sda]; + }); + + it("uses its name as part of the text for the selection button", async () => { + plainRender(); + screen.getByText(/new LVM .* \/dev\/sda/); + }); + }); + + describe("and there are more than one selected device", () => { + beforeEach(() => { + props.targetPVDevices = [sda, sdb]; + }); + + it("does not use the names as part of the text for the selection button", async () => { + plainRender(); + screen.getByText("new LVM volume group"); + }); + }); +}); + +it("allows changing the selected device", async () => { + const { user } = plainRender(); + const button = screen.getByRole("button", { name: /installation device/i }); + + await user.click(button); + + const selector = await screen.findByRole("dialog", { name: /Device for installing/ }); + const diskGrid = within(selector).getByRole("grid", { name: /target disk/ }); + const sdbRow = within(diskGrid).getByRole("row", { name: /sdb/ }); + const sdbOption = within(sdbRow).getByRole("radio"); + const accept = within(selector).getByRole("button", { name: "Confirm" }); + + await user.click(sdbOption); + await user.click(accept); + + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(props.onChange).toHaveBeenCalledWith({ + target: "DISK", + targetDevice: sdb, + targetPVDevices: [] + }); +}); + +it("allows canceling a device selection", async () => { + const { user } = plainRender(); + const button = screen.getByRole("button", { name: /installation device/i }); + + await user.click(button); + + const selector = await screen.findByRole("dialog", { name: /Device for installing/ }); + const diskGrid = within(selector).getByRole("grid", { name: /target disk/ }); + const sdbRow = within(diskGrid).getByRole("row", { name: /sdb/ }); + const sdbOption = within(sdbRow).getByRole("radio"); + const cancel = within(selector).getByRole("button", { name: "Cancel" }); + + await user.click(sdbOption); + await user.click(cancel); + + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(props.onChange).not.toHaveBeenCalled(); +}); diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 96021a8a24..24d8c371d4 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -168,7 +168,7 @@ beforeEach(() => { onStatusChange: jest.fn() }; - // @ts-expect-error Mocking method does not exist fo InstallerClient type. + // @ts-expect-error Mocking method does not exist for InstallerClient type. createClient.mockImplementation(() => ({ storage })); }); diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 79bd9caa87..6839c8a6ea 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -25,15 +25,17 @@ import React, { useEffect, useState } from "react"; import { Checkbox, Form, Skeleton, Switch, Tooltip } from "@patternfly/react-core"; import { _ } from "~/i18n"; +import InstallationDeviceField from "~/components/storage/InstallationDeviceField"; import SpacePolicyField from "~/components/storage/SpacePolicyField"; import PartitionsField from "~/components/storage/PartitionsField"; import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core"; import { Icon } from "~/components/layout"; -import { noop } from "~/utils"; +import { compact, noop } from "~/utils"; import { SPACE_POLICIES } from "~/components/storage/utils"; /** * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings + * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget * @typedef {import ("~/client/storage").SpaceAction} SpaceAction * @typedef {import ("~/components/storage/utils").SpacePolicy} SpacePolicy * @typedef {import ("~/client/storage").StorageDevice} StorageDevice @@ -237,6 +239,17 @@ export default function ProposalSettingsSection({ isLoading = false, onChange }) { + /** + * @param {import("~/components/storage/InstallationDeviceField").Target} targetOptions + */ + const changeTarget = ({ target, targetDevice, targetPVDevices }) => { + onChange({ + target, + targetDevice: targetDevice?.name, + targetPVDevices: targetPVDevices.map(d => d.name) + }); + }; + const changeEncryption = ({ password, method }) => { onChange({ encryptionPassword: password, encryptionMethod: method }); }; @@ -259,11 +272,20 @@ export default function ProposalSettingsSection({ }); }; - const targetDevice = availableDevices.find(d => d.name === settings.targetDevice); + /** + * @param {string} name + * @returns {StorageDevice|undefined} + */ + const findDevice = (name) => availableDevices.find(a => a.name === name); + + /** @type {StorageDevice|undefined} */ + const targetDevice = findDevice(settings.targetDevice); + /** @type {StorageDevice[]} */ + const targetPVDevices = compact(settings.targetPVDevices?.map(findDevice) || []); const useEncryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0; const { volumes = [], installationDevices = [], spaceActions = [] } = settings; - const bootDevice = availableDevices.find(d => d.name === settings.bootDevice); - const defaultBootDevice = availableDevices.find(d => d.name === settings.defaultBootDevice); + const bootDevice = findDevice(settings.bootDevice); + const defaultBootDevice = findDevice(settings.defaultBootDevice); const spacePolicy = SPACE_POLICIES.find(p => p.id === settings.spacePolicy); // Templates for already existing mount points are filtered out @@ -277,6 +299,14 @@ export default function ProposalSettingsSection({ return ( <>
+ { }; }); +/** @type {StorageDevice} */ +const sda = { + sid: 59, + isDrive: true, + type: "disk", + description: "", + vendor: "Micron", + model: "Micron 1100 SATA", + driver: ["ahci", "mmcblk"], + bus: "IDE", + busId: "", + transport: "usb", + dellBOSS: false, + sdCard: true, + active: true, + name: "/dev/sda", + size: 1024, + recoverableSize: 0, + systems : [], + udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"], + udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], +}; + +/** @type {StorageDevice} */ +const sdb = { + sid: 62, + isDrive: true, + type: "disk", + description: "", + vendor: "Samsung", + model: "Samsung Evo 8 Pro", + driver: ["ahci"], + bus: "IDE", + busId: "", + transport: "", + dellBOSS: false, + sdCard: false, + active: true, + name: "/dev/sdb", + size: 2048, + recoverableSize: 0, + systems : [], + udevIds: [], + udevPaths: ["pci-0000:00-19"] +}; + /** @type {Volume} */ let volume; @@ -71,6 +117,7 @@ beforeEach(() => { props = { settings: { target: "DISK", + targetDevice: "/dev/sda", targetPVDevices: [], configureBoot: false, bootDevice: "", @@ -80,7 +127,7 @@ beforeEach(() => { spacePolicy: "", spaceActions: [], volumes: [], - installationDevices: [] + installationDevices: [sda, sdb] }, availableDevices: [], encryptionMethods: [], @@ -89,7 +136,13 @@ beforeEach(() => { }; }); -const rootVolume = { mountPath: "/", fsType: "Btrfs", outline: { snapshotsConfigurable: true } }; +it("allows changing the selected device", async () => { + const { user } = plainRender(); + const button = screen.getByRole("button", { name: /installation device/i }); + + await user.click(button); + await screen.findByRole("dialog", { name: /Device for installing/ }); +}); describe("when snapshots are configurable", () => { beforeEach(() => { From e0910bf7283759d781679f35a3d3b354d46438ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 13:29:15 +0100 Subject: [PATCH 10/31] web: Remove ProposalDeviceSection --- .../storage/ProposalDeviceSection.jsx | 198 --------------- .../storage/ProposalDeviceSection.test.jsx | 233 ------------------ web/src/components/storage/ProposalPage.jsx | 7 - web/src/components/storage/index.js | 1 - 4 files changed, 439 deletions(-) delete mode 100644 web/src/components/storage/ProposalDeviceSection.jsx delete mode 100644 web/src/components/storage/ProposalDeviceSection.test.jsx diff --git a/web/src/components/storage/ProposalDeviceSection.jsx b/web/src/components/storage/ProposalDeviceSection.jsx deleted file mode 100644 index a076529db2..0000000000 --- a/web/src/components/storage/ProposalDeviceSection.jsx +++ /dev/null @@ -1,198 +0,0 @@ -/* - * Copyright (c) [2024] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -// @ts-check - -import React, { useState } from "react"; -import { - Button, - Skeleton, -} from "@patternfly/react-core"; - -import { _ } from "~/i18n"; -import { DeviceSelectionDialog } from "~/components/storage"; -import { deviceLabel } from '~/components/storage/utils'; -import { If, Section } from "~/components/core"; -import { sprintf } from "sprintf-js"; -import { compact, noop } from "~/utils"; - -/** - * @typedef {import ("~/client/storage").ProposalTarget} ProposalTarget - * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings - * @typedef {import ("~/client/storage").StorageDevice} StorageDevice - */ - -/** - * Renders a button that allows changing the target device for installation. - * - * @param {object} props - * @param {ProposalTarget} props.target - * @param {StorageDevice|undefined} props.targetDevice - * @param {StorageDevice[]} props.targetPVDevices - * @param {import("react").MouseEventHandler} [props.onClick=noop] - */ -const TargetDeviceButton = ({ target, targetDevice, targetPVDevices, onClick = noop }) => { - const label = () => { - if (target === "DISK" && targetDevice) return deviceLabel(targetDevice); - if (target === "NEW_LVM_VG" && targetPVDevices.length > 0) { - if (targetPVDevices.length > 1) return _("new LVM volume group"); - - if (targetPVDevices.length === 1) { - // TRANSLATORS: %s is the disk used for the LVM physical volumes (eg. "/dev/sda, 80 GiB) - return sprintf(_("new LVM volume group on %s"), deviceLabel(targetPVDevices[0])); - } - } - - return _("No device selected yet"); - }; - - return ( - - ); -}; - -/** - * Allows to select the installation device. - * @component - * - * @param {object} props - * @param {ProposalTarget} props.target - Installation target - * @param {StorageDevice|undefined} props.targetDevice - Target device (for target "DISK"). - * @param {StorageDevice[]} props.targetPVDevices - Target devices for the LVM volume group (target "NEW_LVM_VG"). - * @param {StorageDevice[]} props.devices - Available devices for installation. - * @param {boolean} props.isLoading - * @param {(target: Target) => void} props.onChange - * - * @typedef {object} Target - * @property {ProposalTarget} target - * @property {StorageDevice|undefined} targetDevice - * @property {StorageDevice[]} targetPVDevices - */ -const InstallationDeviceField = ({ - target, - targetDevice, - targetPVDevices, - devices, - isLoading, - onChange -}) => { - const [isDialogOpen, setIsDialogOpen] = useState(false); - - const openDialog = () => setIsDialogOpen(true); - - const closeDialog = () => setIsDialogOpen(false); - - const onAccept = ({ target, targetDevice, targetPVDevices }) => { - closeDialog(); - onChange({ target, targetDevice, targetPVDevices }); - }; - - if (isLoading) { - return ; - } - - return ( -
- {_("Installation device")} - - - } - /> -
- ); -}; - -/** - * Section for editing the target device for installation. - * @component - * - * @param {object} props - * @param {ProposalSettings} props.settings - * @param {StorageDevice[]} [props.availableDevices=[]] - * @param {boolean} [props.isLoading=false] - * @param {(settings: object) => void} [props.onChange=noop] - */ -export default function ProposalDeviceSection({ - settings, - availableDevices = [], - isLoading = false, - onChange = noop -}) { - const findDevice = (name) => availableDevices.find(a => a.name === name); - - const target = settings.target; - const targetDevice = findDevice(settings.targetDevice); - const targetPVDevices = compact(settings.targetPVDevices?.map(findDevice) || []); - - const changeTarget = ({ target, targetDevice, targetPVDevices }) => { - onChange({ - target, - targetDevice: targetDevice?.name, - targetPVDevices: targetPVDevices.map(d => d.name) - }); - }; - - const Description = () => ( - LVM \ -Volume Group for installation.") - }} - /> - ); - - return ( -
} - > - -
- ); -} diff --git a/web/src/components/storage/ProposalDeviceSection.test.jsx b/web/src/components/storage/ProposalDeviceSection.test.jsx deleted file mode 100644 index bdbab62055..0000000000 --- a/web/src/components/storage/ProposalDeviceSection.test.jsx +++ /dev/null @@ -1,233 +0,0 @@ -/* - * Copyright (c) [2024] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -// @ts-check - -import React from "react"; -import { screen, within } from "@testing-library/react"; -import { plainRender } from "~/test-utils"; -import { ProposalDeviceSection } from "~/components/storage"; - -const sda = { - sid: 59, - isDrive: true, - type: "disk", - vendor: "Micron", - model: "Micron 1100 SATA", - driver: ["ahci", "mmcblk"], - bus: "IDE", - busId: "", - transport: "usb", - dellBOSS: false, - sdCard: true, - active: true, - name: "/dev/sda", - size: 1024, - recoverableSize: 0, - systems : [], - udevIds: ["ata-Micron_1100_SATA_512GB_12563", "scsi-0ATA_Micron_1100_SATA_512GB"], - udevPaths: ["pci-0000:00-12", "pci-0000:00-12-ata"], -}; - -const sdb = { - sid: 62, - isDrive: true, - type: "disk", - vendor: "Samsung", - model: "Samsung Evo 8 Pro", - driver: ["ahci"], - bus: "IDE", - busId: "", - transport: "", - dellBOSS: false, - sdCard: false, - active: true, - name: "/dev/sdb", - size: 2048, - recoverableSize: 0, - systems : [], - udevIds: [], - udevPaths: ["pci-0000:00-19"] -}; - -let props; - -describe("ProposalDeviceSection", () => { - beforeEach(() => { - props = { - settings: { - target: "DISK", - targetDevice: "/dev/sda", - }, - availableDevices: [sda, sdb], - isLoading: false, - onChange: jest.fn() - }; - }); - - describe("Installation device field", () => { - describe("when set as loading", () => { - beforeEach(() => { - props.isLoading = true; - }); - - describe("and selected device is not defined yet", () => { - beforeEach(() => { - props.settings.target = undefined; - }); - - it("renders a loading hint", () => { - plainRender(); - screen.getByText("Waiting for information about selected device"); - }); - }); - }); - - describe("when the target is a disk", () => { - beforeEach(() => { - props.settings.target = "DISK"; - }); - - describe("and installation device is not selected yet", () => { - beforeEach(() => { - props.settings.targetDevice = ""; - }); - - it("uses a 'No device selected yet' text for the selection button", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: "No device selected yet" }); - - await user.click(button); - - screen.getByRole("dialog", { name: /Device for installing/i }); - }); - }); - - describe("and an installation device is selected", () => { - beforeEach(() => { - props.settings.targetDevice = "/dev/sda"; - }); - - it("uses its name as part of the text for the selection button", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: /\/dev\/sda/ }); - - await user.click(button); - - screen.getByRole("dialog", { name: /Device for installing/i }); - }); - }); - }); - - describe("when the target is a new LVM volume group", () => { - beforeEach(() => { - props.settings.target = "NEW_LVM_VG"; - }); - - describe("and the target devices are not selected yet", () => { - beforeEach(() => { - props.settings.targetPVDevices = []; - }); - - it("uses a 'No device selected yet' text for the selection button", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: "No device selected yet" }); - - await user.click(button); - - screen.getByRole("dialog", { name: /Device for installing/i }); - }); - }); - - describe("and there is a selected device", () => { - beforeEach(() => { - props.settings.targetPVDevices = ["/dev/sda"]; - }); - - it("uses its name as part of the text for the selection button", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: /new LVM .* \/dev\/sda/ }); - - await user.click(button); - - screen.getByRole("dialog", { name: /Device for installing/i }); - }); - }); - - describe("and there are more than one selected device", () => { - beforeEach(() => { - props.settings.targetPVDevices = ["/dev/sda", "/dev/sdb"]; - }); - - it("does not use the names as part of the text for the selection button", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: "new LVM volume group" }); - - await user.click(button); - - screen.getByRole("dialog", { name: /Device for installing/i }); - }); - }); - }); - - it("allows changing the selected device", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" }); - - await user.click(button); - - const selector = await screen.findByRole("dialog", { name: /Device for installing/ }); - const diskGrid = within(selector).getByRole("grid", { name: /target disk/ }); - const sdbRow = within(diskGrid).getByRole("row", { name: /sdb/ }); - const sdbOption = within(sdbRow).getByRole("radio"); - const accept = within(selector).getByRole("button", { name: "Confirm" }); - - await user.click(sdbOption); - await user.click(accept); - - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).toHaveBeenCalledWith({ - target: "DISK", - targetDevice: sdb.name, - targetPVDevices: [] - }); - }); - - it("allows canceling a device selection", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: "/dev/sda, 1 KiB" }); - - await user.click(button); - - const selector = await screen.findByRole("dialog", { name: /Device for installing/ }); - const diskGrid = within(selector).getByRole("grid", { name: /target disk/ }); - const sdbRow = within(diskGrid).getByRole("row", { name: /sdb/ }); - const sdbOption = within(sdbRow).getByRole("radio"); - const cancel = within(selector).getByRole("button", { name: "Cancel" }); - - await user.click(sdbOption); - await user.click(cancel); - - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index 6123835fae..b2c4169566 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -27,7 +27,6 @@ import { toValidationError, useCancellablePromise } from "~/utils"; import { Page } from "~/components/core"; import { ProposalPageMenu, - ProposalDeviceSection, ProposalTransactionalInfo, ProposalSettingsSection, ProposalResultSection @@ -230,12 +229,6 @@ export default function ProposalPage() { - Date: Fri, 12 Apr 2024 11:48:22 +0100 Subject: [PATCH 11/31] web: move storage/EncryptionField to its own file Adapting it to use a core/Field/SettingsField component. Still pending to extract the storage/EncryptionSettingsDialog and also to add there a switch to make possible enable/disable the encryption. --- .../components/storage/EncryptionField.jsx | 187 +++++++++++++++++ .../storage/EncryptionField.test.jsx | 86 ++++++++ .../components/storage/PartitionsField.jsx | 4 +- .../storage/ProposalSettingsSection.jsx | 189 +----------------- .../storage/ProposalSettingsSection.test.jsx | 147 -------------- 5 files changed, 281 insertions(+), 332 deletions(-) create mode 100644 web/src/components/storage/EncryptionField.jsx create mode 100644 web/src/components/storage/EncryptionField.test.jsx diff --git a/web/src/components/storage/EncryptionField.jsx b/web/src/components/storage/EncryptionField.jsx new file mode 100644 index 0000000000..9833018ec8 --- /dev/null +++ b/web/src/components/storage/EncryptionField.jsx @@ -0,0 +1,187 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React, { useEffect, useState } from "react"; +import { Checkbox, Form, Skeleton } from "@patternfly/react-core"; +import { _ } from "~/i18n"; +import { noop } from "~/utils"; +import { If, SettingsField, PasswordAndConfirmationInput, Popup } from "~/components/core"; +import { EncryptionMethods } from "~/client/storage"; + +/** + * @typedef {import ("~/client/storage").StorageDevice} StorageDevice + */ + +const LABEL = _("Encryption"); +const DESCRIPTION = _("Full disk encryption allows to protect the information stored at \ +the device, including data, programs, and system files."); + +/** + * Form for configuring the encryption password. + * @component + * + * @todo: improve typechecking for method and methods + * + * @param {object} props + * @param {string} props.id - Form ID. + * @param {string} props.password - Password for encryption. + * @param {string} props.method - Encryption method. + * @param {string[]} props.methods - Possible encryption methods. + * @param {(password: string, method: string) => void} [props.onSubmit=noop] - On submit callback. + * @param {(valid: boolean) => void} [props.onValidate=noop] - On validate callback. + */ +const EncryptionSettingsForm = ({ + id, + password: passwordProp, + method: methodProp, + methods, + onSubmit = noop, + onValidate = noop +}) => { + const [password, setPassword] = useState(passwordProp || ""); + const [method, setMethod] = useState(methodProp); + + useEffect(() => { + if (password.length === 0) onValidate(false); + }, [password, onValidate]); + + const changePassword = (_, v) => setPassword(v); + + const changeMethod = (_, value) => { + const newMethod = value ? EncryptionMethods.TPM : EncryptionMethods.LUKS2; + setMethod(newMethod); + }; + + const submitForm = (e) => { + e.preventDefault(); + onSubmit(password, method); + }; + + const Description = () => ( + TPM can verify the integrity of the system. TPM sealing requires the new system to be booted directly on its first run.") + }} + /> + ); + + return ( +
+ + } + isChecked={method === EncryptionMethods.TPM} + onChange={changeMethod} + /> + } + /> + + ); +}; + +/** + * Allows to define encryption + * @component + * + * @typedef {object} EncryptionConfig + * @property {string} password + * @property {string} [method] + * + * @typedef {object} EncryptionFieldProps + * @property {string} [password=""] - Password for encryption + * @property {string} [method=""] - Encryption method + * @property {string[]} [methods=[]] - Possible encryption methods + * @property {boolean} [isLoading=false] - Whether to show the selector as loading + * @property {(config: EncryptionConfig) => void} [onChange=noop] - On change callback + * + * @param {EncryptionFieldProps} props + */ +export default function EncryptionField({ + password = "", + method = "", + // FIXME: should be available methods actually a prop? + methods = [], + isLoading = false, + onChange = noop +}) { + const [isFormOpen, setIsFormOpen] = useState(false); + const [isFormValid, setIsFormValid] = useState(true); + + const openForm = () => setIsFormOpen(true); + + const closeForm = () => setIsFormOpen(false); + + const acceptForm = (newPassword, newMethod) => { + closeForm(); + onChange({ password: newPassword, method: newMethod }); + }; + + const validateForm = (valid) => setIsFormValid(valid); + + if (isLoading) return ; + + // FIXME: extract to the top to avoid redefinitions? + const FieldValue = () => { + if (isLoading) return ; + + if (!password || password === "") return _("disabled"); + if (method === EncryptionMethods.LUKS2) return _("enabled"); + if (method === EncryptionMethods.TPM) return _("enabled using TPM"); + }; + + return ( + } + onClick={openForm} + > + + + + {_("Accept")} + + + + + ); +} diff --git a/web/src/components/storage/EncryptionField.test.jsx b/web/src/components/storage/EncryptionField.test.jsx new file mode 100644 index 0000000000..2a1351ee04 --- /dev/null +++ b/web/src/components/storage/EncryptionField.test.jsx @@ -0,0 +1,86 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { EncryptionMethods } from "~/client/storage"; +import EncryptionField from "~/components/storage/EncryptionField"; + +let props; +const onChangeFn = jest.fn(); + +describe("Encryption field", () => { + beforeEach(() => { + props = { onChange: onChangeFn }; + }); + + it("renders proper value depending of encryption status", () => { + // No encryption set + const { rerender } = plainRender(); + screen.getByText("disabled"); + + // Encryption set with LUKS2 + rerender(); + screen.getByText("enabled"); + + // Encryption set with TPM + rerender(); + screen.getByText("enabled using TPM"); + }); + + it("allows changing the encryption settings", async () => { + const { user } = plainRender(); + + const button = screen.getByRole("button", { name: /Encryption/ }); + await user.click(button); + + const popup = await screen.findByRole("dialog"); + screen.getByText("Encryption settings"); + const passwordInput = screen.getByLabelText("Password"); + const passwordConfirmInput = screen.getByLabelText("Password confirmation"); + const accept = within(popup).getByRole("button", { name: "Accept" }); + await user.type(passwordInput, "1234"); + await user.type(passwordConfirmInput, "1234"); + await user.click(accept); + + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(props.onChange).toHaveBeenCalled(); + }); + + it("allows closing the encryption settings without triggering changes", async () => { + const { user } = plainRender(); + + const button = screen.getByRole("button", { name: /Encryption settings/ }); + await user.click(button); + + const popup = await screen.findByRole("dialog"); + screen.getByText("Encryption settings"); + + const cancel = within(popup).getByRole("button", { name: "Cancel" }); + await user.click(cancel); + + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(props.onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index ff1a6b5560..64d3047ea8 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -538,7 +538,7 @@ const Basic = ({ volumes, configureBoot, bootDevice, target, isLoading }) => { return (
- { volumes.map((v, i) => ) } + {volumes.map((v, i) => )}
); @@ -630,7 +630,7 @@ const Advanced = ({ * @property {TODO} defaultBootDevice - TODO * @property {boolean} [isLoading=false] - Whether to show the content as loading * @property {(volumes: Volume[]) => void} onVolumesChange - Function to use for changing the volumes - * @property {(volumes: Volume[]) => void} onBootChange - Function for changing the boot settings + * @property {TODO} onBootChange - Function for changing the boot settings * * @param {PartitionsFieldProps} props */ diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 6839c8a6ea..0f7cc869ee 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -21,17 +21,15 @@ // @ts-check -import React, { useEffect, useState } from "react"; -import { Checkbox, Form, Skeleton, Switch, Tooltip } from "@patternfly/react-core"; - +import React from "react"; import { _ } from "~/i18n"; +import { compact } from "~/utils"; +import { Section } from "~/components/core"; +import { SPACE_POLICIES } from "~/components/storage/utils"; +import EncryptionField from "~/components/storage/EncryptionField"; import InstallationDeviceField from "~/components/storage/InstallationDeviceField"; -import SpacePolicyField from "~/components/storage/SpacePolicyField"; import PartitionsField from "~/components/storage/PartitionsField"; -import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core"; -import { Icon } from "~/components/layout"; -import { compact, noop } from "~/utils"; -import { SPACE_POLICIES } from "~/components/storage/utils"; +import SpacePolicyField from "~/components/storage/SpacePolicyField"; /** * @typedef {import ("~/client/storage").ProposalSettings} ProposalSettings @@ -42,181 +40,6 @@ import { SPACE_POLICIES } from "~/components/storage/utils"; * @typedef {import ("~/client/storage").Volume} Volume */ -/** - * Form for configuring the encryption password. - * @component - * - * @param {object} props - * @param {string} props.id - Form ID. - * @param {string} props.password - Password for encryption. - * @param {string} props.method - Encryption method. - * @param {string[]} props.methods - Possible encryption methods. - * @param {(password: string, method: string) => void} [props.onSubmit=noop] - On submit callback. - * @param {(valid: boolean) => void} [props.onValidate=noop] - On validate callback. - */ -const EncryptionSettingsForm = ({ - id, - password: passwordProp, - method: methodProp, - methods, - onSubmit = noop, - onValidate = noop -}) => { - const [password, setPassword] = useState(passwordProp || ""); - const [method, setMethod] = useState(methodProp); - const tpmId = "tpm_fde"; - const luks2Id = "luks2"; - - useEffect(() => { - if (password.length === 0) onValidate(false); - }, [password, onValidate]); - - const changePassword = (_, v) => setPassword(v); - - const changeMethod = (_, value) => { - value ? setMethod(tpmId) : setMethod(luks2Id); - }; - - const submitForm = (e) => { - e.preventDefault(); - onSubmit(password, method); - }; - - const Description = () => ( - TPM can verify the integrity of the system. TPM sealing requires the new system to be booted directly on its first run.") - }} - /> - ); - - return ( -
- - } - isChecked={method === tpmId} - onChange={changeMethod} - /> - } - /> - - ); -}; - -/** - * Allows to define encryption - * @component - * - * @param {object} props - * @param {string} [props.password=""] - Password for encryption - * @param {string} [props.method=""] - Encryption method - * @param {string[]} [props.methods] - Possible encryption methods - * @param {boolean} [props.isChecked=false] - Whether encryption is selected - * @param {boolean} [props.isLoading=false] - Whether to show the selector as loading - * @param {(config: EncryptionConfig) => void} [props.onChange=noop] - On change callback - * - * @typedef {object} EncryptionConfig - * @property {string} password - * @property {string} [method] - */ -const EncryptionField = ({ - password = "", - method = "", - methods, - isChecked: defaultIsChecked = false, - isLoading = false, - onChange = noop -}) => { - const [isChecked, setIsChecked] = useState(defaultIsChecked); - const [isFormOpen, setIsFormOpen] = useState(false); - const [isFormValid, setIsFormValid] = useState(true); - - const openForm = () => setIsFormOpen(true); - - const closeForm = () => setIsFormOpen(false); - - const acceptForm = (newPassword, newMethod) => { - closeForm(); - onChange({ password: newPassword, method: newMethod }); - }; - - const cancelForm = () => { - setIsChecked(defaultIsChecked); - closeForm(); - }; - - const validateForm = (valid) => setIsFormValid(valid); - - const changeSelected = (_, value) => { - setIsChecked(value); - - if (value && password.length === 0) openForm(); - - if (!value) { - onChange({ password: "" }); - } - }; - - const ChangeSettingsButton = () => { - return ( - - - - ); - }; - - if (isLoading) return ; - - return ( - <> -
- - {isChecked && } -
- - - - {_("Accept")} - - - - - ); -}; - /** * Section for editing the proposal settings * @component diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx index 33b43df27e..7aa0da0c36 100644 --- a/web/src/components/storage/ProposalSettingsSection.test.jsx +++ b/web/src/components/storage/ProposalSettingsSection.test.jsx @@ -188,150 +188,3 @@ it("requests a volume change when onChange callback is triggered", async () => { { volumes: expect.any(Array) } ); }); - -describe("Encryption field", () => { - describe.skip("if encryption password setting is not set yet", () => { - beforeEach(() => { - // Currently settings cannot be undefined. - props.settings = undefined; - }); - - it("does not render the encryption switch", () => { - plainRender(); - - expect(screen.queryByLabelText("Use encryption")).toBeNull(); - }); - }); - - describe("if encryption password setting is set", () => { - beforeEach(() => { - props.settings.encryptionPassword = ""; - }); - - it("renders the encryption switch", () => { - plainRender(); - - screen.getByRole("checkbox", { name: "Use encryption" }); - }); - }); - - describe("if encryption password is not empty", () => { - beforeEach(() => { - props.settings.encryptionPassword = "1234"; - }); - - it("renders the encryption switch as selected", () => { - plainRender(); - - const checkbox = screen.getByRole("checkbox", { name: "Use encryption" }); - expect(checkbox).toBeChecked(); - }); - - it("renders a button for changing the encryption settings", () => { - plainRender(); - - screen.getByRole("button", { name: /Encryption settings/ }); - }); - - it("changes the selection on click", async () => { - const { user } = plainRender(); - - const checkbox = screen.getByRole("checkbox", { name: "Use encryption" }); - await user.click(checkbox); - - expect(checkbox).not.toBeChecked(); - expect(props.onChange).toHaveBeenCalled(); - }); - - it("allows changing the encryption settings when clicking on the settings button", async () => { - const { user } = plainRender(); - - const button = screen.getByRole("button", { name: /Encryption settings/ }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - screen.getByText("Encryption settings"); - - const accept = within(popup).getByRole("button", { name: "Accept" }); - await user.click(accept); - - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).toHaveBeenCalled(); - }); - - it("allows canceling the changes of the encryption settings", async () => { - const { user } = plainRender(); - - const button = screen.getByRole("button", { name: /Encryption settings/ }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - screen.getByText("Encryption settings"); - - const cancel = within(popup).getByRole("button", { name: "Cancel" }); - await user.click(cancel); - - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).not.toHaveBeenCalled(); - }); - }); - - describe("if encryption password is empty", () => { - beforeEach(() => { - props.settings.encryptionPassword = ""; - }); - - it("renders the encryption switch as not selected", () => { - plainRender(); - - const checkbox = screen.getByRole("checkbox", { name: "Use encryption" }); - expect(checkbox).not.toBeChecked(); - }); - - it("does not render a button for changing the encryption settings", () => { - plainRender(); - - const button = screen.queryByRole("button", { name: /Encryption settings/ }); - expect(button).toBeNull(); - }); - - it("changes the selection and allows changing the settings on click", async () => { - const { user } = plainRender(); - - const checkbox = screen.getByRole("checkbox", { name: "Use encryption" }); - await user.click(checkbox); - - const popup = await screen.findByRole("dialog"); - screen.getByText("Encryption settings"); - - const passwordInput = screen.getByLabelText("Password"); - const passwordConfirmInput = screen.getByLabelText("Password confirmation"); - await user.type(passwordInput, "1234"); - await user.type(passwordConfirmInput, "1234"); - const accept = within(popup).getByRole("button", { name: "Accept" }); - await user.click(accept); - - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - - expect(props.onChange).toHaveBeenCalled(); - expect(checkbox).toBeChecked(); - }); - - it("does not select encryption if the settings are canceled", async () => { - const { user } = plainRender(); - - const checkbox = screen.getByRole("checkbox", { name: "Use encryption" }); - await user.click(checkbox); - - const popup = await screen.findByRole("dialog"); - screen.getByText("Encryption settings"); - - const cancel = within(popup).getByRole("button", { name: "Cancel" }); - await user.click(cancel); - - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).not.toHaveBeenCalled(); - expect(checkbox).not.toBeChecked(); - }); - }); -}); From a0d09c2402a9c6a2dcb055c0aa160afafdef7f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 15:18:15 +0100 Subject: [PATCH 12/31] web: Add link for activating disks --- web/src/assets/styles/blocks.scss | 3 ++ web/src/components/core/PageMenu.jsx | 41 ++++++++++++------- .../storage/InstallationDeviceField.jsx | 9 +++- .../components/storage/ProposalPageMenu.jsx | 13 ++++-- 4 files changed, 47 insertions(+), 19 deletions(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index f03405fcaa..46c38c4374 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -719,6 +719,9 @@ section [data-type="agama/reminder"] { } } +[data-type="agama/field"] button.pf-v5-c-menu-toggle.pf-m-plain { + padding: 0; +} [data-type="agama/expandable-selector"] { // The expandable selector is built on top of PF/Table#expandable diff --git a/web/src/components/core/PageMenu.jsx b/web/src/components/core/PageMenu.jsx index ac4c3f432a..608c9a0384 100644 --- a/web/src/components/core/PageMenu.jsx +++ b/web/src/components/core/PageMenu.jsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2023] SUSE LLC + * Copyright (c) [2023-2024] SUSE LLC * * All Rights Reserved. * @@ -19,6 +19,8 @@ * find current contact information at www.suse.com. */ +// @ts-check + import React, { useState } from 'react'; import { Dropdown, DropdownGroup, DropdownItem, DropdownList, @@ -31,18 +33,23 @@ import { Icon } from "~/components/layout"; * Internal component to build the {PageMenu} toggler * @component * - * @param {object} props - * @param {string} [props.aria-label="Show page menu"] - * @param {function} props.onClick + * @typedef {object} TogglerBaseProps + * @property {React.Ref} toggleRef + * @property {string} label + * + * @typedef {TogglerBaseProps & import('@patternfly/react-core').MenuToggleProps} TogglerProps + * + * @param {TogglerProps} props */ -const Toggler = ({ toggleRef, onClick, "aria-label": ariaLabel = _(("Show page menu")) }) => { +const Toggler = ({ toggleRef, label, onClick, "aria-label": ariaLabel = _(("Show page menu")) }) => { return ( + {label} ); @@ -54,9 +61,9 @@ const Toggler = ({ toggleRef, onClick, "aria-label": ariaLabel = _(("Show page m * * Built on top of {@link https://www.patternfly.org/components/menus/dropdown#dropdowngroup PF/DropdownGroup} * - * @see {PageMenu } examples. + * @see {PageMenu} examples. * - * @param {object} props - PF/DropdownGroup props, See {@link https://www.patternfly.org/components/menus/dropdown#dropdowngroup} + * @param {import('@patternfly/react-core').DropdownGroupProps} props */ const Group = ({ children, ...props }) => { return ( @@ -74,7 +81,7 @@ const Group = ({ children, ...props }) => { * * @see {PageMenu} examples. * - * @param {object} props - PF/DropdownItem props, See {@link https://www.patternfly.org/components/menus/dropdown#dropdownitem} + * @param {import('@patternfly/react-core').DropdownItemProps} props */ const Option = ({ children, ...props }) => { return ( @@ -92,7 +99,7 @@ const Option = ({ children, ...props }) => { * * @see {PageMenu} examples. * - * @param {object} props - PF/DropdownList props, See {@link https://www.patternfly.org/components/menus/dropdown#dropdownlist} + * @param {import('@patternfly/react-core').DropdownListProps} props */ const Options = ({ children, ...props }) => { return ( @@ -147,10 +154,14 @@ const Options = ({ children, ...props }) => { * * * - * @param {object} props - * @param {Group|Item|Array} props.children + * @typedef {object} PageMenuProps + * @property {string} [togglerAriaLabel] + * @property {string} label + * @property {React.ReactNode} children + * + * @param {PageMenuProps} props */ -const PageMenu = ({ togglerAriaLabel, children }) => { +const PageMenu = ({ togglerAriaLabel, label, children }) => { const [isOpen, setIsOpen] = useState(false); const toggle = () => setIsOpen(!isOpen); @@ -159,14 +170,14 @@ const PageMenu = ({ togglerAriaLabel, children }) => { return ( } + toggle={(toggleRef) => } onSelect={close} onOpenChange={close} popperProps={{ minWidth: "150px", position: "right" }} data-type="agama/page-menu" > - {Array(children)} + {children} ); diff --git a/web/src/components/storage/InstallationDeviceField.jsx b/web/src/components/storage/InstallationDeviceField.jsx index 2dddcd7cd1..cf6732dda8 100644 --- a/web/src/components/storage/InstallationDeviceField.jsx +++ b/web/src/components/storage/InstallationDeviceField.jsx @@ -25,7 +25,7 @@ import React, { useState } from "react"; import { Skeleton } from "@patternfly/react-core"; import { _ } from "~/i18n"; -import { DeviceSelectionDialog } from "~/components/storage"; +import { DeviceSelectionDialog, ProposalPageMenu } from "~/components/storage"; import { deviceLabel } from '~/components/storage/utils'; import { If, SettingsField } from "~/components/core"; import { sprintf } from "sprintf-js"; @@ -75,6 +75,12 @@ Volume Group for installation.") /> ); +const StorageTechSelector = () => { + return ( + + ); +}; + /** * Allows to select the installation device. * @component @@ -127,6 +133,7 @@ export default function InstallationDeviceField({ description={renderDescription()} onClick={openDialog} > + {_("Prepare more devices by configuring advanced")} { /** * Component for rendering the options available from Storage/ProposalPage * @component + * + * @typedef {object} ProposalMenuProps + * @property {string} label + * + * @param {ProposalMenuProps} props */ -export default function ProposalPageMenu () { +export default function ProposalPageMenu ({ label }) { const [showDasdLink, setShowDasdLink] = useState(false); const [showZFCPLink, setShowZFCPLink] = useState(false); const { storage: client } = useInstallerClient(); @@ -95,7 +102,7 @@ export default function ProposalPageMenu () { }, [client.dasd, client.zfcp]); return ( - + } /> From 0fcc1e133c137d9efab0dfc2847ccc34cde9f40f Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 12 Apr 2024 15:25:19 +0100 Subject: [PATCH 13/31] web: Small text adjustments --- web/src/components/storage/BootConfigField.jsx | 6 +++--- web/src/components/storage/BootConfigField.test.jsx | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/web/src/components/storage/BootConfigField.jsx b/web/src/components/storage/BootConfigField.jsx index 483e5d9f18..d23e440e03 100644 --- a/web/src/components/storage/BootConfigField.jsx +++ b/web/src/components/storage/BootConfigField.jsx @@ -94,12 +94,12 @@ export default function BootConfigField ({ let value; if (!configureBoot) { - value = <> {_("Installation will not create boot partitions.")}; + value = <> {_("Installation will not configure partitions for booting.")}; } else if (!bootDevice) { - value = _("Installation might create boot partitions at the installation device."); + value = _("Installation will configure partitions for booting at the installation disk."); } else { // TRANSLATORS: %s is the disk used to configure the boot-related partitions (eg. "/dev/sda, 80 GiB) - value = sprintf(_("Installation might create boot partitions at %s."), deviceLabel(bootDevice)); + value = sprintf(_("Installation will configure partitions for booting at %s."), deviceLabel(bootDevice)); } return ( diff --git a/web/src/components/storage/BootConfigField.test.jsx b/web/src/components/storage/BootConfigField.test.jsx index f4945e46bb..036db095ab 100644 --- a/web/src/components/storage/BootConfigField.test.jsx +++ b/web/src/components/storage/BootConfigField.test.jsx @@ -92,21 +92,21 @@ describe("BootConfigField", () => { describe("when installation is set for not configuring boot", () => { it("renders a text warning about it", () => { plainRender(); - screen.getByText(/will not create boot partitions/); + screen.getByText(/will not configure partitions/); }); }); describe("when installation is set for automatically configuring boot", () => { it("renders a text reporting about it", () => { plainRender(); - screen.getByText(/create boot partitions at the installation device/); + screen.getByText(/configure partitions for booting at the installation disk/); }); }); describe("when installation is set for configuring boot at specific device", () => { it("renders a text reporting about it", () => { plainRender(); - screen.getByText(/boot partitions at \/dev\/sda/); + screen.getByText(/partitions for booting at \/dev\/sda/); }); }); }); From 18a62daaa57ee29ef62b022a7f8ed378580e9fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 15:28:03 +0100 Subject: [PATCH 14/31] web: Fix ProposalPage tests --- .../components/storage/ProposalPage.test.jsx | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index 24d8c371d4..00649c0bf0 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -56,6 +56,8 @@ jest.mock("~/context/product", () => ({ }) })); +const createClientMock = /** @type {jest.Mock} */(createClient); + /** @type {StorageDevice} */ const vda = { sid: 59, @@ -168,8 +170,7 @@ beforeEach(() => { onStatusChange: jest.fn() }; - // @ts-expect-error Mocking method does not exist for InstallerClient type. - createClient.mockImplementation(() => ({ storage })); + createClientMock.mockImplementation(() => ({ storage })); }); it("probes storage if the storage devices are deprecated", async () => { @@ -189,9 +190,6 @@ it("loads the proposal data", async () => { installerRender(); - screen.getAllByText(/PFSkeleton/); - expect(screen.queryByText(/Installation device/)).toBeNull(); - await screen.findByText(/Installation device/); await screen.findByText(/\/dev\/vda/); }); @@ -238,15 +236,6 @@ describe("when the storage devices become deprecated", () => { }); describe("when there is no proposal yet", () => { - it("shows the page as loading", async () => { - proposalResult = undefined; - - installerRender(); - - screen.getAllByText(/PFSkeleton/); - await waitFor(() => expect(screen.queryByText(/Installation device/)).toBeNull()); - }); - it("loads the proposal when the service finishes to calculate", async () => { const defaultResult = proposalResult; proposalResult = undefined; From 4ea77fb01bf6d34854f0be8c101a97c465be7065 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 12 Apr 2024 15:35:16 +0100 Subject: [PATCH 15/31] web: small icon adjustment --- web/src/components/core/Field.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/core/Field.jsx b/web/src/components/core/Field.jsx index e05bfc84b5..ed8662ee83 100644 --- a/web/src/components/core/Field.jsx +++ b/web/src/components/core/Field.jsx @@ -86,7 +86,7 @@ const Field = ({ * @param {Omit} props */ const SettingsField = ({ ...props }) => { - return ; + return ; }; /** From 16a0f2e5f3934a2d521ae2c8648f399b36d854b4 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 12 Apr 2024 15:38:16 +0100 Subject: [PATCH 16/31] web: Fix typo --- web/src/components/storage/PartitionsField.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 64d3047ea8..927b5af95f 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -653,7 +653,7 @@ export default function PartitionsField({ setIsExpanded(!isExpanded)} > Date: Fri, 12 Apr 2024 16:09:04 +0100 Subject: [PATCH 17/31] web: Fix types --- .../components/storage/BootConfigField.jsx | 4 +- .../storage/InstallationDeviceField.jsx | 4 +- .../components/storage/PartitionsField.jsx | 97 ++++++++++++++----- .../storage/ProposalSettingsSection.jsx | 16 +-- .../storage/SnapshotsField.test.jsx | 3 +- .../components/storage/SpacePolicyField.jsx | 21 ++-- 6 files changed, 98 insertions(+), 47 deletions(-) diff --git a/web/src/components/storage/BootConfigField.jsx b/web/src/components/storage/BootConfigField.jsx index d23e440e03..da2d942b19 100644 --- a/web/src/components/storage/BootConfigField.jsx +++ b/web/src/components/storage/BootConfigField.jsx @@ -62,9 +62,9 @@ const Button = ({ isBold = false, onClick }) => { * @param {StorageDevice|undefined} props.defaultBootDevice * @param {StorageDevice[]} props.devices * @param {boolean} props.isLoading - * @param {(boot: Boot) => void} props.onChange + * @param {(boot: BootConfig) => void} props.onChange * - * @typedef {object} Boot + * @typedef {object} BootConfig * @property {boolean} configureBoot * @property {StorageDevice} bootDevice */ diff --git a/web/src/components/storage/InstallationDeviceField.jsx b/web/src/components/storage/InstallationDeviceField.jsx index cf6732dda8..a0ef5c6e35 100644 --- a/web/src/components/storage/InstallationDeviceField.jsx +++ b/web/src/components/storage/InstallationDeviceField.jsx @@ -91,9 +91,9 @@ const StorageTechSelector = () => { * @property {StorageDevice[]} targetPVDevices - Target devices for the LVM volume group (target "NEW_LVM_VG"). * @property {StorageDevice[]} devices - Available devices for installation. * @property {boolean} isLoading - * @property {(target: Target) => void} onChange + * @property {(target: TargetConfig) => void} onChange * - * @typedef {object} Target + * @typedef {object} TargetConfig * @property {ProposalTarget} target * @property {StorageDevice|undefined} targetDevice * @property {StorageDevice[]} targetPVDevices diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 927b5af95f..46d7ef8e11 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -42,9 +42,12 @@ import { noop } from "~/utils"; */ /** - * TODO: document + * @component + * + * @param {object} props + * @param {Volume} props.volume */ -const SizeText = (volume) => { +const SizeText = ({ volume }) => { let targetSize; if (volume.target === "FILESYSTEM" || volume.target === "DEVICE") targetSize = volume.targetDevice.size; @@ -60,12 +63,16 @@ const SizeText = (volume) => { }; /** - * TODO: document + * @component + * + * @param {object} props + * @param {Volume} props.volume + * @param {ProposalTarget} props.target */ -const BasicVolumeText = (volume, target) => { +const BasicVolumeText = ({ volume, target }) => { const snapshots = hasSnapshots(volume); const transactional = isTransactionalRoot(volume); - const size = SizeText(volume); + const size = SizeText({ volume }); const lvm = (target === "NEW_LVM_VG"); // When target is "filesystem" or "device" this is irrelevant since the type of device // is not mentioned @@ -141,9 +148,13 @@ const BasicVolumeText = (volume, target) => { }; /** - * TODO: document + * @component + * + * @param {object} props + * @param {boolean} props.configure + * @param {StorageDevice} props.device */ -const BootLabelText = (configure, device) => { +const BootLabelText = ({ configure, device }) => { if (!configure) return _("Do not configure partitions for booting"); @@ -157,12 +168,12 @@ const BootLabelText = (configure, device) => { /** * Generates an hint describing which attributes affect the auto-calculated limits. * If the limits are not affected then it returns `null`. - * @function + * @component * - * @param {object} volume - storage volume object - * @returns {(React.ReactElement|null)} component to display (can be `null`) + * @param {object} props + * @param {Volume} props.volume */ -const AutoCalculatedHint = (volume) => { +const AutoCalculatedHint = ({ volume }) => { const { snapshotsAffectSizes = false, sizeRelevantVolumes = [], adjustByRam } = volume.outline; // no hint, the size is not affected by known criteria @@ -243,23 +254,31 @@ const GeneralActions = ({ templates, onAdd, onReset }) => { }; /** - * TODO: document + * @component + * + * @param {object} props + * @param {Volume} props.volume + * @param {ProposalTarget} props.target */ const VolumeLabel = ({ volume, target }) => { return (
- {BasicVolumeText(volume, target)} + {BasicVolumeText({ volume, target })}
); }; /** - * TODO: document + * @component + * + * @param {object} props + * @param {StorageDevice|undefined} props.bootDevice + * @param {boolean} props.configureBoot */ const BootLabel = ({ bootDevice, configureBoot }) => { return (
- {BootLabelText(configureBoot, bootDevice)} + {BootLabelText({ configure: configureBoot, device: bootDevice })}
); }; @@ -315,9 +334,9 @@ const VolumeRow = ({ return (
- {SizeText(volume)} + {SizeText({ volume })} {/* TRANSLATORS: device flag, the partition size is automatically computed */} - {_("auto")}} /> + {_("auto")}} />
); }; @@ -526,7 +545,15 @@ const VolumesTable = ({ volumes, devices, target, targetDevice, isLoading, onVol }; /** - * TODO: document + * Content to show when the field is collapsed. + * @component + * + * @param {object} props + * @param {Volume[]} props.volumes + * @param {boolean} props.configureBoot + * @param {StorageDevice|undefined} props.bootDevice + * @param {ProposalTarget} props.target + * @param {boolean} props.isLoading */ const Basic = ({ volumes, configureBoot, bootDevice, target, isLoading }) => { if (isLoading) @@ -545,7 +572,21 @@ const Basic = ({ volumes, configureBoot, bootDevice, target, isLoading }) => { }; /** - * TODO: document + * Content to show when the field is expanded. + * @component + * + * @param {object} props + * @param {Volume[]} props.volumes + * @param {Volume[]} props.templates + * @param {StorageDevice[]} props.devices + * @param {ProposalTarget} props.target + * @param {StorageDevice|undefined} props.targetDevice + * @param {boolean} props.configureBoot + * @param {StorageDevice|undefined} props.bootDevice + * @param {StorageDevice|undefined} props.defaultBootDevice + * @param {(volumes: Volume[]) => void} props.onVolumesChange + * @param {(boot: BootConfig) => void} props.onBootChange + * @param {boolean} props.isLoading */ const Advanced = ({ volumes, @@ -616,7 +657,7 @@ const Advanced = ({ * * Create dialog components for the popup forms (e.g., EditVolumeDialog). * * Use a TreeTable, specially if we need to represent subvolumes. * - * Renders information of the volumes and boot-related partitions and actions to modify them + * Renders information of the volumes and boot-related partitions and actions to modify them. * @component * * @typedef {object} PartitionsFieldProps @@ -625,12 +666,16 @@ const Advanced = ({ * @property {StorageDevice[]} devices - Devices available for installation * @property {ProposalTarget} target - Installation target * @property {StorageDevice|undefined} targetDevice - Device selected for installation, if target is a disk - * @property {TODO} configureBoot - TODO - * @property {TODO} bootDevice - TODO - * @property {TODO} defaultBootDevice - TODO + * @property {boolean} configureBoot - Whether to configure boot partitions. + * @property {StorageDevice|undefined} bootDevice - Device to use for creating boot partitions. + * @property {StorageDevice|undefined} defaultBootDevice - Default device for boot partitions if no device has been indicated yet. * @property {boolean} [isLoading=false] - Whether to show the content as loading * @property {(volumes: Volume[]) => void} onVolumesChange - Function to use for changing the volumes - * @property {TODO} onBootChange - Function for changing the boot settings + * @property {(boot: BootConfig) => void} onBootChange - Function for changing the boot settings + * + * @typedef {object} BootConfig + * @property {boolean} configureBoot + * @property {StorageDevice|undefined} bootDevice * * @param {PartitionsFieldProps} props */ @@ -644,8 +689,8 @@ export default function PartitionsField({ bootDevice, defaultBootDevice, isLoading = false, - onVolumesChange = noop, - onBootChange = noop + onVolumesChange, + onBootChange }) { const [isExpanded, setIsExpanded] = useState(false); diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index 0f7cc869ee..bf7aad8661 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -62,9 +62,7 @@ export default function ProposalSettingsSection({ isLoading = false, onChange }) { - /** - * @param {import("~/components/storage/InstallationDeviceField").Target} targetOptions - */ + /** @param {import("~/components/storage/InstallationDeviceField").TargetConfig} targetConfig */ const changeTarget = ({ target, targetDevice, targetPVDevices }) => { onChange({ target, @@ -73,14 +71,17 @@ export default function ProposalSettingsSection({ }); }; + /** @param {import("~/components/storage/EncryptionField").EncryptionConfig} encrytionConfig */ const changeEncryption = ({ password, method }) => { onChange({ encryptionPassword: password, encryptionMethod: method }); }; + /** @param {Volume[]} volumes */ const changeVolumes = (volumes) => { onChange({ volumes }); }; + /** @param {import("~/components/storage/SpacePolicyField").SpacePolicyConfig} spacePolicyConfig */ const changeSpacePolicy = ({ spacePolicy, spaceActions }) => { onChange({ spacePolicy: spacePolicy.id, @@ -88,6 +89,7 @@ export default function ProposalSettingsSection({ }); }; + /** @param {import("~/components/storage/PartitionsField").BootConfig} bootConfig */ const changeBoot = ({ configureBoot, bootDevice }) => { onChange({ configureBoot, @@ -105,13 +107,16 @@ export default function ProposalSettingsSection({ const targetDevice = findDevice(settings.targetDevice); /** @type {StorageDevice[]} */ const targetPVDevices = compact(settings.targetPVDevices?.map(findDevice) || []); - const useEncryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0; const { volumes = [], installationDevices = [], spaceActions = [] } = settings; const bootDevice = findDevice(settings.bootDevice); const defaultBootDevice = findDevice(settings.defaultBootDevice); const spacePolicy = SPACE_POLICIES.find(p => p.id === settings.spacePolicy); - // Templates for already existing mount points are filtered out + /** + * Templates for already existing mount points are filtered out. + * + * @returns {Volume[]} + */ const usefulTemplates = () => { const mountPaths = volumes.map(v => v.mountPath); return volumeTemplates.filter(t => ( @@ -134,7 +139,6 @@ export default function ProposalSettingsSection({ password={settings.encryptionPassword || ""} method={settings.encryptionMethod} methods={encryptionMethods} - isChecked={useEncryption} isLoading={settings.encryptionPassword === undefined} onChange={changeEncryption} /> diff --git a/web/src/components/storage/SnapshotsField.test.jsx b/web/src/components/storage/SnapshotsField.test.jsx index a1cb27e12b..4b2bb43ae5 100644 --- a/web/src/components/storage/SnapshotsField.test.jsx +++ b/web/src/components/storage/SnapshotsField.test.jsx @@ -34,7 +34,7 @@ import SnapshotsField from "~/components/storage/SnapshotsField"; /** @type {Volume} */ const rootVolume = { mountPath: "/", - target: "/dev/sda", + target: "DEFAULT", fsType: "Btrfs", minSize: 1024, autoSize: true, @@ -46,6 +46,7 @@ const rootVolume = { supportAutoSize: true, snapshotsConfigurable: false, snapshotsAffectSizes: true, + adjustByRam: false, sizeRelevantVolumes: ["/home"] } }; diff --git a/web/src/components/storage/SpacePolicyField.jsx b/web/src/components/storage/SpacePolicyField.jsx index a1bc41e64c..3208820dcc 100644 --- a/web/src/components/storage/SpacePolicyField.jsx +++ b/web/src/components/storage/SpacePolicyField.jsx @@ -33,23 +33,24 @@ import SpacePolicyDialog from "~/components/storage/SpacePolicyDialog"; * @typedef {import ("~/client/storage").SpaceAction} SpaceAction * @typedef {import ("~/components/storage/utils").SpacePolicy} SpacePolicy * @typedef {import ("~/client/storage").StorageDevice} StorageDevice - * - * @typedef {object} SpacePolicyConfig - * @property {SpacePolicy} spacePolicy - * @property {SpaceAction[]} spaceActions */ /** * Allows to select the space policy. * @component * - * @param {object} props - * @param {SpacePolicy|undefined} props.policy - * @param {SpaceAction[]} props.actions - * @param {StorageDevice[]} props.devices - * @param {boolean} props.isLoading - * @param {(config: SpacePolicyConfig) => void} props.onChange + * @typedef {object} SpacePolicyFieldProps + * @property {SpacePolicy|undefined} policy + * @property {SpaceAction[]} actions + * @property {StorageDevice[]} devices + * @property {boolean} isLoading + * @property {(config: SpacePolicyConfig) => void} onChange + * + * @typedef {object} SpacePolicyConfig + * @property {SpacePolicy} spacePolicy + * @property {SpaceAction[]} spaceActions * + * @param {SpacePolicyFieldProps} props */ export default function SpacePolicyField({ policy, From ce141a0924975da31ca1b489062d27c69eab11ec Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 12 Apr 2024 16:30:19 +0100 Subject: [PATCH 18/31] web: Fix typo --- web/src/components/storage/ProposalSettingsSection.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index bf7aad8661..04948c5df7 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -71,7 +71,7 @@ export default function ProposalSettingsSection({ }); }; - /** @param {import("~/components/storage/EncryptionField").EncryptionConfig} encrytionConfig */ + /** @param {import("~/components/storage/EncryptionField").EncryptionConfig} encryptionConfig */ const changeEncryption = ({ password, method }) => { onChange({ encryptionPassword: password, encryptionMethod: method }); }; From 35c2a786c3d2fb569c6b6eba558b84a3817a366f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 16:31:51 +0100 Subject: [PATCH 19/31] web: Allow set/unset the encryption Adding a SwichtField to the encryption settings. A few CSS tweaks has been added, but they are most probably something temporary until we have time to improve/change them. --- web/src/assets/styles/blocks.scss | 17 ++++- web/src/components/core/Field.jsx | 9 ++- .../core/PasswordAndConfirmationInput.jsx | 8 +- .../PasswordAndConfirmationInput.test.jsx | 39 ++++++++-- .../components/storage/EncryptionField.jsx | 53 ++++++++++---- .../storage/EncryptionField.test.jsx | 73 +++++++++++++------ 6 files changed, 150 insertions(+), 49 deletions(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 46c38c4374..68b5b2194d 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -784,8 +784,23 @@ section [data-type="agama/reminder"] { margin-block-start: var(--spacer-small); } + &.highlighted > div:last-child { + --spacing: calc(var(--icon-size-s) / 2); + margin-inline: var(--spacing); + padding-inline: var(--spacing); + border-inline-start: 2px solid; + } + + &.highlighted.on > div:last-child { + border-color: var(--color-link-hover); + } + + &.highlighted.off > div:last-child { + border-color: var(--color-gray-darker); + } + &.on { - button { + button:not(.password-toggler) { fill: var(--color-link-hover); } } diff --git a/web/src/components/core/Field.jsx b/web/src/components/core/Field.jsx index ed8662ee83..74d955733a 100644 --- a/web/src/components/core/Field.jsx +++ b/web/src/components/core/Field.jsx @@ -90,17 +90,18 @@ const SettingsField = ({ ...props }) => { }; /** - * @param {Omit & {isChecked: boolean}} props + * @param {Omit & {isChecked: boolean, highlightContent?: boolean}} props */ -const SwitchField = ({ isChecked = false, ...props }) => { +const SwitchField = ({ isChecked = false, highlightContent = false, ...props }) => { const iconName = isChecked ? "toggle_on" : "toggle_off"; - const className = isChecked ? "on" : "off"; + const baseClassnames = highlightContent ? "highlighted" : ""; + const stateClassnames = isChecked ? "on" : "off"; return ( ); diff --git a/web/src/components/core/PasswordAndConfirmationInput.jsx b/web/src/components/core/PasswordAndConfirmationInput.jsx index 90fb19132a..6f4a5cc23a 100644 --- a/web/src/components/core/PasswordAndConfirmationInput.jsx +++ b/web/src/components/core/PasswordAndConfirmationInput.jsx @@ -19,7 +19,9 @@ * find current contact information at www.suse.com. */ -import React, { useState } from "react"; +// @ts-check + +import React, { useEffect, useState } from "react"; import { FormGroup } from "@patternfly/react-core"; import { FormValidationError, PasswordInput } from "~/components/core"; import { _ } from "~/i18n"; @@ -28,6 +30,10 @@ const PasswordAndConfirmationInput = ({ value, onChange, onValidation, isDisable const [confirmation, setConfirmation] = useState(value || ""); const [error, setError] = useState(""); + useEffect(() => { + if (isDisabled) setError(""); + }, [isDisabled]); + const validate = (password, passwordConfirmation) => { let newError = ""; diff --git a/web/src/components/core/PasswordAndConfirmationInput.test.jsx b/web/src/components/core/PasswordAndConfirmationInput.test.jsx index a8c080cf66..817ee64feb 100644 --- a/web/src/components/core/PasswordAndConfirmationInput.test.jsx +++ b/web/src/components/core/PasswordAndConfirmationInput.test.jsx @@ -49,14 +49,37 @@ it("uses the given password value for confirmation too", async () => { expect(passwordInput.value).toEqual(confirmationInput.value); }); -it("disables both, password and confirmation, when isDisabled prop is given", async () => { - plainRender( - - ); +describe("when isDisabled", () => { + it("disables both, password and confirmation", async () => { + plainRender( + + ); - const passwordInput = screen.getByLabelText("Password"); - const confirmationInput = screen.getByLabelText("Password confirmation"); + const passwordInput = screen.getByLabelText("Password"); + const confirmationInput = screen.getByLabelText("Password confirmation"); + + expect(passwordInput).toBeDisabled(); + expect(confirmationInput).toBeDisabled(); + }); + + it("clean errors", async () => { + const CleanErrorTest = () => { + const [isDisabled, setIsDisabled] = React.useState(false); - expect(passwordInput).toBeDisabled(); - expect(confirmationInput).toBeDisabled(); + return ( + <> + + + + ); + }; + + const { user } = plainRender(); + const passwordInput = screen.getByLabelText("Password"); + user.type(passwordInput, "123456"); + await screen.findByText("Passwords do not match"); + const setAsDisabledButton = screen.getByRole("button", { name: "Set as disabled" }); + await user.click(setAsDisabledButton); + expect(screen.queryByText("Passwords do not match")).toBeNull(); + }); }); diff --git a/web/src/components/storage/EncryptionField.jsx b/web/src/components/storage/EncryptionField.jsx index 9833018ec8..7b2e56ac30 100644 --- a/web/src/components/storage/EncryptionField.jsx +++ b/web/src/components/storage/EncryptionField.jsx @@ -25,7 +25,7 @@ import React, { useEffect, useState } from "react"; import { Checkbox, Form, Skeleton } from "@patternfly/react-core"; import { _ } from "~/i18n"; import { noop } from "~/utils"; -import { If, SettingsField, PasswordAndConfirmationInput, Popup } from "~/components/core"; +import { If, SwitchField, SettingsField, PasswordAndConfirmationInput, Popup } from "~/components/core"; import { EncryptionMethods } from "~/client/storage"; /** @@ -44,6 +44,7 @@ the device, including data, programs, and system files."); * * @param {object} props * @param {string} props.id - Form ID. + * @param {boolean} props.isDisabled=false - Whether the form is disabled or not. * @param {string} props.password - Password for encryption. * @param {string} props.method - Encryption method. * @param {string[]} props.methods - Possible encryption methods. @@ -52,6 +53,7 @@ the device, including data, programs, and system files."); */ const EncryptionSettingsForm = ({ id, + isDisabled = false, password: passwordProp, method: methodProp, methods, @@ -62,8 +64,14 @@ const EncryptionSettingsForm = ({ const [method, setMethod] = useState(methodProp); useEffect(() => { + if (isDisabled) { + onValidate(true); + setPassword(""); + return; + } + if (password.length === 0) onValidate(false); - }, [password, onValidate]); + }, [isDisabled, password, onValidate]); const changePassword = (_, v) => setPassword(v); @@ -94,6 +102,7 @@ const EncryptionSettingsForm = ({ value={password} onChange={changePassword} onValidation={onValidate} + isDisabled={isDisabled} /> } isChecked={method === EncryptionMethods.TPM} + isDisabled={isDisabled} onChange={changeMethod} /> } @@ -136,6 +146,7 @@ export default function EncryptionField({ isLoading = false, onChange = noop }) { + const [isActive, setIsActive] = useState(password !== ""); const [isFormOpen, setIsFormOpen] = useState(false); const [isFormValid, setIsFormValid] = useState(true); @@ -145,7 +156,12 @@ export default function EncryptionField({ const acceptForm = (newPassword, newMethod) => { closeForm(); - onChange({ password: newPassword, method: newMethod }); + + if (isActive) { + onChange({ password: newPassword, method: newMethod }); + } else { + onChange({ password: "" }); + } }; const validateForm = (valid) => setIsFormValid(valid); @@ -158,7 +174,7 @@ export default function EncryptionField({ if (!password || password === "") return _("disabled"); if (method === EncryptionMethods.LUKS2) return _("enabled"); - if (method === EncryptionMethods.TPM) return _("enabled using TPM"); + if (method === EncryptionMethods.TPM) return _("using TPM unlocking"); }; return ( @@ -168,15 +184,26 @@ export default function EncryptionField({ value={} onClick={openForm} > - - + +
+ setIsActive(!isActive)} + label={_("Encrypt the system")} + textWrapper="span" + > + + +
{_("Accept")} diff --git a/web/src/components/storage/EncryptionField.test.jsx b/web/src/components/storage/EncryptionField.test.jsx index 2a1351ee04..3473ee0e2d 100644 --- a/web/src/components/storage/EncryptionField.test.jsx +++ b/web/src/components/storage/EncryptionField.test.jsx @@ -22,7 +22,7 @@ // @ts-check import React from "react"; -import { screen, within } from "@testing-library/react"; +import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { EncryptionMethods } from "~/client/storage"; import EncryptionField from "~/components/storage/EncryptionField"; @@ -30,6 +30,16 @@ import EncryptionField from "~/components/storage/EncryptionField"; let props; const onChangeFn = jest.fn(); +const openEncryptionSettings = async ({ password = "", onChange = onChangeFn }) => { + const { user } = plainRender(); + const button = screen.getByRole("button", { name: /Encryption/ }); + await user.click(button); + const dialog = await screen.findByRole("dialog"); + screen.getByRole("heading", { name: "Encryption" }); + + return { user, dialog }; +}; + describe("Encryption field", () => { beforeEach(() => { props = { onChange: onChangeFn }; @@ -41,46 +51,65 @@ describe("Encryption field", () => { screen.getByText("disabled"); // Encryption set with LUKS2 - rerender(); + rerender(); screen.getByText("enabled"); // Encryption set with TPM - rerender(); - screen.getByText("enabled using TPM"); + rerender(); + screen.getByText("using TPM unlocking"); }); - it("allows changing the encryption settings", async () => { - const { user } = plainRender(); - - const button = screen.getByRole("button", { name: /Encryption/ }); - await user.click(button); + it("allows setting the encryption", async () => { + const { user } = await openEncryptionSettings({}); - const popup = await screen.findByRole("dialog"); - screen.getByText("Encryption settings"); + const switchField = screen.getByRole("switch", { name: "Encrypt the system" }); const passwordInput = screen.getByLabelText("Password"); const passwordConfirmInput = screen.getByLabelText("Password confirmation"); - const accept = within(popup).getByRole("button", { name: "Accept" }); + const accept = screen.getByRole("button", { name: "Accept" }); + expect(switchField).toHaveAttribute("aria-checked", "false"); + expect(passwordInput).not.toBeEnabled(); + expect(passwordConfirmInput).not.toBeEnabled(); + await user.click(switchField); + expect(switchField).toHaveAttribute("aria-checked", "true"); + expect(passwordInput).toBeEnabled(); + expect(passwordConfirmInput).toBeEnabled(); await user.type(passwordInput, "1234"); await user.type(passwordConfirmInput, "1234"); await user.click(accept); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).toHaveBeenCalled(); + expect(props.onChange).toHaveBeenCalledWith( + expect.objectContaining({ password: "1234" }) + ); }); - it("allows closing the encryption settings without triggering changes", async () => { - const { user } = plainRender(); + it("allows unsetting the encryption", async () => { + const { user } = await openEncryptionSettings({ password: "1234" }); - const button = screen.getByRole("button", { name: /Encryption settings/ }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - screen.getByText("Encryption settings"); + const switchField = screen.getByRole("switch", { name: "Encrypt the system" }); + const passwordInput = screen.getByLabelText("Password"); + const passwordConfirmInput = screen.getByLabelText("Password confirmation"); + const accept = screen.getByRole("button", { name: "Accept" }); + expect(switchField).toHaveAttribute("aria-checked", "true"); + expect(passwordInput).toBeEnabled(); + expect(passwordConfirmInput).toBeEnabled(); + await user.click(switchField); + expect(switchField).toHaveAttribute("aria-checked", "false"); + expect(passwordInput).not.toBeEnabled(); + expect(passwordConfirmInput).not.toBeEnabled(); + await user.click(accept); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(props.onChange).toHaveBeenCalledWith({ password: "" }); + }); - const cancel = within(popup).getByRole("button", { name: "Cancel" }); + it("allows discarding the encryption settings dialog", async () => { + const { user } = await openEncryptionSettings({}); + const cancel = screen.getByRole("button", { name: "Cancel" }); await user.click(cancel); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); expect(props.onChange).not.toHaveBeenCalled(); }); + + test.todo("allows setting the TPM"); + test.todo("improve above tests"); }); From 7ada3fb7852cd5e3ad9e999bd91394ce2be085ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 16:31:13 +0100 Subject: [PATCH 20/31] web: Fix ProposalSettingsSection tests --- .../storage/ProposalSettingsSection.test.jsx | 54 ++++++------------- 1 file changed, 15 insertions(+), 39 deletions(-) diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx index 7aa0da0c36..35f8d64a65 100644 --- a/web/src/components/storage/ProposalSettingsSection.test.jsx +++ b/web/src/components/storage/ProposalSettingsSection.test.jsx @@ -22,8 +22,8 @@ // @ts-check import React from "react"; -import { screen, within } from "@testing-library/react"; -import { plainRender } from "~/test-utils"; +import { screen } from "@testing-library/react"; +import { installerRender } from "~/test-utils"; import { ProposalSettingsSection } from "~/components/storage"; /** @@ -124,7 +124,7 @@ beforeEach(() => { defaultBootDevice: "", encryptionPassword: "", encryptionMethod: "", - spacePolicy: "", + spacePolicy: "delete", spaceActions: [], volumes: [], installationDevices: [sda, sdb] @@ -137,54 +137,30 @@ beforeEach(() => { }); it("allows changing the selected device", async () => { - const { user } = plainRender(); + const { user } = installerRender(); const button = screen.getByRole("button", { name: /installation device/i }); await user.click(button); await screen.findByRole("dialog", { name: /Device for installing/ }); }); -describe("when snapshots are configurable", () => { - beforeEach(() => { - props.settings.volumes = [volume]; - }); +it("allows changing the encryption settings", async () => { + const { user } = installerRender(); + const button = screen.getByRole("button", { name: /Encryption/ }); - it("renders the snapshots field", () => { - plainRender(); - screen.getByRole("switch", { name: /snapshots for the root file system/ }); - }); -}); - -describe("when snapshots are not configurable", () => { - beforeEach(() => { - volume.outline.snapshotsConfigurable = false; - }); - - it("does not render the snapshots field", () => { - plainRender(); - const snapshotsSwitch = screen.queryByRole("switch", { name: /snapshots for the root file system/ }); - expect(snapshotsSwitch).toBeNull(); - }); + await user.click(button); + await screen.findByRole("dialog", { name: /Encryption settings/ }); }); it("renders a section holding file systems related stuff", () => { - plainRender(); - screen.getByRole("grid", { name: "Table with mount points" }); - screen.getByRole("grid", { name: /mount points/ }); + installerRender(); + screen.getByRole("button", { name: /Partitions and file systems/ }); }); -it("requests a volume change when onChange callback is triggered", async () => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: "Actions" }); +it("allows changing the space policy settings", async () => { + const { user } = installerRender(); + const button = screen.getByRole("button", { name: /Find space/ }); await user.click(button); - - const menu = screen.getByRole("menu"); - const reset = within(menu).getByRole("menuitem", { name: /Reset/ }); - - await user.click(reset); - - expect(props.onChange).toHaveBeenCalledWith( - { volumes: expect.any(Array) } - ); + await screen.findByRole("dialog", { name: /Find space/ }); }); From 318ac15832342b801856e5cb779054aa3ced0786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 16:35:12 +0100 Subject: [PATCH 21/31] web: Remove leftover Skeleton --- web/src/components/storage/EncryptionField.jsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/src/components/storage/EncryptionField.jsx b/web/src/components/storage/EncryptionField.jsx index 7b2e56ac30..d6182eded2 100644 --- a/web/src/components/storage/EncryptionField.jsx +++ b/web/src/components/storage/EncryptionField.jsx @@ -166,8 +166,6 @@ export default function EncryptionField({ const validateForm = (valid) => setIsFormValid(valid); - if (isLoading) return ; - // FIXME: extract to the top to avoid redefinitions? const FieldValue = () => { if (isLoading) return ; From 11f7439604611e0925011a01dfc693e7710c1b22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 16:40:40 +0100 Subject: [PATCH 22/31] web: Fix styles of PartitionsField basic view --- web/src/components/storage/PartitionsField.jsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/web/src/components/storage/PartitionsField.jsx b/web/src/components/storage/PartitionsField.jsx index 46d7ef8e11..9f883ad8e5 100644 --- a/web/src/components/storage/PartitionsField.jsx +++ b/web/src/components/storage/PartitionsField.jsx @@ -558,13 +558,15 @@ const VolumesTable = ({ volumes, devices, target, targetDevice, isLoading, onVol const Basic = ({ volumes, configureBoot, bootDevice, target, isLoading }) => { if (isLoading) return ( -
- +
+ + +
); return ( -
+
{volumes.map((v, i) => )}
From 38fdd5cc3980ef94f4cf00646b2244df51b0381a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 17:03:59 +0100 Subject: [PATCH 23/31] web: Fix EncryptionField value calculation --- .../components/storage/EncryptionField.jsx | 4 +-- .../storage/EncryptionField.test.jsx | 26 ++++++++----------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/web/src/components/storage/EncryptionField.jsx b/web/src/components/storage/EncryptionField.jsx index d6182eded2..b956fc29b3 100644 --- a/web/src/components/storage/EncryptionField.jsx +++ b/web/src/components/storage/EncryptionField.jsx @@ -146,7 +146,7 @@ export default function EncryptionField({ isLoading = false, onChange = noop }) { - const [isActive, setIsActive] = useState(password !== ""); + const [isActive, setIsActive] = useState(password?.length > 0); const [isFormOpen, setIsFormOpen] = useState(false); const [isFormValid, setIsFormValid] = useState(true); @@ -170,7 +170,7 @@ export default function EncryptionField({ const FieldValue = () => { if (isLoading) return ; - if (!password || password === "") return _("disabled"); + if (!isActive) return _("disabled"); if (method === EncryptionMethods.LUKS2) return _("enabled"); if (method === EncryptionMethods.TPM) return _("using TPM unlocking"); }; diff --git a/web/src/components/storage/EncryptionField.test.jsx b/web/src/components/storage/EncryptionField.test.jsx index 3473ee0e2d..6cad1a8611 100644 --- a/web/src/components/storage/EncryptionField.test.jsx +++ b/web/src/components/storage/EncryptionField.test.jsx @@ -27,7 +27,6 @@ import { plainRender } from "~/test-utils"; import { EncryptionMethods } from "~/client/storage"; import EncryptionField from "~/components/storage/EncryptionField"; -let props; const onChangeFn = jest.fn(); const openEncryptionSettings = async ({ password = "", onChange = onChangeFn }) => { @@ -41,21 +40,18 @@ const openEncryptionSettings = async ({ password = "", onChange = onChangeFn }) }; describe("Encryption field", () => { - beforeEach(() => { - props = { onChange: onChangeFn }; - }); - - it("renders proper value depending of encryption status", () => { - // No encryption set - const { rerender } = plainRender(); + it("renders 'disabled' when encryption is not set", () => { + plainRender(); screen.getByText("disabled"); + }); - // Encryption set with LUKS2 - rerender(); + it("renders 'enabled' when encryption is set", () => { + plainRender(); screen.getByText("enabled"); + }); - // Encryption set with TPM - rerender(); + it("renders 'using TPM unlocking' when encryption is set with TPM", () => { + plainRender(); screen.getByText("using TPM unlocking"); }); @@ -78,7 +74,7 @@ describe("Encryption field", () => { await user.click(accept); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).toHaveBeenCalledWith( + expect(onChangeFn).toHaveBeenCalledWith( expect.objectContaining({ password: "1234" }) ); }); @@ -99,7 +95,7 @@ describe("Encryption field", () => { expect(passwordConfirmInput).not.toBeEnabled(); await user.click(accept); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).toHaveBeenCalledWith({ password: "" }); + expect(onChangeFn).toHaveBeenCalledWith({ password: "" }); }); it("allows discarding the encryption settings dialog", async () => { @@ -107,7 +103,7 @@ describe("Encryption field", () => { const cancel = screen.getByRole("button", { name: "Cancel" }); await user.click(cancel); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).not.toHaveBeenCalled(); + expect(onChangeFn).not.toHaveBeenCalled(); }); test.todo("allows setting the TPM"); From 0ed651e386315ef6b9b5e41eab8bc015e6de6919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 17:17:42 +0100 Subject: [PATCH 24/31] web: Fix and adapt tests --- .../storage/PartitionsField.test.jsx | 104 ++++++++---------- .../storage/ProposalSettingsSection.test.jsx | 26 +---- 2 files changed, 47 insertions(+), 83 deletions(-) diff --git a/web/src/components/storage/PartitionsField.test.jsx b/web/src/components/storage/PartitionsField.test.jsx index 34557c521e..1ca64e36e8 100644 --- a/web/src/components/storage/PartitionsField.test.jsx +++ b/web/src/components/storage/PartitionsField.test.jsx @@ -24,10 +24,10 @@ import React from "react"; import { screen, within } from "@testing-library/react"; import { plainRender } from "~/test-utils"; -import { ProposalVolumes } from "~/components/storage"; +import PartitionsField from "~/components/storage/PartitionsField"; /** - * @typedef {import ("~/components/storage/ProposalVolumes").ProposalVolumesProps} ProposalVolumesProps + * @typedef {import("~/components/storage/PartitionsField").PartitionsFieldProps} PartitionsFieldProps * @typedef {import ("~/client/storage").StorageDevice} StorageDevice * @typedef {import ("~/client/storage").Volume} Volume */ @@ -38,7 +38,6 @@ jest.mock("@patternfly/react-core", () => { return { ...original, Skeleton: () =>
PFSkeleton
- }; }); @@ -145,79 +144,69 @@ const sda2 = { } }; -/** @type {ProposalVolumesProps} */ +/** @type {PartitionsFieldProps} */ let props; +const expandField = async () => { + const render = plainRender(); + const button = screen.getByRole("button", { name: "Partitions and file systems" }); + await render.user.click(button); + return render; +}; + beforeEach(() => { props = { - volumes: [], + volumes: [rootVolume, swapVolume], templates: [], devices: [], target: "DISK", targetDevice: undefined, - onChange: jest.fn() + configureBoot: false, + bootDevice: undefined, + defaultBootDevice: undefined, + onVolumesChange: jest.fn(), + onBootChange: jest.fn() }; }); -it("renders a button for the generic actions", async () => { - const { user } = plainRender(); +/** @todo Add tests for collapsed field. */ - const button = screen.getByRole("button", { name: "Actions" }); +it("allows to reset the file systems", async () => { + const { user } = await expandField(); + const button = screen.getByRole("button", { name: "Reset to defaults" }); await user.click(button); - const menu = screen.getByRole("menu"); - within(menu).getByRole("menuitem", { name: /Reset/ }); - within(menu).getByRole("menuitem", { name: /Add/ }); + expect(props.onVolumesChange).toHaveBeenCalledWith([]); }); -it("changes the volumes if reset action is used", async () => { - const { user } = plainRender(); - - const button = screen.getByRole("button", { name: "Actions" }); - await user.click(button); - const menu = screen.getByRole("menu"); - const reset = within(menu).getByRole("menuitem", { name: /Reset/ }); - await user.click(reset); - - expect(props.onChange).toHaveBeenCalledWith([]); -}); - -it("allows to add a volume if add action is used", async () => { +it("allows to add a file system", async () => { props.templates = [homeVolume]; + const { user } = await expandField(); - const { user } = plainRender(); - - const button = screen.getByRole("button", { name: "Actions" }); + const button = screen.getByRole("button", { name: "Add file system" }); await user.click(button); - const menu = screen.getByRole("menu"); - const add = within(menu).getByRole("menuitem", { name: /Add/ }); - await user.click(add); const popup = await screen.findByRole("dialog"); const accept = within(popup).getByRole("button", { name: "Accept" }); await user.click(accept); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).toHaveBeenCalledWith([props.templates[0]]); + expect(props.onVolumesChange).toHaveBeenCalledWith([rootVolume, swapVolume, homeVolume]); }); -it("allows to cancel if add action is used", async () => { +it("allows to cancel adding a file system", async () => { props.templates = [homeVolume]; + const { user } = await expandField(); - const { user } = plainRender(); - - const button = screen.getByRole("button", { name: "Actions" }); + const button = screen.getByRole("button", { name: "Add file system" }); await user.click(button); - const menu = screen.getByRole("menu"); - const add = within(menu).getByRole("menuitem", { name: /Add/ }); - await user.click(add); const popup = await screen.findByRole("dialog"); const cancel = within(popup).getByRole("button", { name: "Cancel" }); await user.click(cancel); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(props.onChange).not.toHaveBeenCalled(); + expect(props.onVolumesChange).not.toHaveBeenCalled(); }); describe("if there are volumes", () => { @@ -227,8 +216,7 @@ describe("if there are volumes", () => { it("renders skeleton for each volume if loading", async () => { props.isLoading = true; - - plainRender(); + await expandField(); const [, body] = await screen.findAllByRole("rowgroup"); @@ -240,34 +228,34 @@ describe("if there are volumes", () => { }); it("renders the information for each volume", async () => { - plainRender(); + await expandField(); const [, body] = await screen.findAllByRole("rowgroup"); expect(within(body).queryAllByRole("row").length).toEqual(3); within(body).getByRole("row", { name: "/ Btrfs 1 KiB - 2 KiB Partition at installation disk" }); - within(body).getByRole("row", { name: "/home XFS At least 1 KiB Partition at installation disk" }); + within(body).getByRole("row", { name: "/home XFS at least 1 KiB Partition at installation disk" }); within(body).getByRole("row", { name: "swap Swap 1 KiB Partition at installation disk" }); }); it("allows deleting the volume", async () => { - const { user } = plainRender(); + const { user } = await expandField(); const [, body] = await screen.findAllByRole("rowgroup"); - const row = within(body).getByRole("row", { name: "/home XFS At least 1 KiB Partition at installation disk" }); + const row = within(body).getByRole("row", { name: "/home XFS at least 1 KiB Partition at installation disk" }); const actions = within(row).getByRole("button", { name: "Actions" }); await user.click(actions); const deleteAction = within(row).queryByRole("menuitem", { name: "Delete" }); await user.click(deleteAction); - expect(props.onChange).toHaveBeenCalledWith(expect.not.arrayContaining([homeVolume])); + expect(props.onVolumesChange).toHaveBeenCalledWith(expect.not.arrayContaining([homeVolume])); }); it("allows editing the volume", async () => { - const { user } = plainRender(); + const { user } = await expandField(); const [, body] = await screen.findAllByRole("rowgroup"); - const row = within(body).getByRole("row", { name: "/home XFS At least 1 KiB Partition at installation disk" }); + const row = within(body).getByRole("row", { name: "/home XFS at least 1 KiB Partition at installation disk" }); const actions = within(row).getByRole("button", { name: "Actions" }); await user.click(actions); const editAction = within(row).queryByRole("menuitem", { name: "Edit" }); @@ -278,10 +266,10 @@ describe("if there are volumes", () => { }); it("allows changing the location of the volume", async () => { - const { user } = plainRender(); + const { user } = await expandField(); const [, body] = await screen.findAllByRole("rowgroup"); - const row = within(body).getByRole("row", { name: "/home XFS At least 1 KiB Partition at installation disk" }); + const row = within(body).getByRole("row", { name: "/home XFS at least 1 KiB Partition at installation disk" }); const actions = within(row).getByRole("button", { name: "Actions" }); await user.click(actions); const locationAction = within(row).queryByRole("menuitem", { name: "Change location" }); @@ -297,7 +285,7 @@ describe("if there are volumes", () => { }); it("renders 'transactional' legend as part of its information", async () => { - plainRender(); + await expandField(); const [, volumes] = await screen.findAllByRole("rowgroup"); @@ -311,7 +299,7 @@ describe("if there are volumes", () => { }); it("renders 'with snapshots' legend as part of its information", async () => { - plainRender(); + await expandField(); const [, volumes] = await screen.findAllByRole("rowgroup"); @@ -329,12 +317,12 @@ describe("if there are volumes", () => { }); it("renders the locations", async () => { - plainRender(); + await expandField(); const [, volumes] = await screen.findAllByRole("rowgroup"); within(volumes).getByRole("row", { name: "swap Swap 1 KiB Partition at /dev/sda" }); - within(volumes).getByRole("row", { name: "/home XFS At least 1 KiB Separate LVM at /dev/sda" }); + within(volumes).getByRole("row", { name: "/home XFS at least 1 KiB Separate LVM at /dev/sda" }); }); }); @@ -348,7 +336,7 @@ describe("if there are volumes", () => { }); it("renders the locations", async () => { - plainRender(); + await expandField(); const [, volumes] = await screen.findAllByRole("rowgroup"); @@ -364,7 +352,7 @@ describe("if there are not volumes", () => { }); it("renders an empty table if it is not loading", async () => { - plainRender(); + await expandField(); const [, body] = await screen.findAllByRole("rowgroup"); expect(body).toBeEmptyDOMElement(); @@ -373,7 +361,7 @@ describe("if there are not volumes", () => { it("renders an skeleton row if it is loading", async () => { props.isLoading = true; - plainRender(); + await expandField(); const [, body] = await screen.findAllByRole("rowgroup"); const rows = within(body).getAllByRole("row", { name: "PFSkeleton" }); diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx index 35f8d64a65..7071a83780 100644 --- a/web/src/components/storage/ProposalSettingsSection.test.jsx +++ b/web/src/components/storage/ProposalSettingsSection.test.jsx @@ -29,7 +29,6 @@ import { ProposalSettingsSection } from "~/components/storage"; /** * @typedef {import ("~/components/storage/ProposalSettingsSection").ProposalSettingsSectionProps} ProposalSettingsSectionProps * @typedef {import ("~/client/storage").StorageDevice} StorageDevice - * @typedef {import ("~/client/storage").Volume} Volume */ jest.mock("@patternfly/react-core", () => { @@ -87,33 +86,10 @@ const sdb = { udevPaths: ["pci-0000:00-19"] }; -/** @type {Volume} */ -let volume; - /** @type {ProposalSettingsSectionProps} */ let props; beforeEach(() => { - volume = { - mountPath: "/", - target: "DEFAULT", - fsType: "Btrfs", - minSize: 1024, - maxSize: 2048, - autoSize: false, - snapshots: false, - transactional: false, - outline: { - required: true, - fsTypes: ["Btrfs", "Ext4"], - supportAutoSize: true, - snapshotsConfigurable: true, - snapshotsAffectSizes: true, - sizeRelevantVolumes: [], - adjustByRam: false - } - }; - props = { settings: { target: "DISK", @@ -149,7 +125,7 @@ it("allows changing the encryption settings", async () => { const button = screen.getByRole("button", { name: /Encryption/ }); await user.click(button); - await screen.findByRole("dialog", { name: /Encryption settings/ }); + await screen.findByRole("dialog", { name: /Encryption/ }); }); it("renders a section holding file systems related stuff", () => { From 2b616f457f13c3fb3af397a2001af1a49faeb648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 17:26:08 +0100 Subject: [PATCH 25/31] web: Fix more tests --- web/src/components/core/Field.test.jsx | 4 ++-- .../storage/InstallationDeviceField.test.jsx | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/web/src/components/core/Field.test.jsx b/web/src/components/core/Field.test.jsx index d84295d1e4..2a07467732 100644 --- a/web/src/components/core/Field.test.jsx +++ b/web/src/components/core/Field.test.jsx @@ -65,13 +65,13 @@ describe("Field", () => { }); describe("SettingsField", () => { - it("uses the 'settings' icon", () => { + it("uses the 'shadow' icon", () => { const { container } = plainRender( // Trying to set other icon, although typechecking should catch it. ); const icon = container.querySelector("button > svg"); - expect(icon).toHaveAttribute("data-icon-name", "settings"); + expect(icon).toHaveAttribute("data-icon-name", "shadow"); }); }); diff --git a/web/src/components/storage/InstallationDeviceField.test.jsx b/web/src/components/storage/InstallationDeviceField.test.jsx index 41a9932402..25b8d8c717 100644 --- a/web/src/components/storage/InstallationDeviceField.test.jsx +++ b/web/src/components/storage/InstallationDeviceField.test.jsx @@ -23,7 +23,7 @@ import React from "react"; import { screen, within } from "@testing-library/react"; -import { plainRender } from "~/test-utils"; +import { installerRender } from "~/test-utils"; import InstallationDeviceField from "~/components/storage/InstallationDeviceField"; /** @@ -97,7 +97,7 @@ describe("when set as loading", () => { }); it("renders a loading hint", () => { - plainRender(); + installerRender(); screen.getByText("Waiting for information about selected device"); }); }); @@ -113,7 +113,7 @@ describe("when the target is a disk", () => { }); it("uses a 'No device selected yet' text for the selection button", async () => { - plainRender(); + installerRender(); screen.getByText("No device selected yet"); }); }); @@ -124,7 +124,7 @@ describe("when the target is a disk", () => { }); it("uses its name as part of the text for the selection button", async () => { - plainRender(); + installerRender(); screen.getByText(/\/dev\/sda/); }); }); @@ -141,7 +141,7 @@ describe("when the target is a new LVM volume group", () => { }); it("uses a 'No device selected yet' text for the selection button", async () => { - plainRender(); + installerRender(); screen.getByText("No device selected yet"); }); }); @@ -152,7 +152,7 @@ describe("when the target is a new LVM volume group", () => { }); it("uses its name as part of the text for the selection button", async () => { - plainRender(); + installerRender(); screen.getByText(/new LVM .* \/dev\/sda/); }); }); @@ -163,14 +163,14 @@ describe("when the target is a new LVM volume group", () => { }); it("does not use the names as part of the text for the selection button", async () => { - plainRender(); + installerRender(); screen.getByText("new LVM volume group"); }); }); }); it("allows changing the selected device", async () => { - const { user } = plainRender(); + const { user } = installerRender(); const button = screen.getByRole("button", { name: /installation device/i }); await user.click(button); @@ -193,7 +193,7 @@ it("allows changing the selected device", async () => { }); it("allows canceling a device selection", async () => { - const { user } = plainRender(); + const { user } = installerRender(); const button = screen.getByRole("button", { name: /installation device/i }); await user.click(button); From 51fdee0698e5d31003e2b17c5425704a0cd3d86c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 12 Apr 2024 18:53:52 +0100 Subject: [PATCH 26/31] web: EncryptionField fixes and improvements --- .../components/storage/EncryptionField.jsx | 68 +++++++++---------- .../storage/EncryptionField.test.jsx | 17 +++-- 2 files changed, 42 insertions(+), 43 deletions(-) diff --git a/web/src/components/storage/EncryptionField.jsx b/web/src/components/storage/EncryptionField.jsx index b956fc29b3..e8c651bf20 100644 --- a/web/src/components/storage/EncryptionField.jsx +++ b/web/src/components/storage/EncryptionField.jsx @@ -21,7 +21,7 @@ // @ts-check -import React, { useEffect, useState } from "react"; +import React, { useCallback, useEffect, useState } from "react"; import { Checkbox, Form, Skeleton } from "@patternfly/react-core"; import { _ } from "~/i18n"; import { noop } from "~/utils"; @@ -32,9 +32,17 @@ import { EncryptionMethods } from "~/client/storage"; * @typedef {import ("~/client/storage").StorageDevice} StorageDevice */ +// Field texts at root level to avoid redefinitions every time the component +// is rendered. const LABEL = _("Encryption"); const DESCRIPTION = _("Full disk encryption allows to protect the information stored at \ the device, including data, programs, and system files."); +const VALUES = { + loading: , + disabled: _("disabled"), + [EncryptionMethods.LUKS2]: _("enabled"), + [EncryptionMethods.TPM]: _("using TPM unlocking") +}; /** * Form for configuring the encryption password. @@ -146,10 +154,15 @@ export default function EncryptionField({ isLoading = false, onChange = noop }) { - const [isActive, setIsActive] = useState(password?.length > 0); + const validPassword = useCallback(() => password?.length > 0, [password]); + const [isEnabled, setIsEnabled] = useState(validPassword()); const [isFormOpen, setIsFormOpen] = useState(false); const [isFormValid, setIsFormValid] = useState(true); + useEffect(() => { + setIsEnabled(validPassword()); + }, [password, validPassword]); + const openForm = () => setIsFormOpen(true); const closeForm = () => setIsFormOpen(false); @@ -157,51 +170,38 @@ export default function EncryptionField({ const acceptForm = (newPassword, newMethod) => { closeForm(); - if (isActive) { + if (isEnabled) { onChange({ password: newPassword, method: newMethod }); } else { onChange({ password: "" }); } }; - const validateForm = (valid) => setIsFormValid(valid); - - // FIXME: extract to the top to avoid redefinitions? - const FieldValue = () => { - if (isLoading) return ; - - if (!isActive) return _("disabled"); - if (method === EncryptionMethods.LUKS2) return _("enabled"); - if (method === EncryptionMethods.TPM) return _("using TPM unlocking"); - }; - return ( } + value={isLoading ? VALUES.loading : VALUES[isEnabled ? method : "disabled"]} onClick={openForm} > -
- setIsActive(!isActive)} - label={_("Encrypt the system")} - textWrapper="span" - > - - -
+ setIsEnabled(!isEnabled)} + label={_("Encrypt the system")} + textWrapper="span" + > + + {_("Accept")} diff --git a/web/src/components/storage/EncryptionField.test.jsx b/web/src/components/storage/EncryptionField.test.jsx index 6cad1a8611..83e25111ac 100644 --- a/web/src/components/storage/EncryptionField.test.jsx +++ b/web/src/components/storage/EncryptionField.test.jsx @@ -40,18 +40,17 @@ const openEncryptionSettings = async ({ password = "", onChange = onChangeFn }) }; describe("Encryption field", () => { - it("renders 'disabled' when encryption is not set", () => { - plainRender(); + it("renders proper value depending of encryption status", () => { + // No encryption set + const { rerender } = plainRender(); screen.getByText("disabled"); - }); - it("renders 'enabled' when encryption is set", () => { - plainRender(); + // Encryption set with LUKS2 + rerender(); screen.getByText("enabled"); - }); - it("renders 'using TPM unlocking' when encryption is set with TPM", () => { - plainRender(); + // Encryption set with TPM + rerender(); screen.getByText("using TPM unlocking"); }); @@ -106,6 +105,6 @@ describe("Encryption field", () => { expect(onChangeFn).not.toHaveBeenCalled(); }); - test.todo("allows setting the TPM"); + test.todo("test that it allows setting the TPM"); test.todo("improve above tests"); }); From 396d855530060c427119b314778be3ab9fe661cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sat, 13 Apr 2024 23:57:11 +0100 Subject: [PATCH 27/31] web: Extract the EncryptionSettingsDialog For following the same strategy than other fields and easily decoupling the field value from the settings form switch. The change also removes the internal EncryptionSettingsForm component in order to reduce the indirection and simplify the code. --- .../components/storage/EncryptionField.jsx | 141 +++------------- .../storage/EncryptionField.test.jsx | 76 +-------- .../storage/EncryptionSettingsDialog.jsx | 130 ++++++++++++++ .../storage/EncryptionSettingsDialog.test.jsx | 158 ++++++++++++++++++ 4 files changed, 319 insertions(+), 186 deletions(-) create mode 100644 web/src/components/storage/EncryptionSettingsDialog.jsx create mode 100644 web/src/components/storage/EncryptionSettingsDialog.test.jsx diff --git a/web/src/components/storage/EncryptionField.jsx b/web/src/components/storage/EncryptionField.jsx index e8c651bf20..ae3844af8a 100644 --- a/web/src/components/storage/EncryptionField.jsx +++ b/web/src/components/storage/EncryptionField.jsx @@ -22,11 +22,12 @@ // @ts-check import React, { useCallback, useEffect, useState } from "react"; -import { Checkbox, Form, Skeleton } from "@patternfly/react-core"; +import { Skeleton } from "@patternfly/react-core"; import { _ } from "~/i18n"; import { noop } from "~/utils"; -import { If, SwitchField, SettingsField, PasswordAndConfirmationInput, Popup } from "~/components/core"; +import { If, SettingsField } from "~/components/core"; import { EncryptionMethods } from "~/client/storage"; +import EncryptionSettingsDialog from "~/components/storage/EncryptionSettingsDialog"; /** * @typedef {import ("~/client/storage").StorageDevice} StorageDevice @@ -44,91 +45,6 @@ const VALUES = { [EncryptionMethods.TPM]: _("using TPM unlocking") }; -/** - * Form for configuring the encryption password. - * @component - * - * @todo: improve typechecking for method and methods - * - * @param {object} props - * @param {string} props.id - Form ID. - * @param {boolean} props.isDisabled=false - Whether the form is disabled or not. - * @param {string} props.password - Password for encryption. - * @param {string} props.method - Encryption method. - * @param {string[]} props.methods - Possible encryption methods. - * @param {(password: string, method: string) => void} [props.onSubmit=noop] - On submit callback. - * @param {(valid: boolean) => void} [props.onValidate=noop] - On validate callback. - */ -const EncryptionSettingsForm = ({ - id, - isDisabled = false, - password: passwordProp, - method: methodProp, - methods, - onSubmit = noop, - onValidate = noop -}) => { - const [password, setPassword] = useState(passwordProp || ""); - const [method, setMethod] = useState(methodProp); - - useEffect(() => { - if (isDisabled) { - onValidate(true); - setPassword(""); - return; - } - - if (password.length === 0) onValidate(false); - }, [isDisabled, password, onValidate]); - - const changePassword = (_, v) => setPassword(v); - - const changeMethod = (_, value) => { - const newMethod = value ? EncryptionMethods.TPM : EncryptionMethods.LUKS2; - setMethod(newMethod); - }; - - const submitForm = (e) => { - e.preventDefault(); - onSubmit(password, method); - }; - - const Description = () => ( - TPM can verify the integrity of the system. TPM sealing requires the new system to be booted directly on its first run.") - }} - /> - ); - - return ( -
- - } - isChecked={method === EncryptionMethods.TPM} - isDisabled={isDisabled} - onChange={changeMethod} - /> - } - /> - - ); -}; - /** * Allows to define encryption * @component @@ -156,25 +72,22 @@ export default function EncryptionField({ }) { const validPassword = useCallback(() => password?.length > 0, [password]); const [isEnabled, setIsEnabled] = useState(validPassword()); - const [isFormOpen, setIsFormOpen] = useState(false); - const [isFormValid, setIsFormValid] = useState(true); + const [isDialogOpen, setIsDialogOpen] = useState(false); useEffect(() => { setIsEnabled(validPassword()); }, [password, validPassword]); - const openForm = () => setIsFormOpen(true); + const openDialog = () => setIsDialogOpen(true); - const closeForm = () => setIsFormOpen(false); + const closeDialog = () => setIsDialogOpen(false); - const acceptForm = (newPassword, newMethod) => { - closeForm(); - - if (isEnabled) { - onChange({ password: newPassword, method: newMethod }); - } else { - onChange({ password: "" }); - } + /** + * @param {import("~/components/storage/EncryptionSettingsDialog").EncryptionSetting} encryptionSetting + */ + const onAccept = (encryptionSetting) => { + closeDialog(); + onChange(encryptionSetting); }; return ( @@ -182,31 +95,21 @@ export default function EncryptionField({ label={LABEL} description={DESCRIPTION} value={isLoading ? VALUES.loading : VALUES[isEnabled ? method : "disabled"]} - onClick={openForm} + onClick={openDialog} > - - setIsEnabled(!isEnabled)} - label={_("Encrypt the system")} - textWrapper="span" - > - - - - {_("Accept")} - - - + } + />
); } diff --git a/web/src/components/storage/EncryptionField.test.jsx b/web/src/components/storage/EncryptionField.test.jsx index 83e25111ac..bb216e7e67 100644 --- a/web/src/components/storage/EncryptionField.test.jsx +++ b/web/src/components/storage/EncryptionField.test.jsx @@ -22,25 +22,13 @@ // @ts-check import React from "react"; -import { screen } from "@testing-library/react"; +import { screen, within } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { EncryptionMethods } from "~/client/storage"; import EncryptionField from "~/components/storage/EncryptionField"; -const onChangeFn = jest.fn(); - -const openEncryptionSettings = async ({ password = "", onChange = onChangeFn }) => { - const { user } = plainRender(); - const button = screen.getByRole("button", { name: /Encryption/ }); - await user.click(button); - const dialog = await screen.findByRole("dialog"); - screen.getByRole("heading", { name: "Encryption" }); - - return { user, dialog }; -}; - -describe("Encryption field", () => { - it("renders proper value depending of encryption status", () => { +describe("EncryptionField", () => { + it("renders proper value depending on encryption status", () => { // No encryption set const { rerender } = plainRender(); screen.getByText("disabled"); @@ -54,57 +42,11 @@ describe("Encryption field", () => { screen.getByText("using TPM unlocking"); }); - it("allows setting the encryption", async () => { - const { user } = await openEncryptionSettings({}); - - const switchField = screen.getByRole("switch", { name: "Encrypt the system" }); - const passwordInput = screen.getByLabelText("Password"); - const passwordConfirmInput = screen.getByLabelText("Password confirmation"); - const accept = screen.getByRole("button", { name: "Accept" }); - expect(switchField).toHaveAttribute("aria-checked", "false"); - expect(passwordInput).not.toBeEnabled(); - expect(passwordConfirmInput).not.toBeEnabled(); - await user.click(switchField); - expect(switchField).toHaveAttribute("aria-checked", "true"); - expect(passwordInput).toBeEnabled(); - expect(passwordConfirmInput).toBeEnabled(); - await user.type(passwordInput, "1234"); - await user.type(passwordConfirmInput, "1234"); - await user.click(accept); - - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(onChangeFn).toHaveBeenCalledWith( - expect.objectContaining({ password: "1234" }) - ); + it("allows opening the encryption settings dialog", async () => { + const { user } = plainRender(); + const button = screen.getByRole("button", { name: /Encryption/ }); + await user.click(button); + const dialog = await screen.findByRole("dialog"); + within(dialog).getByRole("heading", { name: "Encryption" }); }); - - it("allows unsetting the encryption", async () => { - const { user } = await openEncryptionSettings({ password: "1234" }); - - const switchField = screen.getByRole("switch", { name: "Encrypt the system" }); - const passwordInput = screen.getByLabelText("Password"); - const passwordConfirmInput = screen.getByLabelText("Password confirmation"); - const accept = screen.getByRole("button", { name: "Accept" }); - expect(switchField).toHaveAttribute("aria-checked", "true"); - expect(passwordInput).toBeEnabled(); - expect(passwordConfirmInput).toBeEnabled(); - await user.click(switchField); - expect(switchField).toHaveAttribute("aria-checked", "false"); - expect(passwordInput).not.toBeEnabled(); - expect(passwordConfirmInput).not.toBeEnabled(); - await user.click(accept); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(onChangeFn).toHaveBeenCalledWith({ password: "" }); - }); - - it("allows discarding the encryption settings dialog", async () => { - const { user } = await openEncryptionSettings({}); - const cancel = screen.getByRole("button", { name: "Cancel" }); - await user.click(cancel); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(onChangeFn).not.toHaveBeenCalled(); - }); - - test.todo("test that it allows setting the TPM"); - test.todo("improve above tests"); }); diff --git a/web/src/components/storage/EncryptionSettingsDialog.jsx b/web/src/components/storage/EncryptionSettingsDialog.jsx new file mode 100644 index 0000000000..f3cb6ea2f7 --- /dev/null +++ b/web/src/components/storage/EncryptionSettingsDialog.jsx @@ -0,0 +1,130 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React, { useEffect, useState } from "react"; +import { Checkbox, Form } from "@patternfly/react-core"; +import { _ } from "~/i18n"; +import { If, SwitchField, PasswordAndConfirmationInput, Popup } from "~/components/core"; +import { EncryptionMethods } from "~/client/storage"; + +/** + * @typedef {object} EncryptionSetting + * @property {string} password + * @property {string} [method] + */ + +const DIALOG_TITLE = _("Encryption"); +const DIALOG_DESCRIPTION = _("Full disk encryption allows to protect the information stored at \ +the device, including data, programs, and system files."); +const TPM_LABEL = _("Use the TPM to decrypt automatically on each boot"); +// TRANSLATORS: The word 'directly' is key here. For example, booting to the installer media and then choosing +// 'Boot from Hard Disk' from there will not work. Keep it sort (this is a hint in a form) but keep it clear. +// Do not translate 'abbr' and 'title', they are part of the HTML markup. +const TPM_EXPLANATION = _("The password will not be needed to boot and access the data if the \ +TPM can verify the integrity of the system. \ +TPM sealing requires the new system to be booted directly on its first run."); + +/** + * Renders a dialog that allows the user change encryption settings + * @component + * + * @typedef {object} EncryptionSettingsDialogProps + * @property {string} password - Password for encryption. + * @property {string} method - Encryption method. + * @property {string[]} methods - Possible encryption methods. + * @property {boolean} [isOpen=false] - Whether the dialog is visible or not. + * @property {() => void} onCancel - Callback to trigger when on cancel action. + * @property {(settings: EncryptionSetting) => void} onAccept - Callback to trigger on accept action. + */ +export default function EncryptionSettingsDialog({ + password, + method, + methods, + isOpen = false, + onCancel, + onAccept +}) { + const [isEnabled, setIsEnabled] = useState(password?.length > 0); + const [newPassword, setNewPassword] = useState(password); + const [newMethod, setNewMethod] = useState(method); + const [passwordsMatch, setPasswordsMatch] = useState(true); + const [validSettings, setValidSettings] = useState(true); + const formId = "encryptionSettingsForm"; + + useEffect(() => { + setValidSettings(!isEnabled || (newPassword.length > 0 && passwordsMatch)); + }, [isEnabled, newPassword, passwordsMatch]); + + const changePassword = (_, v) => setNewPassword(v); + const changeMethod = (_, value) => setNewMethod(value ? EncryptionMethods.TPM : EncryptionMethods.LUKS2); + + const submitSettings = (e) => { + e.preventDefault(); + + if (isEnabled) { + onAccept({ password: newPassword, method: newMethod }); + } else { + onAccept({ password: "" }); + } + }; + + return ( + + setIsEnabled(!isEnabled)} + label={_("Encrypt the system")} + textWrapper="span" + > +
+ + } + isChecked={method === EncryptionMethods.TPM} + isDisabled={!isEnabled} + onChange={changeMethod} + /> + } + /> + +
+ + + {_("Accept")} + + + +
+ ); +} diff --git a/web/src/components/storage/EncryptionSettingsDialog.test.jsx b/web/src/components/storage/EncryptionSettingsDialog.test.jsx new file mode 100644 index 0000000000..fa041adcb7 --- /dev/null +++ b/web/src/components/storage/EncryptionSettingsDialog.test.jsx @@ -0,0 +1,158 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +// @ts-check + +import React from "react"; +import { screen } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { EncryptionMethods } from "~/client/storage"; +import EncryptionSettingsDialog from "~/components/storage/EncryptionSettingsDialog"; + +/** @type {import("~/components/storage/EncryptionSettingsDialog").EncryptionSettingsDialogProps} */ +let props; +const onCancelFn = jest.fn(); +const onAcceptFn = jest.fn(); + +describe("EncryptionSettingsDialog", () => { + beforeEach(() => { + props = { + password: "1234", + method: EncryptionMethods.LUKS2, + methods: Object.values(EncryptionMethods), + isOpen: true, + onCancel: onCancelFn, + onAccept: onAcceptFn + }; + }); + + describe("when password is not set", () => { + beforeEach(() => { + props.password = ""; + }); + + it("allows settings the encryption", async () => { + const { user } = plainRender(); + const switchField = screen.getByRole("switch", { name: "Encrypt the system" }); + const passwordInput = screen.getByLabelText("Password"); + const confirmationInput = screen.getByLabelText("Password confirmation"); + const tpmCheckbox = screen.getByRole("checkbox", { name: /Use the TPM/ }); + const acceptButton = screen.getByRole("button", { name: "Accept" }); + + expect(switchField).not.toBeChecked(); + expect(passwordInput).toBeDisabled(); + expect(passwordInput).toBeDisabled(); + expect(tpmCheckbox).toBeDisabled(); + + await user.click(switchField); + + expect(switchField).toBeChecked(); + expect(passwordInput).toBeEnabled(); + expect(passwordInput).toBeEnabled(); + expect(tpmCheckbox).toBeEnabled(); + + await user.type(passwordInput, "2345"); + await user.type(confirmationInput, "2345"); + await user.click(acceptButton); + + expect(props.onAccept).toHaveBeenCalledWith( + expect.objectContaining({ password: "2345" }) + ); + }); + }); + + describe("when password is set", () => { + it("allows changing the encryption", async () => { + const { user } = plainRender(); + const passwordInput = screen.getByLabelText("Password"); + const confirmationInput = screen.getByLabelText("Password confirmation"); + const acceptButton = screen.getByRole("button", { name: "Accept" }); + const tpmCheckbox = screen.getByRole("checkbox", { name: /Use the TPM/ }); + + await user.clear(passwordInput); + await user.type(passwordInput, "9876"); + await user.clear(confirmationInput); + await user.type(confirmationInput, "9876"); + await user.click(tpmCheckbox); + await user.click(acceptButton); + + expect(props.onAccept).toHaveBeenCalledWith( + { password: "9876", method: EncryptionMethods.TPM } + ); + }); + + it("allows unsetting the encryption", async () => { + const { user } = plainRender(); + const switchField = screen.getByRole("switch", { name: "Encrypt the system" }); + const acceptButton = screen.getByRole("button", { name: "Accept" }); + expect(switchField).toBeChecked(); + await user.click(switchField); + expect(switchField).not.toBeChecked(); + await user.click(acceptButton); + expect(props.onAccept).toHaveBeenCalledWith({ password: "" }); + }); + }); + + describe("when TPM is not included in given methods", () => { + beforeEach(() => { + props.methods = [EncryptionMethods.LUKS2]; + }); + + it("does not render the TPM checkbox", () => { + plainRender(); + expect(screen.queryByRole("checkbox", { name: /Use the TPM/ })).toBeNull(); + }); + }); + + it("does not allow sending not valid settings", async () => { + const { user } = plainRender(); + const switchField = screen.getByRole("switch", { name: "Encrypt the system" }); + const passwordInput = screen.getByLabelText("Password"); + const confirmationInput = screen.getByLabelText("Password confirmation"); + const acceptButton = screen.getByRole("button", { name: "Accept" }); + + expect(acceptButton).toBeEnabled(); + await user.clear(confirmationInput); + // Now password and passwordConfirmation do not match + expect(acceptButton).toBeDisabled(); + await user.click(switchField); + // But now the user is trying to unset the encryption + expect(acceptButton).toBeEnabled(); + await user.click(switchField); + // Back to a not valid settings state + expect(acceptButton).toBeDisabled(); + await user.clear(passwordInput); + await user.clear(confirmationInput); + // Passwords match... but are empty + expect(acceptButton).toBeDisabled(); + await user.type(passwordInput, "valid"); + await user.type(confirmationInput, "valid"); + // Not empty passwords matching! + expect(acceptButton).toBeEnabled(); + }); + + it("triggers onCancel callback when dialog is discarded", async () => { + const { user } = plainRender(); + const cancelButton = screen.getByRole("button", { name: "Cancel" }); + await user.click(cancelButton); + expect(props.onCancel).toHaveBeenCalled(); + }); +}); From 324b018e33104bacd29cb5ed4fd4cd8e602cfa25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 14 Apr 2024 00:05:18 +0100 Subject: [PATCH 28/31] web: Remove not needed aria-label attributes These inputs already have labels, which should be enough for identifying them. Moreover, the aria-label were tied to a "User authentication" component, which is wrong for a component intended to be used where needed. --- web/src/components/core/PasswordAndConfirmationInput.jsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/src/components/core/PasswordAndConfirmationInput.jsx b/web/src/components/core/PasswordAndConfirmationInput.jsx index 6f4a5cc23a..9cdfbc5d0e 100644 --- a/web/src/components/core/PasswordAndConfirmationInput.jsx +++ b/web/src/components/core/PasswordAndConfirmationInput.jsx @@ -63,7 +63,6 @@ const PasswordAndConfirmationInput = ({ value, onChange, onValidation, isDisable Date: Sun, 14 Apr 2024 00:13:26 +0100 Subject: [PATCH 29/31] web: Small CSS adjustments To make a Menu inside a core/Field a bit more compact. In any case, these styles are going to be polished ASAP. --- web/src/assets/styles/blocks.scss | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 68b5b2194d..9bf8faff5c 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -719,10 +719,6 @@ section [data-type="agama/reminder"] { } } -[data-type="agama/field"] button.pf-v5-c-menu-toggle.pf-m-plain { - padding: 0; -} - [data-type="agama/expandable-selector"] { // The expandable selector is built on top of PF/Table#expandable // Let's tweak some styles @@ -812,6 +808,15 @@ section [data-type="agama/reminder"] { } } +[data-type="agama/field"] button.pf-v5-c-menu-toggle.pf-m-plain { + padding: 0; +} + +[data-type="agama/field"] .pf-v5-c-menu__list { + padding: calc(var(--spacer-smaller) / 2) 0; + margin: 0; +} + #boot-form { legend { label { From e35951793d1328be8f51ee7387e058939b1af997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 14 Apr 2024 00:32:04 +0100 Subject: [PATCH 30/31] web: EncryptionSettingsDialog refinement --- .../storage/EncryptionSettingsDialog.jsx | 24 ++++++++++--------- .../storage/EncryptionSettingsDialog.test.jsx | 19 +++++++++++++++ 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/web/src/components/storage/EncryptionSettingsDialog.jsx b/web/src/components/storage/EncryptionSettingsDialog.jsx index f3cb6ea2f7..1597a13993 100644 --- a/web/src/components/storage/EncryptionSettingsDialog.jsx +++ b/web/src/components/storage/EncryptionSettingsDialog.jsx @@ -55,34 +55,36 @@ TPM sealing requires the new system to be booted directly on its first run."); * @property {boolean} [isOpen=false] - Whether the dialog is visible or not. * @property {() => void} onCancel - Callback to trigger when on cancel action. * @property {(settings: EncryptionSetting) => void} onAccept - Callback to trigger on accept action. + * + * @param {EncryptionSettingsDialogProps} props */ export default function EncryptionSettingsDialog({ - password, - method, + password: passwordProp, + method: methodProp, methods, isOpen = false, onCancel, onAccept }) { - const [isEnabled, setIsEnabled] = useState(password?.length > 0); - const [newPassword, setNewPassword] = useState(password); - const [newMethod, setNewMethod] = useState(method); + const [isEnabled, setIsEnabled] = useState(passwordProp?.length > 0); + const [password, setPassword] = useState(passwordProp); + const [method, setMethod] = useState(methodProp); const [passwordsMatch, setPasswordsMatch] = useState(true); const [validSettings, setValidSettings] = useState(true); const formId = "encryptionSettingsForm"; useEffect(() => { - setValidSettings(!isEnabled || (newPassword.length > 0 && passwordsMatch)); - }, [isEnabled, newPassword, passwordsMatch]); + setValidSettings(!isEnabled || (password.length > 0 && passwordsMatch)); + }, [isEnabled, password, passwordsMatch]); - const changePassword = (_, v) => setNewPassword(v); - const changeMethod = (_, value) => setNewMethod(value ? EncryptionMethods.TPM : EncryptionMethods.LUKS2); + const changePassword = (_, v) => setPassword(v); + const changeMethod = (_, useTPM) => setMethod(useTPM ? EncryptionMethods.TPM : EncryptionMethods.LUKS2); const submitSettings = (e) => { e.preventDefault(); if (isEnabled) { - onAccept({ password: newPassword, method: newMethod }); + onAccept({ password, method }); } else { onAccept({ password: "" }); } @@ -99,7 +101,7 @@ export default function EncryptionSettingsDialog({ >
{ }); }); + describe("when using TPM", () => { + beforeEach(() => { + props.method = EncryptionMethods.TPM; + }); + + it("allows to stop using it", async () => { + const { user } = plainRender(); + const tpmCheckbox = screen.queryByRole("checkbox", { name: /Use the TPM/ }); + const acceptButton = screen.queryByRole("button", { name: "Accept" }); + expect(tpmCheckbox).toBeChecked(); + await user.click(tpmCheckbox); + expect(tpmCheckbox).not.toBeChecked(); + await user.click(acceptButton); + expect(props.onAccept).toHaveBeenCalledWith( + expect.not.objectContaining({ method: EncryptionMethods.TPM }) + ); + }); + }); + describe("when TPM is not included in given methods", () => { beforeEach(() => { props.methods = [EncryptionMethods.LUKS2]; From bcc4e8aec9217684fbf180bbf9e0f8468d5914fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 15 Apr 2024 08:17:46 +0100 Subject: [PATCH 31/31] web: Add changelog entry --- web/package/cockpit-agama.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 6a5e0f302a..d8ddc43df9 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Apr 15 07:14:35 UTC 2024 - David Diaz + +- Enhance the storage page to make it easier to use and understand. + (gh#openSUSE/agama#1138). + ------------------------------------------------------------------- Thu Apr 11 15:16:42 UTC 2024 - José Iván López González