Skip to content

Commit

Permalink
fix(web): render InstallButton only when needed
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dgdavid committed Oct 30, 2024
1 parent b0d9d1b commit fe18300
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 17 deletions.
22 changes: 9 additions & 13 deletions web/src/components/core/InstallButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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(<InstallButton />);
expect(container).toBeEmptyDOMElement();
});
});

describe("but installer is configuring a product", () => {
beforeEach(() => {
mockRoutes(PRODUCT_PATHS.progress);
mockRoutes(path);
});

it("renders nothing", () => {
Expand Down
25 changes: 21 additions & 4 deletions web/src/components/core/InstallButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Popup title={_("Confirm Installation")} isOpen>
Expand Down Expand Up @@ -60,18 +76,19 @@ 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<ButtonProps, "onClick">) => {
const issues = useAllIssues();
const [isOpen, setIsOpen] = useState(false);
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);
Expand Down

0 comments on commit fe18300

Please sign in to comment.