Skip to content

Commit

Permalink
web: Revamp storage proposal page (#1138)
Browse files Browse the repository at this point in the history
The Storage Page has become a bit difficult to navigate and a bit
overwhelming at first glance, making it more likely to scare users than
help them.

The changes in this PR address several weak points we identified:
dynamic button/link text, inconsistent widget placement/alignment, and
too much information exposed in the first render. As such, the UI
proposed is more straightforward, but it is still relevant and helpful
for everyone, including advanced and basic users. It keeps contextual
help visible and close to the widget/action it belongs to, without
diverting user attention from the important bits.

At the time of introducing these changes, the rest of Agama was also
taken into consideration so that we could easily maintain consistency by
just using the new widgets when needed.

For a better understanding of the changes made, please check commit by
commit.

---

Related to
https://trello.com/c/czpTfm3y/3628-5-agama-polish-storage-section
(internal link)
  • Loading branch information
ancorgs authored Apr 15, 2024
2 parents 545a8a3 + bcc4e8a commit f5946ff
Show file tree
Hide file tree
Showing 36 changed files with 2,389 additions and 1,317 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 @@
-------------------------------------------------------------------
Mon Apr 15 07:14:35 UTC 2024 - David Diaz <[email protected]>

- Enhance the storage page to make it easier to use and understand.
(gh#openSUSE/agama#1138).

-------------------------------------------------------------------
Thu Apr 11 15:16:42 UTC 2024 - José Iván López González <[email protected]>

Expand Down
76 changes: 75 additions & 1 deletion web/src/assets/styles/blocks.scss
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,6 @@ section [data-type="agama/reminder"] {
}
}


[data-type="agama/expandable-selector"] {
// The expandable selector is built on top of PF/Table#expandable
// Let's tweak some styles
Expand All @@ -743,6 +742,81 @@ section [data-type="agama/reminder"] {
}
}

[data-type="agama/field"] {
> div:first-child {
font-size: var(--fs-large);

button {
padding-inline: 0;
}

button:hover {
color: var(--color-link-hover);
fill: var(--color-link-hover);
}

button b, button:hover b {
text-decoration: underline;
text-underline-offset: var(--spacer-smaller);
}

div.pf-v5-c-skeleton {
display: inline-block;
vertical-align: middle;
height: 1.5ex;
}
}

> div:nth-child(n+2) {
margin-inline-start: calc(var(--icon-size-s) + 1ch);
}

> div:nth-child(2) {
color: gray;
font-size: var(--fs-medium);
}

> div:nth-child(n+3) {
margin-block-start: var(--spacer-small);
}

&.highlighted > div:last-child {
--spacing: calc(var(--icon-size-s) / 2);
margin-inline: var(--spacing);
padding-inline: var(--spacing);
border-inline-start: 2px solid;
}

&.highlighted.on > div:last-child {
border-color: var(--color-link-hover);
}

&.highlighted.off > div:last-child {
border-color: var(--color-gray-darker);
}

&.on {
button:not(.password-toggler) {
fill: var(--color-link-hover);
}
}

hr {
margin-block: var(--spacer-normal);
border: 0;
border-bottom: thin dashed var(--color-gray);
}
}

[data-type="agama/field"] button.pf-v5-c-menu-toggle.pf-m-plain {
padding: 0;
}

[data-type="agama/field"] .pf-v5-c-menu__list {
padding: calc(var(--spacer-smaller) / 2) 0;
margin: 0;
}

#boot-form {
legend {
label {
Expand Down
9 changes: 9 additions & 0 deletions web/src/assets/styles/utilities.scss
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,15 @@
color: inherit;
font: inherit;
padding: 0;
text-align: start;
}

.inline-flex-button{
@extend .plain-button;
display: inline-flex;
align-items: center;
gap: 0.7ch;
text-decoration: underline;
}

.p-0 {
Expand Down
121 changes: 121 additions & 0 deletions web/src/components/core/Field.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* 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";

/**
* @typedef {import("react").ButtonHTMLAttributes} ButtonHTMLAttributes
* @typedef {import("~/components/layout/Icon").IconName} IconName
* @typedef {import("~/components/layout/Icon").IconSize} IconSize
*/

/**
* @typedef {object} FieldProps
* @property {React.ReactNode} label - The field label.
* @property {React.ReactNode} [value] - The field value.
* @property {React.ReactNode} [description] - A field description, useful for providing context to the user.
* @property {IconName} [icon] - The name of the icon for the field.
* @property {IconSize} [iconSize="s"] - The size for the field icon.
* @property {("b"|"span")} [textWrapper="b"] - The element used for wrapping the label.
* @property {ButtonHTMLAttributes} [buttonAttrs={}] - The element used for wrapping the label.
* @property {string} [className] - ClassName
* @property {() => void} [onClick] - Callback
* @property {React.ReactNode} [children] - A content to be rendered as field children
*
* @typedef {Omit<FieldProps, 'icon'>} FieldPropsWithoutIcon
*/

/**
* Component for laying out a page field
*
* @param {FieldProps} props
*/
const Field = ({
label,
value,
description,
icon,
iconSize = "s",
onClick,
children,
textWrapper = "b",
buttonAttrs = {},
...props
}) => {
const FieldIcon = () => icon?.length > 0 && <Icon name={icon} size={iconSize} />;
const TextWrapper = textWrapper;
return (
<div {...props} data-type="agama/field">
<div>
<button {...buttonAttrs} className="plain-button" onClick={onClick}>
<FieldIcon /> <TextWrapper>{label}</TextWrapper>
</button> {value}
</div>
<div>
{description}
</div>
<div>
{children}
</div>
</div>
);
};

/**
* @param {Omit<FieldProps, 'icon'>} props
*/
const SettingsField = ({ ...props }) => {
return <Field {...props} icon="shadow" />;
};

/**
* @param {Omit<FieldProps, 'icon'> & {isChecked: boolean, highlightContent?: boolean}} props
*/
const SwitchField = ({ isChecked = false, highlightContent = false, ...props }) => {
const iconName = isChecked ? "toggle_on" : "toggle_off";
const baseClassnames = highlightContent ? "highlighted" : "";
const stateClassnames = isChecked ? "on" : "off";

return (
<Field
{...props}
icon={iconName}
className={[baseClassnames, stateClassnames].join(" ")}
buttonAttrs={{ role: "switch", "aria-checked": isChecked }}
/>
);
};

/**
* @param {Omit<FieldProps, 'icon'> & {isExpanded: boolean}} props
*/
const ExpandableField = ({ isExpanded, ...props }) => {
const iconName = isExpanded ? "collapse_all" : "expand_all";
const className = isExpanded ? "expanded" : "collapsed";

return <Field {...props} icon={iconName} className={className} />;
};

export default Field;
export { ExpandableField, SettingsField, SwitchField };
129 changes: 129 additions & 0 deletions web/src/components/core/Field.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* 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 } from "@testing-library/react";
import { plainRender } from "~/test-utils";
import { Field, ExpandableField, SettingsField, SwitchField } from "~/components/core";

const onClick = jest.fn();

describe("Field", () => {
it("renders a button with given icon and label", () => {
const { container } = plainRender(
<Field icon="edit" label="Theme" value="dark" onClick={onClick} />
);
screen.getByRole("button", { name: "Theme" });
const icon = container.querySelector("button > svg");
expect(icon).toHaveAttribute("data-icon-name", "edit");
});

it("renders value, description, and given children", () => {
plainRender(
<Field
icon="edit"
label="Theme"
value="dark"
description="Choose your preferred color schema."
onClick={onClick}
>
<p>This is a <b>preview</b></p>;
</Field>
);
screen.getByText("dark");
screen.getByText("Choose your preferred color schema.");
screen.getByText("This is a");
screen.getByText("preview");
});

it("triggers the onClick callback when users clicks the button", async () => {
const { user } = plainRender(
<Field label="Theme" value="dark" onClick={onClick} />
);
const button = screen.getByRole("button");
await user.click(button);
expect(onClick).toHaveBeenCalled();
});
});

describe("SettingsField", () => {
it("uses the 'shadow' icon", () => {
const { container } = plainRender(
// Trying to set other icon, although typechecking should catch it.
<SettingsField icon="edit" label="Theme" value="dark" onClick={onClick} />
);
const icon = container.querySelector("button > svg");
expect(icon).toHaveAttribute("data-icon-name", "shadow");
});
});

describe("SwitchField", () => {
it("sets button role to switch", () => {
plainRender(<SwitchField label="Zoom" value="enabled" isChecked />);
const switchButton = screen.getByRole("switch", { name: "Zoom" });
expect(switchButton instanceof HTMLButtonElement).toBe(true);
});

it("keeps aria-checked attribute in sync with isChecked prop", () => {
let switchButton;
const { rerender } = plainRender(<SwitchField label="Zoom" value="enabled" isChecked />);
switchButton = screen.getByRole("switch", { name: "Zoom" });
expect(switchButton).toHaveAttribute("aria-checked", "true");

rerender(<SwitchField label="Zoom" value="disabled" />);
switchButton = screen.getByRole("switch", { name: "Zoom" });
expect(switchButton).toHaveAttribute("aria-checked", "false");
});

it("uses the 'toggle_on' icon when isChecked", () => {
const { container } = plainRender(
<SwitchField label="Zoom" value="enabled" isChecked />
);
const icon = container.querySelector("button > svg");
expect(icon).toHaveAttribute("data-icon-name", "toggle_on");
});

it("uses the 'toggle_off' icon when not isChecked", () => {
const { container } = plainRender(
<SwitchField label="Zoom" value="disabled" />
);
const icon = container.querySelector("button > svg");
expect(icon).toHaveAttribute("data-icon-name", "toggle_off");
});
});

describe("ExpandableField", () => {
it("uses the 'collapse_all' icon when isExpanded", () => {
const { container } = plainRender(
<ExpandableField label="More settings" isExpanded />
);
const icon = container.querySelector("button > svg");
expect(icon).toHaveAttribute("data-icon-name", "collapse_all");
});

it("uses the 'expand_all' icon when not isExpanded", () => {
const { container } = plainRender(
<ExpandableField label="More settings" />
);
const icon = container.querySelector("button > svg");
expect(icon).toHaveAttribute("data-icon-name", "expand_all");
});
});
Loading

0 comments on commit f5946ff

Please sign in to comment.