From fe183008d88bbe160934e495c2aba2df8ed5a78a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 30 Oct 2024 21:03:03 +0000 Subject: [PATCH] fix(web): render InstallButton only when needed Since it does not make sense to render the InstallButton when the installation is in progress or finished. The same apply for the login screen, product selection, and product progress. --- .../components/core/InstallButton.test.tsx | 22 +++++++--------- web/src/components/core/InstallButton.tsx | 25 ++++++++++++++++--- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/web/src/components/core/InstallButton.test.tsx b/web/src/components/core/InstallButton.test.tsx index 1adb4570c8..bbc147543b 100644 --- a/web/src/components/core/InstallButton.test.tsx +++ b/web/src/components/core/InstallButton.test.tsx @@ -25,6 +25,7 @@ import { screen, waitFor } from "@testing-library/react"; import { installerRender, mockRoutes } from "~/test-utils"; import { InstallButton } from "~/components/core"; import { IssuesList } from "~/types/issues"; +import { PATHS as ROOT_PATHS } from "~/router"; import { PATHS as PRODUCT_PATHS } from "~/routes/products"; const mockStartInstallationFn = jest.fn(); @@ -97,20 +98,15 @@ describe("when there are not installation issues", () => { }); }); - describe("but installer is rendering the product selection", () => { + describe.each([ + ["login", ROOT_PATHS.login], + ["product selection", PRODUCT_PATHS.changeProduct], + ["product selection progress", PRODUCT_PATHS.progress], + ["installation progress", ROOT_PATHS.installationProgress], + ["installation finished", ROOT_PATHS.installationFinished], + ])(`but the installer is rendering the %s screen`, (_, path) => { beforeEach(() => { - mockRoutes(PRODUCT_PATHS.changeProduct); - }); - - it("renders nothing", () => { - const { container } = installerRender(); - expect(container).toBeEmptyDOMElement(); - }); - }); - - describe("but installer is configuring a product", () => { - beforeEach(() => { - mockRoutes(PRODUCT_PATHS.progress); + mockRoutes(path); }); it("renders nothing", () => { diff --git a/web/src/components/core/InstallButton.tsx b/web/src/components/core/InstallButton.tsx index 0d7c56512f..f24825cfc9 100644 --- a/web/src/components/core/InstallButton.tsx +++ b/web/src/components/core/InstallButton.tsx @@ -29,8 +29,24 @@ import { _ } from "~/i18n"; import { startInstallation } from "~/api/manager"; import { useAllIssues } from "~/queries/issues"; import { useLocation } from "react-router-dom"; +import { PATHS as ROOT_PATHS } from "~/router"; import { PATHS as PRODUCT_PATHS } from "~/routes/products"; +/** + * List of paths where the InstallButton must not be shown. + * + * Apart from obvious login and installation paths, it does not make sense to + * show the button neither, when the user is about to change the product nor + * when the installer is setting the chosen product. + * */ +const EXCLUDED_FROM = [ + ROOT_PATHS.login, + PRODUCT_PATHS.changeProduct, + PRODUCT_PATHS.progress, + ROOT_PATHS.installationProgress, + ROOT_PATHS.installationFinished, +]; + const InstallConfirmationPopup = ({ onAccept, onClose }) => { return ( @@ -60,8 +76,11 @@ according to the provided installation settings.", /** * Installation button * - * It starts the installation after asking for confirmation. + * It will be shown only if there aren't installation issues and the current + * path is not in the EXCLUDED_FROM list. * + * When clicked, it will ask for a confirmation before triggering the request + * for starting the installation. */ const InstallButton = (props: Omit) => { const issues = useAllIssues(); @@ -69,9 +88,7 @@ const InstallButton = (props: Omit) => { const location = useLocation(); if (!issues.isEmpty) return; - // Do not show the button if the user is about to change the product or the - // installer is configuring a product. - if ([PRODUCT_PATHS.changeProduct, PRODUCT_PATHS.progress].includes(location.pathname)) return; + if (EXCLUDED_FROM.includes(location.pathname)) return; const open = async () => setIsOpen(true); const close = () => setIsOpen(false);