Skip to content

Commit

Permalink
[web] Move information about transactional system to the top of the s…
Browse files Browse the repository at this point in the history
…torage page (#1075)

## Problem

Agama shows a message in the storage screen if the product being
installed uses a transactional Btrfs as root file-system (ie. read-only
root subvolume and Btrfs snapshots as a mechanism to perform updates).
But that message was displayed in the middle of the "Settings" section
taking the place usually occupied by the Btrfs snapshots switch. Having
it there didn't make much sense.

![Old
screen](https://github.com/openSUSE/agama/assets/3638289/7e4d472e-3d9e-4fab-b5b3-f3dd6b4369dc)

Moreover, the sentence was a bit complicated and it was not so clear how
the statement was related to other settings in the same page.

## Solution

The message is now displayed at the top of the page and the wording has
been slightly modified to, hopefully, explain why some of the usual
configuration options (like disabling Btrfs snapshots or changing the
file-system type for "/") are not available.


![micro-reminder](https://github.com/openSUSE/agama/assets/1691872/12a55af3-e5ec-4ff2-a3b1-fe6e9e22c5a5)


If the system is not transactional, everything looks as before, as
displayed in the following screenshots.


![macro2](https://github.com/openSUSE/agama/assets/3638289/f89ab1f3-72ea-4ebe-8581-03fb90ad2f0d)

## Testing

- Tested manually
- Updated unit tests

## Note

This is broken into many small commits to ease collaboration during
development. The plan is to squash when merging.
  • Loading branch information
ancorgs authored Mar 8, 2024
2 parents c43827a + 338b8e5 commit 49dfd5b
Show file tree
Hide file tree
Showing 12 changed files with 329 additions and 62 deletions.
21 changes: 21 additions & 0 deletions web/src/assets/styles/blocks.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
84 changes: 84 additions & 0 deletions web/src/components/core/Reminder.jsx
Original file line number Diff line number Diff line change
@@ -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 (
<div>
<Icon name={name} size="xs" />
</div>
);
};

/**
* 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 (
<h4>{children}</h4>
);
};

/**
* 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 (
<div role={role} data-type="agama/reminder">
<ReminderIcon name={icon} />
<div>
<ReminderTitle>{title}</ReminderTitle>
{ children }
</div>
</div>
);
}
65 changes: 65 additions & 0 deletions web/src/components/core/Reminder.test.jsx
Original file line number Diff line number Diff line change
@@ -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(<Reminder>Example</Reminder>);
const reminder = screen.getByRole("status");
within(reminder).getByText("Example");
});

it("renders a region with given role", () => {
plainRender(<Reminder role="alert">Example</Reminder>);
const reminder = screen.getByRole("alert");
within(reminder).getByText("Example");
});

it("renders given title", () => {
plainRender(
<Reminder title={<span><strong>Kindly</strong> reminder</span>}>
<a href="#">Visit the settings section</a>
</Reminder>
);
screen.getByRole("heading", { name: "Kindly reminder", level: 4 });
});

it("does not render a heading if title is not given", () => {
plainRender(<Reminder>Without title</Reminder>);
expect(screen.queryByRole("heading")).toBeNull();
});

it("does not render a heading if title is an empty string", () => {
plainRender(<Reminder title="">Without title</Reminder>);
expect(screen.queryByRole("heading")).toBeNull();
});

it("renders given children", () => {
plainRender(
<Reminder><a href="#">Visit the settings section</a></Reminder>
);
screen.getByRole("link", { name: "Visit the settings section" });
});
});
1 change: 1 addition & 0 deletions web/src/components/core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
6 changes: 5 additions & 1 deletion web/src/components/storage/ProposalPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ import {
ProposalSettingsSection,
ProposalSpacePolicySection,
ProposalDeviceSection,
ProposalFileSystemsSection
ProposalFileSystemsSection,
ProposalTransactionalInfo
} from "~/components/storage";
import { IDLE } from "~/client/status";

Expand Down Expand Up @@ -201,6 +202,9 @@ export default function ProposalPage() {
const PageContent = () => {
return (
<>
<ProposalTransactionalInfo
settings={state.settings}
/>
<ProposalDeviceSection
settings={state.settings}
availableDevices={state.availableDevices}
Expand Down
63 changes: 16 additions & 47 deletions web/src/components/storage/ProposalSettingsSection.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@

import React, { useEffect, useState } from "react";
import { Checkbox, Form, Skeleton, Switch, Tooltip } from "@patternfly/react-core";
import { sprintf } from "sprintf-js";

import { _ } from "~/i18n";
import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core";
import { Icon } from "~/components/layout";
import { noop } from "~/utils";
import { hasFS, isTransactionalSystem } from "~/components/storage/utils";
import { useProduct } from "~/context/product";
import { hasFS } from "~/components/storage/utils";

/**
* @typedef {import ("~/client/storage").ProposalManager.ProposalSettings} ProposalSettings
Expand Down Expand Up @@ -145,33 +143,23 @@ const SnapshotsField = ({
onChange({ active: checked, settings });
};

const configurableSnapshots = rootVolume.outline.snapshotsConfigurable;
const forcedSnapshots = !configurableSnapshots && hasFS(rootVolume, "Btrfs") && rootVolume.snapshots;
if (!rootVolume.outline.snapshotsConfigurable) return;

const SnapshotsToggle = () => {
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 (
<>
<Switch
id="snapshots"
label={_("Use Btrfs Snapshots")}
isReversed
isChecked={isChecked}
onChange={switchState}
/>
<div>
{explanation}
</div>
</>
);
};

return (
<div>
<If condition={forcedSnapshots} then={_("Btrfs snapshots required by product.")} />
<If condition={configurableSnapshots} then={<SnapshotsToggle />} />
<Switch
id="snapshots"
label={_("Use Btrfs Snapshots")}
isReversed
isChecked={isChecked}
onChange={switchState}
/>
<div>
{explanation}
</div>
</div>
);
};
Expand Down Expand Up @@ -297,8 +285,6 @@ export default function ProposalSettingsSection({
encryptionMethods = [],
onChange = noop
}) {
const { selectedProduct } = useProduct();

const changeEncryption = ({ password, method }) => {
onChange({ encryptionPassword: password, encryptionMethod: method });
};
Expand All @@ -318,29 +304,12 @@ export default function ProposalSettingsSection({

const encryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0;

const transactional = isTransactionalSystem(settings?.volumes || []);

return (
<>
<Section title={_("Settings")}>
<If
condition={transactional}
then={
<div>
<label>{_("Transactional system")}</label>
<div>
{/* 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)}
</div>
</div>
}
else={
<SnapshotsField
settings={settings}
onChange={changeBtrfsSnapshots}
/>
}
<SnapshotsField
settings={settings}
onChange={changeBtrfsSnapshots}
/>
<EncryptionField
password={settings.encryptionPassword || ""}
Expand Down
17 changes: 5 additions & 12 deletions web/src/components/storage/ProposalSettingsSection.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,6 @@ jest.mock("@patternfly/react-core", () => {
};
});

jest.mock("~/context/product", () => ({
...jest.requireActual("~/context/product"),
useProduct: () => ({
selectedProduct : { name: "Test" }
})
}));

let props;

beforeEach(() => {
Expand All @@ -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] };
});
Expand All @@ -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("does not render the snapshots switch", () => {
plainRender(<ProposalSettingsSection {...props} />);

screen.getByText("Transactional system");
expect(screen.queryByRole("checkbox", { name: "Use Btrfs Snapshots" })).toBeNull();
});
});

Expand Down
Loading

0 comments on commit 49dfd5b

Please sign in to comment.