feat(web): move installation issues to a drawer and keep Install button always visible

web/package/agama-web-ui.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Mon Nov 25 11:12:36 UTC 2024 - David Diaz <[email protected]>
+
+- Unify Install and "warning" header buttons.
+- Move installation issues to a drawer shown when Install button
+  is clicked (gh#agama-project#agama#1778).
+
 -------------------------------------------------------------------
 Fri Nov 15 16:48:44 UTC 2024 - Ladislav Slezák <[email protected]>
 

web/src/assets/styles/app.scss

@@ -62,7 +62,38 @@ button.remove-link:hover {
       right: 0;
       bottom: 0;
       left: 0;
-      z-index: 999;
     }
   }
 }
+
+.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);
+
+  h4 a {
+    text-decoration: underline;
+    font-weight: var(--fw-bold);
+  }
+
+  ul li.pf-m-info,
+  ul li.pf-m-warning {
+    --pf-v5-c-notification-drawer__list-item--before--BackgroundColor: none;
+  }
+}

web/src/assets/styles/patternfly-overrides.scss

@@ -347,3 +347,12 @@
     inline-size: 250px;
   }
 }
+
+// A temporary workaround to fix "stacking contexts" problems with scroll and
+// sticky page sections in Agama layout. It will not be needed when migrating to
+// latest PF6 release, since the root of the problem has been addressed there by
+// removing the DrawerContentBody from the Page component. See
+// https://github.com/patternfly/patternfly/pull/7130 and related links
+.pf-v5-c-drawer__body {
+  display: contents;
+}

web/src/components/core/InstallButton.test.tsx

@@ -67,9 +67,23 @@ describe("InstallButton", () => {
       );
     });
 
-    it("renders nothing", () => {
+    it("renders additional information to warn users about found problems", () => {
       const { container } = installerRender(<InstallButton />);
-      expect(container).toBeEmptyDOMElement();
+      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 the 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());
     });
   });
 
@@ -78,16 +92,21 @@ describe("InstallButton", () => {
       mockIssuesList = new IssuesList([], [], [], []);
     });
 
-    it("renders an 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 the 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" });
     });
 

web/src/components/core/InstallButton.tsx

@@ -21,15 +21,14 @@
  */
 
 import React, { useState } from "react";
-
 import { Button, ButtonProps, Stack } from "@patternfly/react-core";
-
 import { Popup } from "~/components/core";
-import { _ } from "~/i18n";
 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.
@@ -81,26 +80,44 @@ according to the provided installation settings.",
  * When clicked, it will ask for a confirmation before triggering the request
  * for starting the installation.
  */
-const InstallButton = (props: Omit<ButtonProps, "onClick">) => {
+const InstallButton = (
+  props: Omit<ButtonProps, "onClick"> & { onClickWithIssues?: () => void },
+) => {
   const issues = useAllIssues();
   const [isOpen, setIsOpen] = useState(false);
   const location = useLocation();
+  const hasIssues = !issues.isEmpty;
 
-  if (!issues.isEmpty) return;
   if (EXCLUDED_FROM.includes(location.pathname)) return;
 
+  const { onClickWithIssues, ...buttonProps } = props;
   const open = async () => setIsOpen(true);
   const close = () => setIsOpen(false);
   const onAccept = () => {
     close();
     startInstallation();
   };
 
+  // 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 the current setup. Click to know more.");
+
   return (
     <>
-      <Button variant="primary" {...props} onClick={open}>
-        {/* TRANSLATORS: button label */}
-        {_("Install")}
+      <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} />}

web/src/components/core/IssuesDrawer.test.tsx

@@ -0,0 +1,130 @@
+/*
+ * 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 the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * 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 { installerRender } from "~/test-utils";
+import { InstallationPhase } from "~/types/status";
+import { IssuesList } from "~/types/issues";
+import IssuesDrawer from "./IssuesDrawer";
+
+let phase = InstallationPhase.Config;
+let mockIssuesList: IssuesList;
+const onCloseFn = jest.fn();
+
+jest.mock("~/queries/issues", () => ({
+  ...jest.requireActual("~/queries/issues"),
+  useAllIssues: () => mockIssuesList,
+}));
+
+jest.mock("~/queries/status", () => ({
+  useInstallerStatus: () => ({
+    phase,
+  }),
+}));
+
+const itRendersNothing = () =>
+  it("renders nothing", () => {
+    const { container } = installerRender(<IssuesDrawer onClose={onCloseFn} />);
+    expect(container).toBeEmptyDOMElement();
+  });
+
+describe("IssuesDrawer", () => {
+  describe("when there are no installation issues", () => {
+    beforeEach(() => {
+      mockIssuesList = new IssuesList([], [], [], []);
+    });
+
+    itRendersNothing();
+  });
+
+  describe("when there are installation issues", () => {
+    beforeEach(() => {
+      mockIssuesList = new IssuesList(
+        [],
+        [
+          {
+            description: "Software Fake Issue",
+            source: 0,
+            severity: 0,
+            details: "Software Fake Issue details",
+          },
+        ],
+        [
+          {
+            description: "Storage Fake Issue 1",
+            source: 0,
+            severity: 0,
+            details: "Storage Fake Issue 1 details",
+          },
+          {
+            description: "Storage Fake Issue 2",
+            source: 0,
+            severity: 0,
+            details: "Storage Fake Issue 2 details",
+          },
+        ],
+        [
+          {
+            description: "Users Fake Issue",
+            source: 0,
+            severity: 0,
+            details: "Users Fake Issue details",
+          },
+        ],
+      );
+    });
+
+    it("renders the drawer with categorized issues linking to their scope", async () => {
+      const { user } = installerRender(<IssuesDrawer onClose={onCloseFn} />);
+
+      const softwareIssues = screen.getByRole("region", { name: "Software" });
+      const storageIssues = screen.getByRole("region", { name: "Storage" });
+      const usersIssues = screen.getByRole("region", { name: "Users" });
+
+      const softwareLink = within(softwareIssues).getByRole("link", { name: "Software" });
+      expect(softwareLink).toHaveAttribute("href", "/software");
+      within(softwareIssues).getByText("Software Fake Issue");
+
+      const storageLink = within(storageIssues).getByRole("link", { name: "Storage" });
+      expect(storageLink).toHaveAttribute("href", "/storage");
+      within(storageIssues).getByText("Storage Fake Issue 1");
+      within(storageIssues).getByText("Storage Fake Issue 2");
+
+      const usersLink = within(usersIssues).getByRole("link", { name: "Users" });
+      expect(usersLink).toHaveAttribute("href", "/users");
+      within(usersIssues).getByText("Users Fake Issue");
+
+      const closeButton = screen.getByRole("button", { name: "Close" });
+      await user.click(closeButton);
+      expect(onCloseFn).toHaveBeenCalled();
+    });
+
+    describe("at install phase", () => {
+      beforeEach(() => {
+        phase = InstallationPhase.Install;
+      });
+
+      itRendersNothing();
+    });
+  });
+});