diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 5902146b4a..6c06495717 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Thu Jan 04 21:44:32 UTC 2024 - Balsa Asanovic + +- Removing global issues page and using popup dialog instead. + It shows issues only from a specific category (software, + product, storage, ...) and not all at once. (gh#openSUSE/agama#886). + ------------------------------------------------------------------- Tue Dec 26 11:12:45 UTC 2023 - David Diaz diff --git a/web/src/assets/styles/blocks.scss b/web/src/assets/styles/blocks.scss index 66775b89ef..8141e1ad7e 100644 --- a/web/src/assets/styles/blocks.scss +++ b/web/src/assets/styles/blocks.scss @@ -442,3 +442,16 @@ ul[data-of="agama/timezones"] { text-align: end; } } + +[role="dialog"] { + section:not([class^="pf-c"]) { + > svg:first-child { + block-size: 24px; + inline-size: 24px; + } + + h2 { + font-size: var(--fs-h3); + } + } +} diff --git a/web/src/assets/styles/patternfly-overrides.scss b/web/src/assets/styles/patternfly-overrides.scss index 47afc4c496..5e2d2f9fe5 100644 --- a/web/src/assets/styles/patternfly-overrides.scss +++ b/web/src/assets/styles/patternfly-overrides.scss @@ -236,6 +236,15 @@ table td > .pf-v5-c-empty-state { padding-inline: 0; } +ul { + list-style: initial; + margin-inline: var(--spacer-normal); + + li:not(:last-child) { + margin-block-end: var(--spacer-small); + } +} + @media screen and (width <= 768px) { .pf-m-grid-md.pf-v5-c-table tr:where(.pf-v5-c-table__tr):not(.pf-v5-c-table__expandable-row) { padding-inline: 0; diff --git a/web/src/assets/styles/variables.scss b/web/src/assets/styles/variables.scss index 12fb205916..fb96b3058f 100644 --- a/web/src/assets/styles/variables.scss +++ b/web/src/assets/styles/variables.scss @@ -13,6 +13,7 @@ --fs-large: 1rem; --fs-h1: 1.5rem; --fs-h2: 1.2rem; + --fs-h3: 1rem; --lh-normal: 1.5; --lh-medium: 1.6; diff --git a/web/src/components/core/InstallButton.jsx b/web/src/components/core/InstallButton.jsx index dbd348d85c..ed969a0ac1 100644 --- a/web/src/components/core/InstallButton.jsx +++ b/web/src/components/core/InstallButton.jsx @@ -23,7 +23,6 @@ import React, { useState } from "react"; import { useInstallerClient } from "~/context/installer"; import { Button } from "@patternfly/react-core"; -import { Link } from "react-router-dom"; import { If, Popup } from "~/components/core"; import { _ } from "~/i18n"; @@ -31,22 +30,20 @@ import { _ } from "~/i18n"; const InstallConfirmationPopup = ({ hasIssues, onAccept, onClose }) => { const IssuesWarning = () => { // TRANSLATORS: the installer reports some errors, - // the text in square brackets [] is a clickable link - const [msgStart, msgLink, msgEnd] = _("There are some reported issues. \ -Please, check [the list of issues] \ -before proceeding with the installation.").split(/[[\]]/); return (

- {msgStart} - {msgLink} - {msgEnd} + { _("There are some reported issues. \ +Please review them in the previous steps before proceeding with the installation.") }

); }; + const Cancel = hasIssues ? Popup.PrimaryAction : Popup.SecondaryAction; + const Continue = hasIssues ? Popup.SecondaryAction : Popup.PrimaryAction; + return ( - + + {/* TRANSLATORS: button label */} + {_("Cancel")} + + {/* TRANSLATORS: button label */} {_("Continue")} - - + ); diff --git a/web/src/components/core/InstallButton.test.jsx b/web/src/components/core/InstallButton.test.jsx index 4e0937d7b0..6762fa0afb 100644 --- a/web/src/components/core/InstallButton.test.jsx +++ b/web/src/components/core/InstallButton.test.jsx @@ -85,23 +85,22 @@ describe("when the button is clicked and there are not errors", () => { }; }); - it("shows a link to go to the issues page", async () => { + it("shows a message encouraging the user to review them", async () => { const { user } = installerRender(); const button = await screen.findByRole("button", { name: "Install" }); await user.click(button); - - await screen.findByRole("link", { name: /list of issues$/ }); + await screen.findByText(/There are some reported issues/); }); }); describe("if there are not issues", () => { - it("does not show a link to go to the issues page", async () => { + it("doest not show the message encouraging the user to review them", async () => { const { user } = installerRender(); const button = await screen.findByRole("button", { name: "Install" }); await user.click(button); await waitFor(() => { - const link = screen.queryByRole("link", { name: /list of issues$/ }); - expect(link).toBeNull(); + const text = screen.queryByText(/There are some reported issues/); + expect(text).toBeNull(); }); }); }); diff --git a/web/src/components/core/IssuesDialog.jsx b/web/src/components/core/IssuesDialog.jsx new file mode 100644 index 0000000000..42e30d20b2 --- /dev/null +++ b/web/src/components/core/IssuesDialog.jsx @@ -0,0 +1,123 @@ +/* + * 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, { useCallback, useEffect, useState } from "react"; + +import { partition, useCancellablePromise } from "~/utils"; +import { If, Popup } from "~/components/core"; +import { Icon } from "~/components/layout"; +import { useInstallerClient } from "~/context/installer"; +import { _ } from "~/i18n"; + +/** + * Item representing an issue. + * @component + * + * @param {object} props + * @param {import ("~/client/mixins").Issue} props.issue + */ +const IssueItem = ({ issue }) => { + const hasDetails = issue.details.length > 0; + + return ( +
  • + {issue.description} + {issue.details}} + /> +
    • + ); +}; + +/** + * Generates issue items sorted by severity. + * @component + * + * @param {object} props + * @param {import ("~/client/mixins").Issue[]} props.issues + */ +const IssueItems = ({ issues = [] }) => { + const sortedIssues = partition(issues, i => i.severity === "error").flat(); + + const items = sortedIssues.map((issue, index) => { + return ; + }); + + return
      {items}
    ; +}; + +/** + * Popup to show more issues details from the installation overview page. + * + * It initially shows a loading state, + * then fetches and displays a list of issues of the selected category, either 'product' or 'storage' or 'software'. + * + * It uses a Popup component to display the issues, and an If component to toggle between + * a loading state and the content state. + * + * @component + * + * @param {object} props + * @param {boolean} [props.isOpen] - A boolean value used to determine wether to show the popup or not. + * @param {function} props.onClose - A function to call when the close action is triggered. + * @param {string} props.sectionId - A string which indicates what type of issues are going to be shown in the popup. + * @param {string} props.title - Title of the popup. + */ +export default function IssuesDialog({ isOpen = false, onClose, sectionId, title }) { + const [isLoading, setIsLoading] = useState(true); + const [issues, setIssues] = useState([]); + const client = useInstallerClient(); + const { cancellablePromise } = useCancellablePromise(); + + const load = useCallback(async () => { + setIsLoading(true); + const issues = await cancellablePromise(client.issues()); + setIsLoading(false); + return issues; + }, [client, cancellablePromise, setIsLoading]); + + const update = useCallback((issues) => { + setIssues(current => ([...current, ...(issues[sectionId] || [])])); + }, [setIssues, sectionId]); + + useEffect(() => { + load().then(update); + return client.onIssuesChange(update); + }, [client, load, update]); + + return ( + + } + else={} + /> + + {_("Close")} + + + ); +} diff --git a/web/src/components/core/IssuesDialog.test.jsx b/web/src/components/core/IssuesDialog.test.jsx new file mode 100644 index 0000000000..df3ea86970 --- /dev/null +++ b/web/src/components/core/IssuesDialog.test.jsx @@ -0,0 +1,73 @@ +/* + * 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 } from "@testing-library/react"; +import { installerRender } from "~/test-utils"; +import { createClient } from "~/client"; +import { IssuesDialog } from "~/components/core"; + +jest.mock("~/client"); +jest.mock("@patternfly/react-core", () => { + return { + ...jest.requireActual("@patternfly/react-core"), + Skeleton: () =>
    PFSkeleton
    + }; +}); +jest.mock("~/components/core/Sidebar", () => () =>
    Agama sidebar
    ); + +const issues = { + product: [], + storage: [ + { description: "storage issue 1", details: "Details 1", source: "system", severity: "warn" }, + { description: "storage issue 2", details: "Details 2", source: "config", severity: "error" } + ], + software: [ + { description: "software issue 1", details: "Details 1", source: "system", severity: "warn" } + ] +}; + +let mockIssues; + +beforeEach(() => { + mockIssues = { ...issues }; + + createClient.mockImplementation(() => { + return { + issues: jest.fn().mockResolvedValue(mockIssues), + onIssuesChange: jest.fn() + }; + }); +}); + +it("loads the issues", async () => { + installerRender(); + + await screen.findByText(/storage issue 1/); +}); + +it('calls onClose callback when close button is clicked', async () => { + const mockOnClose = jest.fn(); + const { user } = installerRender(); + + await user.click(screen.getByText("Close")); + expect(mockOnClose).toHaveBeenCalled(); +}); diff --git a/web/src/components/core/IssuesLink.jsx b/web/src/components/core/IssuesLink.jsx deleted file mode 100644 index ca9c851267..0000000000 --- a/web/src/components/core/IssuesLink.jsx +++ /dev/null @@ -1,47 +0,0 @@ -/* - * 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 { Link } from "react-router-dom"; - -import { Icon } from "~/components/layout"; -import { If, NotificationMark } from "~/components/core"; -import { useNotification } from "~/context/notification"; -import { _ } from "~/i18n"; - -/** - * Link to go to the issues page - * @component - */ -export default function IssuesLink() { - const [notification] = useNotification(); - - return ( - - - {_("Show issues")} - } - /> - - ); -} diff --git a/web/src/components/core/IssuesLink.test.jsx b/web/src/components/core/IssuesLink.test.jsx deleted file mode 100644 index 8073276beb..0000000000 --- a/web/src/components/core/IssuesLink.test.jsx +++ /dev/null @@ -1,72 +0,0 @@ -/* - * 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 { installerRender, withNotificationProvider } from "~/test-utils"; -import { createClient } from "~/client"; -import { IssuesLink } from "~/components/core"; - -let mockIssues = {}; - -jest.mock("~/client"); - -beforeEach(() => { - createClient.mockImplementation(() => { - return { - issues: jest.fn().mockResolvedValue(mockIssues), - onIssuesChange: jest.fn() - }; - }); -}); - -it("renders a link for navigating to the issues page", async () => { - installerRender(withNotificationProvider()); - const link = await screen.findByRole("link", { name: "Show issues" }); - expect(link).toHaveAttribute("href", "/issues"); -}); - -describe("if there are issues", () => { - beforeEach(() => { - mockIssues = { - storage: [{ description: "issue 1" }] - }; - }); - - it("includes a notification mark", async () => { - installerRender(withNotificationProvider()); - const link = await screen.findByRole("link", { name: /new issues/ }); - within(link).getByRole("status", { name: /new issues/ }); - }); -}); - -describe("if there are not issues", () => { - beforeEach(() => { - mockIssues = {}; - }); - - it("does not include a notification mark", async () => { - installerRender(withNotificationProvider()); - const link = await screen.findByRole("link", { name: "Show issues" }); - const mark = within(link).queryByRole("status", { name: /new issues/ }); - expect(mark).toBeNull(); - }); -}); diff --git a/web/src/components/core/IssuesPage.jsx b/web/src/components/core/IssuesPage.jsx deleted file mode 100644 index 8a6fa853ee..0000000000 --- a/web/src/components/core/IssuesPage.jsx +++ /dev/null @@ -1,184 +0,0 @@ -/* - * 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, { useCallback, useEffect, useState } from "react"; - -import { HelperText, HelperTextItem } from "@patternfly/react-core"; - -import { partition, useCancellablePromise } from "~/utils"; -import { If, Page, Section, SectionSkeleton } from "~/components/core"; -import { Icon } from "~/components/layout"; -import { useInstallerClient } from "~/context/installer"; -import { useNotification } from "~/context/notification"; -import { _ } from "~/i18n"; - -/** - * Item representing an issue. - * @component - * - * @param {object} props - * @param {import ("~/client/mixins").Issue} props.issue - */ -const IssueItem = ({ issue }) => { - const variant = issue.severity === "warn" ? "warning" : "error"; - const icon = issue.severity === "warn" ? "warning" : "error"; - const hasDetails = issue.details.length > 0; - - return ( -
    - - }> - {issue.description} - - 
    {issue.details}
    } - /> -     -
    - ); -}; - -/** - * Generates issue items sorted by severity. - * @component - * - * @param {object} props - * @param {import ("~/client/mixins").Issue[]} props.issues - */ -const IssueItems = ({ issues = [] }) => { - const sortedIssues = partition(issues, i => i.severity === "error").flat(); - - return sortedIssues.map((issue, index) => { - return ; - }); -}; - -/** - * Generates the sections with issues. - * @component - * - * @param {object} props - * @param {import ("~/client/issues").ClientsIssues} props.issues - */ -const IssuesSections = ({ issues }) => { - const productIssues = issues.product || []; - const storageIssues = issues.storage || []; - const softwareIssues = issues.software || []; - - return ( - <> - 0} - then={ -
    - -
    - } - /> - 0} - then={ -
    - -
    - } - /> - 0} - then={ -
    - -
    - } - /> - - ); -}; - -/** - * Generates sections with issues. If there are no issues, then a success message is shown. - * @component - * - * @param {object} props - * @param {import ("~/client").Issues} props.issues - */ -const IssuesContent = ({ issues }) => { - const NoIssues = () => { - return ( - - }> - {_("No issues found. Everything looks ok.")} - - - ); - }; - - const allIssues = Object.values(issues).flat(); - - return ( - } - else={} - /> - ); -}; - -/** - * Page to show all issues. - * @component - */ -export default function IssuesPage() { - const [isLoading, setIsLoading] = useState(true); - const [issues, setIssues] = useState(); - const client = useInstallerClient(); - const { cancellablePromise } = useCancellablePromise(); - const [notification, updateNotification] = useNotification(); - - const load = useCallback(async () => { - setIsLoading(true); - const issues = await cancellablePromise(client.issues()); - setIsLoading(false); - return issues; - }, [client, cancellablePromise, setIsLoading]); - - const update = useCallback((issues) => { - setIssues(current => ({ ...current, ...issues })); - if (notification.issues) updateNotification({ issues: false }); - }, [notification, setIssues, updateNotification]); - - useEffect(() => { - load().then(update); - return client.onIssuesChange(update); - }, [client, load, update]); - - return ( - // TRANSLATORS: page title - - } - else={} - /> - - ); -} diff --git a/web/src/components/core/IssuesPage.test.jsx b/web/src/components/core/IssuesPage.test.jsx deleted file mode 100644 index 4ffeb7fb75..0000000000 --- a/web/src/components/core/IssuesPage.test.jsx +++ /dev/null @@ -1,112 +0,0 @@ -/* - * 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 { act, screen, waitFor, within } from "@testing-library/react"; -import { installerRender, createCallbackMock, withNotificationProvider } from "~/test-utils"; -import { createClient } from "~/client"; -import { IssuesPage } from "~/components/core"; - -jest.mock("~/client"); -jest.mock("@patternfly/react-core", () => { - return { - ...jest.requireActual("@patternfly/react-core"), - Skeleton: () =>
    PFSkeleton
    - }; -}); -jest.mock("~/components/core/Sidebar", () => () =>
    Agama sidebar
    ); - -const issues = { - product: [], - storage: [ - { description: "storage issue 1", details: "Details 1", source: "system", severity: "warn" }, - { description: "storage issue 2", details: "Details 2", source: "config", severity: "error" } - ], - software: [ - { description: "software issue 1", details: "Details 1", source: "system", severity: "warn" } - ] -}; - -let mockIssues; - -let mockOnIssuesChange; - -beforeEach(() => { - mockIssues = { ...issues }; - mockOnIssuesChange = jest.fn(); - - createClient.mockImplementation(() => { - return { - issues: jest.fn().mockResolvedValue(mockIssues), - onIssuesChange: mockOnIssuesChange - }; - }); -}); - -it("loads the issues", async () => { - installerRender(withNotificationProvider()); - - screen.getAllByText(/PFSkeleton/); - await screen.findByText(/storage issue 1/); -}); - -it("renders sections with issues", async () => { - installerRender(withNotificationProvider()); - - await waitFor(() => expect(screen.queryByText("Product")).not.toBeInTheDocument()); - - const storageSection = await screen.findByText(/Storage/); - within(storageSection).findByText(/storage issue 1/); - within(storageSection).findByText(/storage issue 2/); - - const softwareSection = await screen.findByText(/Software/); - within(softwareSection).findByText(/software issue 1/); -}); - -describe("if there are not issues", () => { - beforeEach(() => { - mockIssues = { product: [], storage: [], software: [] }; - }); - - it("renders a success message", async () => { - installerRender(withNotificationProvider()); - - await screen.findByText(/No issues found/); - }); -}); - -describe("if the issues change", () => { - it("shows the new issues", async () => { - const [mockFunction, callbacks] = createCallbackMock(); - mockOnIssuesChange = mockFunction; - - installerRender(withNotificationProvider()); - - await screen.findByText("Storage"); - - mockIssues.storage = []; - act(() => callbacks.forEach(c => c({ storage: mockIssues.storage }))); - - await waitFor(() => expect(screen.queryByText("Storage")).not.toBeInTheDocument()); - const softwareSection = await screen.findByText(/Software/); - within(softwareSection).findByText(/software issue 1/); - }); -}); diff --git a/web/src/components/core/Section.jsx b/web/src/components/core/Section.jsx index 5f65d6479f..0a6b755740 100644 --- a/web/src/components/core/Section.jsx +++ b/web/src/components/core/Section.jsx @@ -52,6 +52,8 @@ import { ValidationErrors } from "~/components/core"; * @property {string} [path] - Path where the section links to. * when user clicks on the title, used for opening a dialog. * @property {boolean} [loading] - Whether the section is busy loading its content or not. + * @property {string} [className] - Class name for section html tag. + * @property {string} [id] - Id of the section ("software", "product", "storage", "storage-actions", ...) * @property {import("~/client/mixins").ValidationError[]} [props.errors] - Validation errors to be shown before the title. * @property {React.ReactElement} [children] - The section content. * @property {string} [aria-label] - aria-label attribute, required if title if not given @@ -64,6 +66,8 @@ export default function Section({ name, path, loading, + className, + id, errors, children, "aria-label": ariaLabel @@ -88,16 +92,17 @@ export default function Section({ return (
    {errors?.length > 0 && - } + } {children}
    diff --git a/web/src/components/core/Section.test.jsx b/web/src/components/core/Section.test.jsx index da9c53bfd8..d7d5db2a81 100644 --- a/web/src/components/core/Section.test.jsx +++ b/web/src/components/core/Section.test.jsx @@ -121,7 +121,7 @@ describe("Section", () => { it("renders given errors", () => { plainRender( -
    +
    ); screen.getByText("Something went wrong"); diff --git a/web/src/components/core/Sidebar.jsx b/web/src/components/core/Sidebar.jsx index 1d9fd88e22..2c499c0183 100644 --- a/web/src/components/core/Sidebar.jsx +++ b/web/src/components/core/Sidebar.jsx @@ -26,7 +26,6 @@ import { About, DevelopmentInfo, Disclosure, - IssuesLink, // FIXME: unify names here by renaming LogsButton -> LogButton or ShowLogButton -> ShowLogsButton LogsButton, ShowLogButton, @@ -134,7 +133,6 @@ export default function Sidebar ({ children, isOpen, onClose = noop }) {
    - diff --git a/web/src/components/core/Sidebar.test.jsx b/web/src/components/core/Sidebar.test.jsx index 577c50abc5..5d714ea32d 100644 --- a/web/src/components/core/Sidebar.test.jsx +++ b/web/src/components/core/Sidebar.test.jsx @@ -27,7 +27,6 @@ import { If, Sidebar } from "~/components/core"; // Mock some components jest.mock("~/components/core/About", () => () =>
    About link mock
    ); jest.mock("~/components/core/DevelopmentInfo", () => () =>
    DevelopmentInfo mock
    ); -jest.mock("~/components/core/IssuesLink", () => () =>
    IssuesLink mock
    ); jest.mock("~/components/core/LogsButton", () => () =>
    LogsButton mock
    ); jest.mock("~/components/core/ShowLogButton", () => () =>
    ShowLogButton mock
    ); jest.mock("~/components/core/ShowTerminalButton", () => () =>
    ShowTerminalButton mock
    ); diff --git a/web/src/components/core/ValidationErrors.jsx b/web/src/components/core/ValidationErrors.jsx index b322afa46b..01c70cf781 100644 --- a/web/src/components/core/ValidationErrors.jsx +++ b/web/src/components/core/ValidationErrors.jsx @@ -22,86 +22,72 @@ // @ts-check import React, { useState } from "react"; -import { - Button, - List, - ListItem, - Popover -} from "@patternfly/react-core"; import { sprintf } from "sprintf-js"; -import { Icon } from '~/components/layout'; import { _, n_ } from "~/i18n"; +import { IssuesDialog } from "~/components/core"; /** - * @param {import("~/client/mixins").ValidationError[]} errors - Validation errors - * @return React.JSX - */ -const popoverContent = (errors) => { - const items = errors.map((e, i) => {e.message}); - return ( - {items} - ); -}; - -/** - * Displays a list of validation errors + * Displays validation errors for given section * * When there is only one error, it displays its message. Otherwise, it displays a generic message - * and the details in an Popover component. + * which can be clicked to see more details in a popup dialog. * - * @component + * @note It will retrieve issues for the area matching the first part of the + * given sectionId. I.e., given an `storage-actions` id it will retrieve and + * display issues for the `storage` area. If `software-patterns-conflicts` is + * given instead, it will retrieve and display errors for the `software` area. * - * @todo This component might be more generic. - * @todo Improve the contents of the Popover (e.g., using a list) + * @component * * @param {object} props - * @param {string} props.title - A title for the Popover + * @param {string} props.sectionId - Id of the section which is displaying errors. ("product", "software", "storage", "storage-actions", ...) * @param {import("~/client/mixins").ValidationError[]} props.errors - Validation errors */ -const ValidationErrors = ({ title = _("Errors"), errors }) => { - const [popoverVisible, setPopoverVisible] = useState(false); +const ValidationErrors = ({ errors, sectionId: sectionKey }) => { + const [showIssuesPopUp, setShowIssuesPopUp] = useState(false); - if (!errors || errors.length === 0) return null; + const [sectionId,] = sectionKey?.split("-") || ""; + const dialogTitles = { + // TRANSLATORS: Titles used for the popup displaying found section issues + software: _("Software issues"), + product: _("Product issues"), + storage: _("Storage issues") + }; + const dialogTitle = dialogTitles[sectionId] || _("Found Issues"); - const warningIcon = ; + if (!errors || errors.length === 0) return null; if (errors.length === 1) { return ( -
    {warningIcon} {errors[0].message}
    - ); - } else { - return ( - <> -
    - - setPopoverVisible(false)} - shouldOpen={() => setPopoverVisible(true)} - aria-label={_("Basic popover")} - headerContent={title} - bodyContent={popoverContent(errors)} - > -
    - +
    {errors[0].message}
    ); } + + return ( +
    + + + setShowIssuesPopUp(false)} + sectionId={sectionId} + title={dialogTitle} + /> +
    + ); }; export default ValidationErrors; diff --git a/web/src/components/core/ValidationErrors.test.jsx b/web/src/components/core/ValidationErrors.test.jsx index b7fe95f52c..74fdfda223 100644 --- a/web/src/components/core/ValidationErrors.test.jsx +++ b/web/src/components/core/ValidationErrors.test.jsx @@ -22,32 +22,48 @@ import React from "react"; import { screen, waitFor } from "@testing-library/react"; import { plainRender } from "~/test-utils"; -import ValidationErrors from "./ValidationErrors"; +import { ValidationErrors } from "~/components/core"; + +jest.mock("~/components/core/IssuesDialog", () => ({ isOpen }) => isOpen &&
    IssuesDialog
    ); + +let issues = []; + +describe("when there are no errors", () => { + it("renders nothing", async () => { + const { container } = plainRender(); + await waitFor(() => expect(container).toBeEmptyDOMElement()); + }); +}); describe("when there is a single error", () => { + beforeEach(() => { + issues = [{ severity: 0, message: "It is wrong" }]; + }); + it("renders a list containing the given errors", () => { - const errors = [ - { severity: 0, message: "It is wrong" }, - ]; - plainRender(); + plainRender(); expect(screen.queryByText("It is wrong")).toBeInTheDocument(); }); }); describe("when there are multiple errors", () => { - it("renders a list containing the given errors", async () => { - const errors = [ + beforeEach(() => { + issues = [ { severity: 0, message: "It is wrong" }, { severity: 1, message: "It might be better" } ]; + }); - const { user } = plainRender(); + it("shows a button for listing them and opens a dialog when user clicks on it", async () => { + const { user } = plainRender(); const button = await screen.findByRole("button", { name: "2 errors found" }); - await user.click(button); - await waitFor(() => { - expect(screen.queryByText(/It is wrong/)).toBeInTheDocument(); - }); + // See IssuesDialog mock at the top of the file + const dialog = await screen.queryByText("IssuesDialog"); + expect(dialog).toBeNull(); + + await user.click(button); + await screen.findByText("IssuesDialog"); }); }); diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index fea582e23b..45558fe8dc 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -36,8 +36,7 @@ export { default as Installation } from "./Installation"; export { default as InstallationFinished } from "./InstallationFinished"; export { default as InstallationProgress } from "./InstallationProgress"; export { default as InstallButton } from "./InstallButton"; -export { default as IssuesLink } from "./IssuesLink"; -export { default as IssuesPage } from "./IssuesPage"; +export { default as IssuesDialog } from "./IssuesDialog"; export { default as SectionSkeleton } from "./SectionSkeleton"; export { default as ListSearch } from "./ListSearch"; export { default as LogsButton } from "./LogsButton"; diff --git a/web/src/components/overview/L10nSection.jsx b/web/src/components/overview/L10nSection.jsx index de6d1b5eb5..fb74076cac 100644 --- a/web/src/components/overview/L10nSection.jsx +++ b/web/src/components/overview/L10nSection.jsx @@ -53,6 +53,7 @@ export default function L10nSection() { loading={isLoading} icon="globe" path="/l10n" + id="l10n" >
    diff --git a/web/src/components/overview/ProductSection.jsx b/web/src/components/overview/ProductSection.jsx index 83121fab21..ad8481645d 100644 --- a/web/src/components/overview/ProductSection.jsx +++ b/web/src/components/overview/ProductSection.jsx @@ -73,6 +73,7 @@ export default function ProductSection() { errors={errors} loading={isLoading} path="/product" + id="product" >
    diff --git a/web/src/components/overview/SoftwareSection.jsx b/web/src/components/overview/SoftwareSection.jsx index 1408f791c4..a5ac99b8fc 100644 --- a/web/src/components/overview/SoftwareSection.jsx +++ b/web/src/components/overview/SoftwareSection.jsx @@ -146,6 +146,7 @@ export default function SoftwareSection({ showErrors }) { errors={errors.map(toValidationError)} // do not display the pattern selector when there are no patterns to display path={Object.keys(state.patterns).length > 0 ? "/software" : null} + id="software" > diff --git a/web/src/components/overview/StorageSection.jsx b/web/src/components/overview/StorageSection.jsx index adbe1978d8..1246183b4e 100644 --- a/web/src/components/overview/StorageSection.jsx +++ b/web/src/components/overview/StorageSection.jsx @@ -210,6 +210,7 @@ export default function StorageSection({ showErrors = false }) { icon="hard_drive" loading={busy} errors={errors} + id="storage" > diff --git a/web/src/components/overview/UsersSection.jsx b/web/src/components/overview/UsersSection.jsx index 433f557864..fc9d64b50a 100644 --- a/web/src/components/overview/UsersSection.jsx +++ b/web/src/components/overview/UsersSection.jsx @@ -122,6 +122,7 @@ export default function UsersSection({ showErrors }) { loading={state.busy} errors={errors} path="/users" + id="users" > { state.busy ? : } diff --git a/web/src/components/software/PatternSelector.jsx b/web/src/components/software/PatternSelector.jsx index 30b550a947..06de61c3b0 100644 --- a/web/src/components/software/PatternSelector.jsx +++ b/web/src/components/software/PatternSelector.jsx @@ -21,7 +21,6 @@ import React, { useCallback, useEffect, useState } from "react"; import { SearchInput } from "@patternfly/react-core"; -import { sprintf } from "sprintf-js"; import { useInstallerClient } from "~/context/installer"; import { Section, ValidationErrors } from "~/components/core"; @@ -202,10 +201,6 @@ function PatternSelector() { ); }); - // TRANSLATORS: error summary, always plural, %d is replaced by number of errors (2 or more) - // if there is just a single error then the error is displayed directly instead of this summary - const errorLabel = sprintf(_("%d errors"), errors.length); - // FIXME: ValidationErrors should be replaced by an equivalent component to show issues. // Note that only the Users client uses the old Validation D-Bus interface. const validationErrors = errors.map(toValidationError); @@ -214,7 +209,7 @@ function PatternSelector() { <>
    - + +
    } diff --git a/web/src/context/agama.jsx b/web/src/context/agama.jsx index 5ee0b7d22e..c17f6a8803 100644 --- a/web/src/context/agama.jsx +++ b/web/src/context/agama.jsx @@ -26,7 +26,6 @@ import { InstallerClientProvider } from "./installer"; import { InstallerL10nProvider } from "./installerL10n"; import { L10nProvider } from "./l10n"; import { ProductProvider } from "./product"; -import { NotificationProvider } from "./notification"; /** * Combines all application providers. @@ -40,9 +39,7 @@ function AgamaProviders({ children }) { - - {children} - + {children} diff --git a/web/src/context/notification.jsx b/web/src/context/notification.jsx deleted file mode 100644 index d04df31486..0000000000 --- a/web/src/context/notification.jsx +++ /dev/null @@ -1,80 +0,0 @@ -/* - * 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, { useCallback, useContext, useEffect, useState } from "react"; -import { useCancellablePromise } from "~/utils"; -import { useInstallerClient } from "./installer"; - -const NotificationContext = React.createContext([{ issues: false }]); - -function NotificationProvider({ children }) { - const client = useInstallerClient(); - const { cancellablePromise } = useCancellablePromise(); - const [state, setState] = useState({ issues: false }); - - const update = useCallback(({ issues }) => { - setState(s => ({ ...s, issues })); - }, [setState]); - - const load = useCallback(async () => { - if (!client) return; - - const issues = await cancellablePromise(client.issues()); - const hasIssues = Object.values(issues).flat().length > 0; - update({ issues: hasIssues }); - }, [client, cancellablePromise, update]); - - useEffect(() => { - if (!client) return; - - load(); - return client.onIssuesChange(load); - }, [client, load]); - - const value = [state, update]; - - return {children}; -} - -/** - * Returns the current notification state and a function to update the state - * @function - * - * @typedef {object} NotificationState - * @property {boolean} [issues] - Whether there is a notification for pending to read issues - * - * @callback NotificationStateUpdater - * @param {NotificationState} state - * @return {void} - * - * @returns {[NotificationState, NotificationStateUpdater]} - */ -function useNotification() { - const context = useContext(NotificationContext); - - if (!context) { - throw new Error("useNotification must be used within a NotificationProvider"); - } - - return context; -} - -export { NotificationProvider, useNotification }; diff --git a/web/src/index.js b/web/src/index.js index b09353cb6d..1762013289 100644 --- a/web/src/index.js +++ b/web/src/index.js @@ -41,7 +41,6 @@ import { ProposalPage as StoragePage, ISCSIPage, DASDPage, ZFCPPage } from "~/co import { UsersPage } from "~/components/users"; import { L10nPage } from "~/components/l10n"; import { NetworkPage } from "~/components/network"; -import { IssuesPage } from "~/components/core"; /** * As JSX components might import CSS stylesheets, our styles must be imported @@ -84,7 +83,6 @@ root.render( } /> } /> } /> - } /> } /> diff --git a/web/src/test-utils.js b/web/src/test-utils.js index 6175e3022f..ea41f8c730 100644 --- a/web/src/test-utils.js +++ b/web/src/test-utils.js @@ -32,7 +32,6 @@ import { render } from "@testing-library/react"; import { createClient } from "~/client/index"; import { InstallerClientProvider } from "~/context/installer"; -import { NotificationProvider } from "~/context/notification"; import { noop } from "./utils"; import cockpit from "./lib/cockpit"; import { InstallerL10nProvider } from "./context/installerL10n"; @@ -164,20 +163,6 @@ const createCallbackMock = () => { return [on, callbacks]; }; -/** - * Wraps the content with a notification provider - * - * @param {React.ReactNode} content - * @returns {React.ReactNode} - */ -const withNotificationProvider = (content) => { - return ( - - {content} - - ); -}; - /** * Mocks the cockpit.gettext() method with an identity function (returns * the original untranslated text) @@ -197,6 +182,5 @@ export { createCallbackMock, mockGettext, mockNavigateFn, - mockRoutes, - withNotificationProvider + mockRoutes };