Skip to content

Commit

Permalink
feature(web): unify Install and IssuesDrawerToggle buttons
Browse files Browse the repository at this point in the history
Because it provides a benefit to the user both visually and cognitively,
as they don't have to figure out why the install button was disabled,
nor be told where to find the reasons for it.
  • Loading branch information
dgdavid committed Nov 22, 2024
1 parent 6ecdceb commit a125c5c
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 233 deletions.
18 changes: 18 additions & 0 deletions web/src/assets/styles/app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,24 @@ button.remove-link:hover {
}
}

.agama-install-button {
padding: var(--pf-v5-global--spacer--sm) var(--pf-v5-global--spacer--md);
font-size: var(--fs-large);
}

.agama-issues-mark {
background: white;
width: 24px;
height: 24px;
border-radius: 24px;
top: -7px;
right: -7px;
position: absolute;
display: flex;
align-content: center;
justify-content: center;
}

.agama-issues-drawer-body {
padding: var(--pf-v5-global--spacer--lg);

Expand Down
38 changes: 28 additions & 10 deletions web/src/components/core/InstallButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,23 @@ describe("InstallButton", () => {
);
});

it("renders a disabled Install button", () => {
installerRender(<InstallButton />);
const button = screen.getByRole("button", { name: "Install" });
expect(button).toHaveAttribute("aria-disabled", "true");
it("renders additional information to warn users about found problems", () => {
const { container } = installerRender(<InstallButton />);
const button = screen.getByRole("button", { name: /Install/ });
// An exlamation icon as visual mark
const icon = container.querySelector("svg");
expect(icon).toHaveAttribute("data-icon-name", "exclamation");
// An aria-label for users using an screen reader
within(button).getByLabelText(/Not possible with current setup/);
});

it("triggers the onClickWithIssues callback without rendering the confirmation dialog", async () => {
const onClickWithIssuesFn = jest.fn();
const { user } = installerRender(<InstallButton onClickWithIssues={onClickWithIssuesFn} />);
const button = screen.getByRole("button", { name: /Install/ });
await user.click(button);
expect(onClickWithIssuesFn).toHaveBeenCalled();
await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument());
});
});

Expand All @@ -79,16 +92,21 @@ describe("InstallButton", () => {
mockIssuesList = new IssuesList([], [], [], []);
});

it("renders an enabled Install button", () => {
installerRender(<InstallButton />);
screen.getByRole("button", { name: "Install" });
it("renders the button without any additional information", () => {
const { container } = installerRender(<InstallButton />);
const button = screen.getByRole("button", { name: "Install" });
// Renders nothing else
const icon = container.querySelector("svg");
expect(icon).toBeNull();
expect(within(button).queryByLabelText(/Not possible with current setup/)).toBeNull();
});

it("renders a confirmation dialog when clicked", async () => {
const { user } = installerRender(<InstallButton />);
it("renders a confirmation dialog when clicked without triggering the onClickWithIssues callback", async () => {
const onClickWithIssuesFn = jest.fn();
const { user } = installerRender(<InstallButton onClickWithIssues={onClickWithIssuesFn} />);
const button = await screen.findByRole("button", { name: "Install" });
await user.click(button);

expect(onClickWithIssuesFn).not.toHaveBeenCalled();
screen.getByRole("dialog", { name: "Confirm Installation" });
});

Expand Down
49 changes: 26 additions & 23 deletions web/src/components/core/InstallButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
*/

import React, { useState } from "react";
import { Button, ButtonProps, Stack, Tooltip } from "@patternfly/react-core";
import { Button, ButtonProps, Stack } from "@patternfly/react-core";
import { Popup } from "~/components/core";
import { startInstallation } from "~/api/manager";
import { useAllIssues } from "~/queries/issues";
import { useLocation } from "react-router-dom";
import { PRODUCT, ROOT } from "~/routes/paths";
import { _ } from "~/i18n";
import { Icon } from "../layout";

/**
* List of paths where the InstallButton must not be shown.
Expand Down Expand Up @@ -70,18 +71,6 @@ according to the provided installation settings.",
);
};

/** Internal component for rendering the disabled Install button */
const DisabledButton = (props: ButtonProps) => (
<Tooltip
position="bottom-end"
content={_(
"Installation is not possible with current setup. Please, see the preflight checks by clicking in the warning at the header area",
)}
>
<Button {...props} />
</Tooltip>
);

/**
* Installation button
*
Expand All @@ -91,32 +80,46 @@ const DisabledButton = (props: ButtonProps) => (
* When clicked, it will ask for a confirmation before triggering the request
* for starting the installation.
*/
const InstallButton = (buttonProps: Omit<ButtonProps, "onClick">) => {
const InstallButton = (
props: Omit<ButtonProps, "onClick"> & { onClickWithIssues?: () => void },
) => {
const issues = useAllIssues();
const [isOpen, setIsOpen] = useState(false);
const location = useLocation();
const isEnabled = issues.isEmpty;
const hasIssues = !issues.isEmpty;

if (EXCLUDED_FROM.includes(location.pathname)) return;

const { onClickWithIssues, ...buttonProps } = props;
const open = async () => setIsOpen(true);
const close = () => setIsOpen(false);
const onAccept = () => {
close();
startInstallation();
};

const props: ButtonProps = {
...buttonProps,
variant: "primary",
onClick: open,
/* TRANSLATORS: Install button label */
children: _("Install"),
};
// TRANSLATORS: The install button label
const buttonText = _("Install");
// TRANSLATORS: Accessible text included with the install button when there are issues
const withIssuesAriaLabel = _("Not possible with current setup. Click to know more.");

return (
<>
{isEnabled ? <Button {...props} /> : <DisabledButton {...props} isAriaDisabled />}
<Button
variant="primary"
size="lg"
className="agama-install-button"
{...buttonProps}
onClick={hasIssues ? onClickWithIssues : open}
>
{buttonText}
{hasIssues && (
<div className="agama-issues-mark" aria-label={withIssuesAriaLabel}>
<Icon name="exclamation" size="xs" color="custom-color-300" />
</div>
)}
</Button>

{isOpen && <InstallConfirmationPopup onAccept={onAccept} onClose={close} />}
</>
);
Expand Down
115 changes: 0 additions & 115 deletions web/src/components/core/IssuesDrawerToggle.test.tsx

This file was deleted.

66 changes: 0 additions & 66 deletions web/src/components/core/IssuesDrawerToggle.tsx

This file was deleted.

1 change: 0 additions & 1 deletion web/src/components/core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,4 @@ export { default as Link } from "./Link";
export { default as EmptyState } from "./EmptyState";
export { default as InstallerOptions } from "./InstallerOptions";
export { default as IssuesDrawer } from "./IssuesDrawer";
export { default as IssuesDrawerToggle } from "./IssuesDrawerToggle";
export { default as Drawer } from "./Drawer";
Loading

0 comments on commit a125c5c

Please sign in to comment.