Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove global issues page and start using popup instead #886

Merged
merged 70 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
c520b56
removed IssuesLink from App.jsx
balsa-asanovic Nov 18, 2023
47d04f4
added class for highlighting
balsa-asanovic Nov 22, 2023
65b2ccb
added warning button to siderbar
balsa-asanovic Nov 22, 2023
8a68dcc
added className prop for styling
balsa-asanovic Nov 22, 2023
3754f47
removed title prop from ValidationErrors
balsa-asanovic Nov 22, 2023
c73c8cd
converted page to popup
balsa-asanovic Nov 23, 2023
951fe01
Removed popover, now it opens Issues popup
balsa-asanovic Nov 23, 2023
e5e85bc
typo correction in prop naming
balsa-asanovic Nov 23, 2023
dfdfd9d
changed name of comp IssuesPage to IssuesDialog
balsa-asanovic Dec 7, 2023
eff9b15
changed comp name IssuesPage to IssuesDialog
balsa-asanovic Dec 7, 2023
3195346
Merge branch 'extract-issues' of https://github.com/balsa-asanovic/ag…
balsa-asanovic Dec 7, 2023
3df181b
Reverting changes to PO files due to conflicts
balsa-asanovic Dec 7, 2023
4b92d6b
gemfile.lock reverts
balsa-asanovic Dec 7, 2023
3f011a0
changed color for highlighting
balsa-asanovic Dec 7, 2023
8223e4f
added new variable for warning icon color
balsa-asanovic Dec 7, 2023
d916474
switched to using a single ref hook
balsa-asanovic Dec 7, 2023
cd0a59e
Section component now uses forwardRef
balsa-asanovic Dec 7, 2023
ef57b49
changed the look of the issues button
balsa-asanovic Dec 7, 2023
7ae32ff
Merge remote-tracking branch 'agama/master' into extract-issues
dgdavid Dec 21, 2023
5cff43d
styling for issues dialog titles
balsa-asanovic Dec 25, 2023
12a17ef
added variable --fs-h3 for size 1rem
balsa-asanovic Dec 25, 2023
1d28178
changes prop name from sectionHighlight to openFrom
balsa-asanovic Dec 25, 2023
a68509f
sectionHighlight prop to openFrom
balsa-asanovic Dec 25, 2023
221cb19
added isOpen prop to issuesDialog
balsa-asanovic Dec 25, 2023
e27ff67
now using isOpen prop for rendering
balsa-asanovic Dec 25, 2023
fdd374d
removed IssuesLink from sidebar
balsa-asanovic Dec 26, 2023
50f5d42
removed IssuesLink from sidebar test
balsa-asanovic Dec 26, 2023
c575e28
Merge branch 'openSUSE:master' into extract-issues
balsa-asanovic Dec 26, 2023
15e0614
removed the global show issues button from Page comp
balsa-asanovic Dec 26, 2023
3dbd8e6
IssuesDialog now only shows issues from a single section
balsa-asanovic Dec 26, 2023
f9f85a8
removed warning icon from "N problems found" link
balsa-asanovic Dec 26, 2023
eb24b4c
Removed link to Issues page which no longer exists
balsa-asanovic Dec 26, 2023
6727d29
Merge branch 'openSUSE:master' into extract-issues
balsa-asanovic Dec 28, 2023
97f1343
classes no longer used
balsa-asanovic Dec 28, 2023
ae4971e
variable no longer used
balsa-asanovic Dec 28, 2023
4089b08
adjusted message text regarding the issues
balsa-asanovic Dec 28, 2023
9ff2b8f
cancel primary if there are issues, otherwise continue primary
balsa-asanovic Dec 28, 2023
2f62c18
removed the use of forward ref, added sectionId prop
balsa-asanovic Dec 30, 2023
3a6a126
adding sectionId prop value
balsa-asanovic Dec 30, 2023
bc464b2
adding sectionId prop value
balsa-asanovic Dec 30, 2023
f272ef3
adding sectionId prop value
balsa-asanovic Dec 30, 2023
bc693bd
adding sectionId prop value
balsa-asanovic Dec 30, 2023
bf745c8
adding sectionId prop value
balsa-asanovic Dec 30, 2023
afc7dae
adding sectionId prop value
balsa-asanovic Dec 30, 2023
dcfc69a
changed prop name to sectionId
balsa-asanovic Dec 30, 2023
bd4c91a
simplified issuesSection comp; no conditions, just a single section
balsa-asanovic Dec 30, 2023
6705f81
removed red color and warning icon from issueItem
balsa-asanovic Jan 3, 2024
9a6aa29
removed code for IssuesLink
balsa-asanovic Jan 3, 2024
16e6b9c
removed IssuesLink
balsa-asanovic Jan 3, 2024
d0ffa4f
changed prop name from sectionId to id
balsa-asanovic Jan 3, 2024
dc80336
prop name change from sectionId to id
balsa-asanovic Jan 3, 2024
27a0127
more simplification of IssuesDialog code
balsa-asanovic Jan 3, 2024
bd72c6e
passing title to IssuesDialog based on sectionId
balsa-asanovic Jan 3, 2024
651211f
removing the use of notificationProvider
balsa-asanovic Jan 3, 2024
f5dad31
removing the use of notificationProvider
balsa-asanovic Jan 3, 2024
5ea7aab
droping the whole notification code
balsa-asanovic Jan 3, 2024
0df8a30
removed old tests, added two basic tests
balsa-asanovic Jan 3, 2024
bddb6b8
[web] Please linters
dgdavid Jan 4, 2024
4e97140
[web] Fix InstallButton indentation and test
dgdavid Jan 4, 2024
8a45d9b
[web] Update ValidationErrors component test
dgdavid Jan 4, 2024
d4a99e1
[web] Do not use core/Section for list of issues
dgdavid Jan 4, 2024
e1b7f79
[web] Allow ValidationErrors guess the right issues key
dgdavid Jan 4, 2024
1260990
[web] Add an id to the storage actions section
dgdavid Jan 4, 2024
b67cf7d
[web] Fix loading icon size in IssuesDialog
dgdavid Jan 4, 2024
5ab2ae4
[web] Fix sectionId prop in software/PatternSelector
dgdavid Jan 4, 2024
d328700
Merge branch 'openSUSE:master' into extract-issues
balsa-asanovic Jan 4, 2024
0e84d9b
removed route for Issues Dialog
balsa-asanovic Jan 4, 2024
75d87a1
changes file update
balsa-asanovic Jan 4, 2024
a1a04a5
[web] Fix styles
dgdavid Jan 4, 2024
e443124
[web] Please ESLint
dgdavid Jan 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions web/package/cockpit-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Thu Jan 04 21:44:32 UTC 2024 - Balsa Asanovic <balsaasanovic95@gmail.com>

