Skip to content

Commit

Permalink
[web] Storage page: break settings into multiple sections (#1045)
Browse files Browse the repository at this point in the history
Break the storage settings section to accomplish the organization
defined in the [storage
UI](https://github.com/openSUSE/agama/blob/master/doc/storage_ui.md)
document. Thus, proposed changes make the page to have _Device_,
_Settings_, and _File systems_ sections instead of only _Settings_.

Also, it includes a last minute change about moving the LVM setting to
the new _Device_ section, because it is more

> consistent with the future we want to go (in which LVM is part of the
device selection)
> 
>  — @ancorgs at #yast IRC channel

---

Related to https://trello.com/c/SK4uBRnw (internal link)
  • Loading branch information
dgdavid authored Feb 22, 2024
2 parents e302716 + 75bd8aa commit 7a6e88a
Show file tree
Hide file tree
Showing 19 changed files with 1,068 additions and 742 deletions.
6 changes: 6 additions & 0 deletions web/package/cockpit-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Thu Feb 22 14:05:56 UTC 2024 - David Diaz <[email protected]>

- Break storage settings in multiple sections to improve the UX
(gh#openSUSE/agama#1045).

-------------------------------------------------------------------
Wed Feb 21 17:40:01 UTC 2024 - David Diaz <[email protected]>

Expand Down
28 changes: 20 additions & 8 deletions web/src/assets/styles/blocks.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,36 @@
margin-block-end: var(--spacer-medium);
}

> h2 {
> header {
display: grid;
grid-area: header;
grid-template-columns: subgrid;
grid-column: bleed / content-end;

svg {
block-size: var(--section-icon-size);
inline-size: var(--section-icon-size);
grid-column: bleed / content;
h2 {
display: grid;
grid-template-columns: subgrid;
grid-column: bleed / content-end;

svg {
block-size: var(--section-icon-size);
inline-size: var(--section-icon-size);
grid-column: bleed / content;
}

:not(svg) {
grid-column: content
}
}

:not(svg) {
grid-column: content
p {
grid-column: content;
color: var(--color-gray-dimmest);
margin-block-end: var(--spacer-smaller);
}
}

> :not(h2) {
> :not(header) {
grid-area: content;
grid-column: content;
}
Expand Down
2 changes: 1 addition & 1 deletion web/src/assets/styles/layout.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
padding: var(--spacer-small);
}

header {
> header {
@extend .bottom-shadow;
grid-area: header;
display: flex;
Expand Down
1 change: 0 additions & 1 deletion web/src/assets/styles/patternfly-overrides.scss
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ table td > .pf-v5-c-empty-state {
--stack-gutter: 0;
--pf-v5-c-toolbar--PaddingTop: 0;
--pf-v5-c-toolbar--PaddingBottom: 0;
border-block-end: 1px solid var(--color-gray-light);
}

.pf-v5-c-toolbar__content {
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 @@ -52,6 +52,7 @@
--color-gray-dark: #efefef; // Fog
--color-gray-darker: #999;
--color-gray-dimmed: #888;
--color-gray-dimmest: #666;

--color-link: #0c322c;
--color-link-hover: #30ba78;
Expand Down
10 changes: 8 additions & 2 deletions web/src/components/core/Section.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import React from "react";
import { Link } from "react-router-dom";
import { Icon } from '~/components/layout';
import { ValidationErrors } from "~/components/core";
import { If, ValidationErrors } from "~/components/core";

/**
* Renders children into an HTML section
Expand All @@ -48,6 +48,7 @@ import { ValidationErrors } from "~/components/core";
* @typedef { Object } SectionProps
* @property {string} [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.
* @property {string} [path] - Path where the section links to.
* when user clicks on the title, used for opening a dialog.
Expand All @@ -63,6 +64,7 @@ import { ValidationErrors } from "~/components/core";
export default function Section({
icon,
title,
description,
name,
path,
loading,
Expand All @@ -84,9 +86,13 @@ export default function Section({
const iconName = loading ? "loading" : icon;
const headerIcon = iconName ? <Icon name={iconName} /> : null;
const headerText = !path?.trim() ? title : <Link to={path}>{title}</Link>;
const renderDescription = React.isValidElement(description) || description?.length > 0;

return (
<h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
<header>
<h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
<If condition={renderDescription} then={<p>{description}</p>} />
</header>
);
};

Expand Down
26 changes: 23 additions & 3 deletions web/src/components/core/Section.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ describe("Section", () => {
describe("when title is given", () => {
it("renders the section header", () => {
plainRender(<Section title="Settings" />);
screen.getByRole("heading", { name: "Settings" });
screen.getByRole("banner");
});

it("renders given title as section heading", () => {
plainRender(<Section title="Settings" />);
const header = screen.getByRole("banner");
within(header).getByRole("heading", { name: "Settings" });
});

it("renders an icon if valid icon name is given", () => {
Expand All @@ -58,15 +64,29 @@ describe("Section", () => {
const icon = container.querySelector("svg");
expect(icon).toBeNull();
});

it("renders given description as part of the header", () => {
plainRender(
<Section title="Settings" description="Short explanation to help the user, if needed" />
);
const header = screen.getByRole("banner");
within(header).getByText(/Short explanation/);
});
});

describe("when title is not given", () => {
it("does not render the section header", async () => {
plainRender(<Section />);
const header = await screen.queryByRole("heading");
plainRender(<Section description="Does not matter" />);
const header = await screen.queryByRole("banner");
expect(header).not.toBeInTheDocument();
});

it("does not render a section heading", async () => {
plainRender(<Section description="Does not matter" />);
const heading = await screen.queryByRole("heading");
expect(heading).not.toBeInTheDocument();
});

it("does not render the section icon", () => {
const { container } = plainRender(<Section icon="settings" />);
const icon = container.querySelector("svg");
Expand Down
17 changes: 10 additions & 7 deletions web/src/components/storage/ProposalActionsSection.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
ListItem,
ExpandableSection,
Skeleton,
Text
} from "@patternfly/react-core";
import { sprintf } from "sprintf-js";

Expand Down Expand Up @@ -74,9 +73,6 @@ const ProposalActions = ({ actions = [] }) => {

return (
<>
<Text>
{_("Actions to create the file systems and to ensure the system boots.")}
</Text>
<ActionsList actions={generalActions} />
{subvolActions.length > 0 && (
<ExpandableSection
Expand Down Expand Up @@ -121,9 +117,16 @@ export default function ProposalActionsSection({ actions = [], errors = [], isLo
if (isLoading) errors = [];

return (
// TRANSLATORS: section title, list of planned actions for the selected device,
// e.g. "delete partition A", "create partition B with filesystem C", ...
<Section title={_("Planned Actions")} id="storage-actions" errors={errors}>
<Section
// TRANSLATORS: The storage "Planned Actions" section's title. The
// section shows a list of planned actions for the selected device, e.g.
// "delete partition A", "create partition B with filesystem C", ...
title={_("Planned Actions")}
// TRANSLATORS: The storage "Planned Actions" section's description
description={_("Actions to create the file systems and to ensure the new system boots.")}
id="storage-actions"
errors={errors}
>
<If
condition={isLoading}
then={<ActionsSkeleton />}
Expand Down
3 changes: 2 additions & 1 deletion web/src/components/storage/ProposalActionsSection.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ it("renders skeleton while loading", () => {
it("renders nothing when there is no actions", () => {
plainRender(<ProposalActionsSection actions={[]} />);

expect(screen.queryByText(/Actions to create/)).toBeNull();
expect(screen.queryAllByText(/Delete/)).toEqual([]);
expect(screen.queryAllByText(/Create/)).toEqual([]);
expect(screen.queryAllByText(/Show/)).toEqual([]);
});

describe("when there are actions", () => {
Expand Down
Loading

0 comments on commit 7a6e88a

Please sign in to comment.