From 6252eebf4e7c00f33ee420f1e5b8f5b875b3167f Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 6 Mar 2024 09:54:04 +0000 Subject: [PATCH 1/5] [web] WIP: move transactional information --- web/src/components/storage/ProposalPage.jsx | 6 +- .../storage/ProposalSettingsSection.jsx | 68 ++++++------------- .../storage/ProposalSettingsSection.test.jsx | 17 ++--- .../storage/ProposalTransactionalInfo.jsx | 66 ++++++++++++++++++ .../ProposalTransactionalInfo.test.jsx | 64 +++++++++++++++++ web/src/components/storage/index.js | 1 + 6 files changed, 162 insertions(+), 60 deletions(-) create mode 100644 web/src/components/storage/ProposalTransactionalInfo.jsx create mode 100644 web/src/components/storage/ProposalTransactionalInfo.test.jsx diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index 51ab17e7bf..f39a4d9db6 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -31,7 +31,8 @@ import { ProposalSettingsSection, ProposalSpacePolicySection, ProposalDeviceSection, - ProposalFileSystemsSection + ProposalFileSystemsSection, + ProposalTransactionalInfo } from "~/components/storage"; import { IDLE } from "~/client/status"; @@ -201,6 +202,9 @@ export default function ProposalPage() { const PageContent = () => { return ( <> + { - const explanation = _("Uses Btrfs for the root file system allowing to boot to a previous \ + 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 ( - <> - + return ( + - {explanation} + +
+ {explanation} +
- - ); - }; - - return ( -
- - } /> -
+ } + /> ); }; @@ -297,8 +290,6 @@ export default function ProposalSettingsSection({ encryptionMethods = [], onChange = noop }) { - const { selectedProduct } = useProduct(); - const changeEncryption = ({ password, method }) => { onChange({ encryptionPassword: password, encryptionMethod: method }); }; @@ -318,29 +309,12 @@ export default function ProposalSettingsSection({ const encryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0; - const transactional = isTransactionalSystem(settings?.volumes || []); - return ( <>
- - -
- {/* TRANSLATORS: %s is replaced by a product name (e.g., openSUSE Tumbleweed) */} - {sprintf(_("%s is an immutable system with atomic updates using a read-only Btrfs \ -root file system."), selectedProduct.name)} -
- - } - else={ - - } + { }; }); -jest.mock("~/context/product", () => ({ - ...jest.requireActual("~/context/product"), - useProduct: () => ({ - selectedProduct : { name: "Test" } - }) -})); - let props; beforeEach(() => { @@ -48,7 +41,7 @@ beforeEach(() => { const rootVolume = { mountPath: "/", fsType: "Btrfs", outline: { snapshotsConfigurable: true } }; -describe("if the system is not transactional", () => { +describe("if snapshots are configurable", () => { beforeEach(() => { props.settings = { volumes: [rootVolume] }; }); @@ -60,15 +53,15 @@ describe("if the system is not transactional", () => { }); }); -describe("if the system is transactional", () => { +describe("if snapshots are not configurable", () => { beforeEach(() => { - props.settings = { volumes: [{ ...rootVolume, transactional: true }] }; + props.settings = { volumes: [{ ...rootVolume, outline: { ...rootVolume.outline, snapshotsConfigurable: false } }] }; }); - it("renders explanation about transactional system", () => { + it("renders the snapshots switch", () => { plainRender(); - screen.getByText("Transactional system"); + expect(screen.queryByRole("checkbox", { name: "Use Btrfs Snapshots" })).toBeNull(); }); }); diff --git a/web/src/components/storage/ProposalTransactionalInfo.jsx b/web/src/components/storage/ProposalTransactionalInfo.jsx new file mode 100644 index 0000000000..3e46a2c021 --- /dev/null +++ b/web/src/components/storage/ProposalTransactionalInfo.jsx @@ -0,0 +1,66 @@ +/* + * 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 { Alert } from "@patternfly/react-core"; +import { sprintf } from "sprintf-js"; + +import { _ } from "~/i18n"; +import { If, Section } from "~/components/core"; +import { isTransactionalSystem } from "~/components/storage/utils"; +import { useProduct } from "~/context/product"; + +/** + * @typedef {import ("~/client/storage").ProposalManager.ProposalSettings} ProposalSettings + */ + +/** + * Information about the system being transactional, if needed + * @component + * + * @param {object} props + * @param {ProposalSettings} props.settings - Settings used for calculating a proposal. + * @param {object} settings + */ +export default function ProposalTransactionalInfo({ settings }) { + const transactional = isTransactionalSystem(settings?.volumes || []); + const { selectedProduct } = useProduct(); + + /* TRANSLATORS: %s is replaced by a product name (e.g., openSUSE Tumbleweed) */ + const description = sprintf( + _("%s is an immutable system with atomic updates. It uses a read-only Btrfs file system updated via snapshots."), + selectedProduct.name + ); + const title = _("Transactional root file system"); + + return ( + + + {description} + +
+ } + /> + ); +} diff --git a/web/src/components/storage/ProposalTransactionalInfo.test.jsx b/web/src/components/storage/ProposalTransactionalInfo.test.jsx new file mode 100644 index 0000000000..8dd6838cd5 --- /dev/null +++ b/web/src/components/storage/ProposalTransactionalInfo.test.jsx @@ -0,0 +1,64 @@ +/* + * 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, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { ProposalTransactionalInfo } from "~/components/storage"; + +jest.mock("~/context/product", () => ({ + ...jest.requireActual("~/context/product"), + useProduct: () => ({ + selectedProduct : { name: "Test" } + }) +})); + +let props; + +beforeEach(() => { + props = {}; +}); + +const rootVolume = { mountPath: "/", fsType: "Btrfs" }; + +describe("if the system is not transactional", () => { + beforeEach(() => { + props.settings = { volumes: [rootVolume] }; + }); + + it("does not render any explanation about transactional system", () => { + plainRender(); + + expect(screen.queryByText("Transactional root file system")).toBeNull(); + }); +}); + +describe("if the system is transactional", () => { + beforeEach(() => { + props.settings = { volumes: [{ ...rootVolume, transactional: true }] }; + }); + + it("renders an explanation about the transactional system", () => { + plainRender(); + + screen.getByText("Transactional root file system"); + }); +}); diff --git a/web/src/components/storage/index.js b/web/src/components/storage/index.js index 6c518c51a0..d4dd30a180 100644 --- a/web/src/components/storage/index.js +++ b/web/src/components/storage/index.js @@ -26,6 +26,7 @@ export { default as ProposalSpacePolicySection } from "./ProposalSpacePolicySect export { default as ProposalDeviceSection } from "./ProposalDeviceSection"; export { default as ProposalFileSystemsSection } from "./ProposalFileSystemsSection"; export { default as ProposalActionsSection } from "./ProposalActionsSection"; +export { default as ProposalTransactionalInfo } from "./ProposalTransactionalInfo"; export { default as ProposalVolumes } from "./ProposalVolumes"; export { default as DASDPage } from "./DASDPage"; export { default as DASDTable } from "./DASDTable"; From 17a40da47792eac909273131061e74c352a61632 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 7 Mar 2024 11:50:52 +0000 Subject: [PATCH 2/5] [web] Fixes from code review --- web/src/components/storage/ProposalTransactionalInfo.jsx | 1 - .../storage/ProposalTransactionalInfo.test.jsx | 9 ++++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/web/src/components/storage/ProposalTransactionalInfo.jsx b/web/src/components/storage/ProposalTransactionalInfo.jsx index 3e46a2c021..daabd72b08 100644 --- a/web/src/components/storage/ProposalTransactionalInfo.jsx +++ b/web/src/components/storage/ProposalTransactionalInfo.jsx @@ -38,7 +38,6 @@ import { useProduct } from "~/context/product"; * * @param {object} props * @param {ProposalSettings} props.settings - Settings used for calculating a proposal. - * @param {object} settings */ export default function ProposalTransactionalInfo({ settings }) { const transactional = isTransactionalSystem(settings?.volumes || []); diff --git a/web/src/components/storage/ProposalTransactionalInfo.test.jsx b/web/src/components/storage/ProposalTransactionalInfo.test.jsx index 8dd6838cd5..e9556107fa 100644 --- a/web/src/components/storage/ProposalTransactionalInfo.test.jsx +++ b/web/src/components/storage/ProposalTransactionalInfo.test.jsx @@ -20,7 +20,7 @@ */ import React from "react"; -import { screen, within } from "@testing-library/react"; +import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { ProposalTransactionalInfo } from "~/components/storage"; @@ -44,10 +44,9 @@ describe("if the system is not transactional", () => { props.settings = { volumes: [rootVolume] }; }); - it("does not render any explanation about transactional system", () => { - plainRender(); - - expect(screen.queryByText("Transactional root file system")).toBeNull(); + it("renders nothing", () => { + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); }); }); From 0acb332ad409cbca3878622dfdd6c4e850756ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 7 Mar 2024 13:35:23 +0000 Subject: [PATCH 3/5] web: Add core/Reminder component Inspired in PF/Alert component, but much more simpler. --- web/src/assets/styles/blocks.scss | 21 ++++++ web/src/components/core/Reminder.jsx | 84 +++++++++++++++++++++++ web/src/components/core/Reminder.test.jsx | 65 ++++++++++++++++++ web/src/components/core/index.js | 1 + 4 files changed, 171 insertions(+) create mode 100644 web/src/components/core/Reminder.jsx create mode 100644 web/src/components/core/Reminder.test.jsx diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 30e95e4aa4..e568ae24db 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -469,6 +469,27 @@ ul[data-type="agama/list"][role="grid"] { } } +[data-type="agama/reminder"] { + --accent-color: var(--color-primary-lighter); + --inline-margin: calc(var(--header-icon-size) + var(--spacer-small)); + + display: flex; + gap: var(--spacer-small); + margin-inline: var(--inline-margin); + margin-block-end: var(--spacer-normal); + padding: var(--spacer-smaller) var(--spacer-small); + border-inline-start: 3px solid var(--accent-color); + + svg { + fill: var(--accent-color); + } + + h4 { + color: var(--accent-color); + margin-block-end: var(--spacer-smaller); + } +} + [role="dialog"] { section:not([class^="pf-c"]) { > svg:first-child { diff --git a/web/src/components/core/Reminder.jsx b/web/src/components/core/Reminder.jsx new file mode 100644 index 0000000000..997e870a91 --- /dev/null +++ b/web/src/components/core/Reminder.jsx @@ -0,0 +1,84 @@ +/* + * 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"; + +/** + * Internal component for rendering the icon + * + * @param {object} props + * @params {string} [props.name] - The icon name. + */ +const ReminderIcon = ({ name }) => { + if (!name?.length) return; + + return ( +
+ +
+ ); +}; + +/** + * Internal component for rendering the title + * + * @param {object} props + * @params {JSX.Element|string} [props.children] - The title content. + */ +const ReminderTitle = ({ children }) => { + if (!children) return; + if (typeof children === "string" && !children.length) return; + + return ( +

{children}

+ ); +}; + +/** + * Renders a reminder with given role, status by default + * @component + * + * @param {object} props + * @param {string} [props.icon] - The name of desired icon. + * @param {JSX.Element|string} [props.title] - The content for the title. + * @param {string} [props.role="status"] - The reminder's role, "status" by + * default. See {@link https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/status_role} + * @param {JSX.Element} [props.children] - The content for the description. + */ +export default function Reminder ({ + icon, + title, + role = "status", + children +}) { + return ( +
+ +
+ {title} + { children } +
+
+ ); +} diff --git a/web/src/components/core/Reminder.test.jsx b/web/src/components/core/Reminder.test.jsx new file mode 100644 index 0000000000..24527cf2d0 --- /dev/null +++ b/web/src/components/core/Reminder.test.jsx @@ -0,0 +1,65 @@ +/* + * Copyright (c) [2023] 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, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import { Reminder } from "~/components/core"; + +describe("Reminder", () => { + it("renders a status region by default", () => { + plainRender(Example); + const reminder = screen.getByRole("status"); + within(reminder).getByText("Example"); + }); + + it("renders a region with given role", () => { + plainRender(Example); + const reminder = screen.getByRole("alert"); + within(reminder).getByText("Example"); + }); + + it("renders given title", () => { + plainRender( + Kindly reminder}> + Visit the settings section + + ); + screen.getByRole("heading", { name: "Kindly reminder", level: 4 }); + }); + + it("does not render a heading if title is not given", () => { + plainRender(Without title); + expect(screen.queryByRole("heading")).toBeNull(); + }); + + it("does not render a heading if title is an empty string", () => { + plainRender(Without title); + expect(screen.queryByRole("heading")).toBeNull(); + }); + + it("renders given children", () => { + plainRender( + Visit the settings section + ); + screen.getByRole("link", { name: "Visit the settings section" }); + }); +}); diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index 0213c66215..6bdac86248 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -57,3 +57,4 @@ export { default as PasswordInput } from "./PasswordInput"; export { default as DevelopmentInfo } from "./DevelopmentInfo"; export { default as Selector } from "./Selector"; export { default as OptionsPicker } from "./OptionsPicker"; +export { default as Reminder } from "./Reminder"; From 9ab4bf15786565fc8a04231cdf92bd5fb0a5fb97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 7 Mar 2024 15:16:12 +0000 Subject: [PATCH 4/5] web: Refactor ProposalTransactionalInfo component To make use of core/Reminder and return undefined as soon as possible. Commit also add few changes to helper method. --- .../storage/ProposalTransactionalInfo.jsx | 23 +++++-------------- web/src/components/storage/utils.js | 8 +++++-- web/src/components/storage/utils.test.js | 8 +++++++ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/web/src/components/storage/ProposalTransactionalInfo.jsx b/web/src/components/storage/ProposalTransactionalInfo.jsx index daabd72b08..aa20477a6d 100644 --- a/web/src/components/storage/ProposalTransactionalInfo.jsx +++ b/web/src/components/storage/ProposalTransactionalInfo.jsx @@ -20,11 +20,10 @@ */ import React from "react"; -import { Alert } from "@patternfly/react-core"; -import { sprintf } from "sprintf-js"; +import { sprintf } from "sprintf-js"; import { _ } from "~/i18n"; -import { If, Section } from "~/components/core"; +import { Reminder } from "~/components/core"; import { isTransactionalSystem } from "~/components/storage/utils"; import { useProduct } from "~/context/product"; @@ -40,26 +39,16 @@ import { useProduct } from "~/context/product"; * @param {ProposalSettings} props.settings - Settings used for calculating a proposal. */ export default function ProposalTransactionalInfo({ settings }) { - const transactional = isTransactionalSystem(settings?.volumes || []); const { selectedProduct } = useProduct(); + if (!isTransactionalSystem(settings?.volumes)) return; + + const title = _("Transactional root file system"); /* TRANSLATORS: %s is replaced by a product name (e.g., openSUSE Tumbleweed) */ const description = sprintf( _("%s is an immutable system with atomic updates. It uses a read-only Btrfs file system updated via snapshots."), selectedProduct.name ); - const title = _("Transactional root file system"); - return ( - - - {description} - - - } - /> - ); + return {description}; } diff --git a/web/src/components/storage/utils.js b/web/src/components/storage/utils.js index d2322bc167..082756b08c 100644 --- a/web/src/components/storage/utils.js +++ b/web/src/components/storage/utils.js @@ -180,8 +180,12 @@ const isTransactionalRoot = (volume) => { * @param {Volume[]} volumes * @returns {boolean} */ -const isTransactionalSystem = (volumes) => { - return volumes.find(v => isTransactionalRoot(v)) !== undefined; +const isTransactionalSystem = (volumes = []) => { + try { + return volumes?.find(v => isTransactionalRoot(v)) !== undefined; + } catch { + return false; + } }; export { diff --git a/web/src/components/storage/utils.test.js b/web/src/components/storage/utils.test.js index a973e27a0a..e5eb3cf0aa 100644 --- a/web/src/components/storage/utils.test.js +++ b/web/src/components/storage/utils.test.js @@ -139,6 +139,14 @@ describe("isTransactionalRoot", () => { }); describe("isTransactionalSystem", () => { + it("returns false when a list of volumes is not given", () => { + expect(isTransactionalSystem(false)).toBe(false); + expect(isTransactionalSystem(undefined)).toBe(false); + expect(isTransactionalSystem(null)).toBe(false); + expect(isTransactionalSystem([])).toBe(false); + expect(isTransactionalSystem("fake")).toBe(false); + }); + it("returns false if volumes does not include a transactional root", () => { expect(isTransactionalSystem([])).toBe(false); From 338b8e50ed9c15de17cb4989f57a658fec7c697c Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 7 Mar 2024 15:47:48 +0000 Subject: [PATCH 5/5] [web] Small code improvement at ProposalSettingsSection --- .../storage/ProposalSettingsSection.jsx | 31 ++++++++----------- .../storage/ProposalSettingsSection.test.jsx | 2 +- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx index c898e82d48..abc6830308 100644 --- a/web/src/components/storage/ProposalSettingsSection.jsx +++ b/web/src/components/storage/ProposalSettingsSection.jsx @@ -143,29 +143,24 @@ const SnapshotsField = ({ onChange({ active: checked, settings }); }; - const configurableSnapshots = rootVolume.outline.snapshotsConfigurable; + 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} -
- - } - /> +
+ +
+ {explanation} +
+
); }; diff --git a/web/src/components/storage/ProposalSettingsSection.test.jsx b/web/src/components/storage/ProposalSettingsSection.test.jsx index 96b22c4404..3077cd7903 100644 --- a/web/src/components/storage/ProposalSettingsSection.test.jsx +++ b/web/src/components/storage/ProposalSettingsSection.test.jsx @@ -58,7 +58,7 @@ describe("if snapshots are not configurable", () => { props.settings = { volumes: [{ ...rootVolume, outline: { ...rootVolume.outline, snapshotsConfigurable: false } }] }; }); - it("renders the snapshots switch", () => { + it("does not render the snapshots switch", () => { plainRender(); expect(screen.queryByRole("checkbox", { name: "Use Btrfs Snapshots" })).toBeNull();