- 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 <dgonzalez@suse.com>

Expand Down
13 changes: 13 additions & 0 deletions web/src/assets/styles/blocks.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
9 changes: 9 additions & 0 deletions web/src/assets/styles/patternfly-overrides.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions web/src/assets/styles/variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
22 changes: 11 additions & 11 deletions web/src/components/core/InstallButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,30 +23,27 @@ 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";

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 (
<p>
<strong>
{msgStart}
<Link to="/issues">{msgLink}</Link>
{msgEnd}
{ _("There are some reported issues. \
Please review them in the previous steps before proceeding with the installation.") }
</strong>
</p>
);
};

const Cancel = hasIssues ? Popup.PrimaryAction : Popup.SecondaryAction;
const Continue = hasIssues ? Popup.SecondaryAction : Popup.PrimaryAction;

return (
<Popup
title={_("Confirm Installation")}
Expand All @@ -63,11 +60,14 @@ according to the provided installation settings.") }
</p>
</div>
<Popup.Actions>
<Popup.Confirm onClick={onAccept}>
<Cancel key="cancel" onClick={onClose} autoFocus={hasIssues}>
{/* TRANSLATORS: button label */}
{_("Cancel")}
</Cancel>
<Continue key="confirm" onClick={onAccept} autoFocus={!hasIssues}>
{/* TRANSLATORS: button label */}
{_("Continue")}
</Popup.Confirm>
<Popup.Cancel onClick={onClose} autoFocus />
</Continue>
</Popup.Actions>
</Popup>
);
Expand Down
11 changes: 5 additions & 6 deletions web/src/components/core/InstallButton.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<InstallButton />);
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(<InstallButton />);
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();
});
});
});
Expand Down
123 changes: 123 additions & 0 deletions web/src/components/core/IssuesDialog.jsx
Original file line number Diff line number Diff line change
@@ -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 (
<li>
{issue.description}
<If
condition={hasDetails}
then={<pre>{issue.details}</pre>}
/>
</li>
);
};

/**
* 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 <IssueItem key={`issue-${index}`} issue={issue} />;
});

return <ul>{items}</ul>;
};

/**
* 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 (
<Popup
isOpen={isOpen}
title={title}
data-content="issues-summary"
>
<If
condition={isLoading}
then={<Icon name="loading" className="icon-xxxl" />}
else={<IssueItems issues={issues} />}
/>
<Popup.Actions>
<Popup.Confirm onClick={onClose} autoFocus>{_("Close")}</Popup.Confirm>
</Popup.Actions>
</Popup>
);
}
73 changes: 73 additions & 0 deletions web/src/components/core/IssuesDialog.test.jsx
Original file line number Diff line number Diff line change
@@ -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: () => <div>PFSkeleton</div>
};
});
jest.mock("~/components/core/Sidebar", () => () => <div>Agama sidebar</div>);

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(<IssuesDialog isOpen sectionId="storage" title="Storage issues" />);

await screen.findByText(/storage issue 1/);
});

it('calls onClose callback when close button is clicked', async () => {
const mockOnClose = jest.fn();
const { user } = installerRender(<IssuesDialog isOpen onClose={mockOnClose} sectionId="software" title="Software issues" />);

await user.click(screen.getByText("Close"));
expect(mockOnClose).toHaveBeenCalled();
});
47 changes: 0 additions & 47 deletions web/src/components/core/IssuesLink.jsx

This file was deleted.

Loading