diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index e24a93e7fb..3f0b2a424d 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Tue Aug 13 14:57:21 UTC 2024 - David Diaz + +- Allow links look like buttons again (gh#openSUSE/agama#1536). + ------------------------------------------------------------------- Fri Jul 26 11:42:05 UTC 2024 - Imobach Gonzalez Sosa diff --git a/web/src/components/core/Link.test.tsx b/web/src/components/core/Link.test.tsx new file mode 100644 index 0000000000..46075c7d0d --- /dev/null +++ b/web/src/components/core/Link.test.tsx @@ -0,0 +1,101 @@ +/* + * 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 { installerRender } from "~/test-utils"; +import { Link } from "~/components/core"; + +describe("Link", () => { + it("renders an HTML `a` tag with the `href` attribute set to given `to` prop", () => { + installerRender(Agama Link); + const link = screen.getByRole("link", { name: "Agama Link" }) as HTMLLinkElement; + // NOTE: Link uses ReactRouter#useHref hook which is mocked in test-utils.js + // TODO: Not using toHaveAttribute("href", "somewhere") because some weird problems at CI + expect(link.href).toContain("somewhere"); + }); + + it("renders it as primary when either, using a truthy `isPrimary` prop or `variant` is set to primary", () => { + const { rerender } = installerRender(Agama Link); + const link = screen.getByRole("link", { name: "Agama Link" }); + + expect(link.classList.contains("pf-m-primary")).not.toBe(true); + + rerender( + + Agama Link + , + ); + expect(link.classList.contains("pf-m-primary")).toBe(true); + + rerender( + + {" "} + Agama Link + , + ); + expect(link.classList.contains("pf-m-primary")).not.toBe(true); + + rerender( + + Agama Link + , + ); + expect(link.classList.contains("pf-m-primary")).toBe(true); + + rerender( + + {" "} + Agama Link + , + ); + expect(link.classList.contains("pf-m-primary")).toBe(true); + }); + + it("renders it as secondary when neither is given, a truthy `isPrimary` nor `variant`", () => { + const { rerender } = installerRender(Agama Link); + const link = screen.getByRole("link", { name: "Agama Link" }); + + expect(link.classList.contains("pf-m-secondary")).toBe(true); + + rerender( + + Agama Link + , + ); + expect(link.classList.contains("pf-m-secondary")).toBe(true); + + rerender( + // Unexpected, but possible since isPrimary is just a "helper" + + Agama Link + , + ); + expect(link.classList.contains("pf-m-secondary")).not.toBe(true); + + rerender( + + Agama Link + , + ); + expect(link.classList.contains("pf-m-secondary")).not.toBe(true); + }); +}); diff --git a/web/src/components/core/ButtonLink.tsx b/web/src/components/core/Link.tsx similarity index 52% rename from web/src/components/core/ButtonLink.tsx rename to web/src/components/core/Link.tsx index 6031085014..800076a3b7 100644 --- a/web/src/components/core/ButtonLink.tsx +++ b/web/src/components/core/Link.tsx @@ -20,22 +20,28 @@ */ import React from "react"; -import { Link } from "react-router-dom"; -import buttonStyles from "@patternfly/react-styles/css/components/Button/button"; +import { Button, ButtonProps } from "@patternfly/react-core"; +import { useHref } from "react-router-dom"; -// TODO: Evaluate which is better, this approach or just using a -// PF/Button with onClick callback and "component" prop sets as "a" +type LinkProps = Omit & { + /** The target route */ + to: string; + /** Whether use PF/Button primary variant */ + isPrimary?: boolean; +}; -export default function ButtonLink({ to, isPrimary = false, children, ...props }) { +/** + * Returns an HTML `` tag built on top of PF/Button and useHref ReactRouter hook + * + * @note when isPrimary not given or false and props does not contain a variant prop, + * it will default to "secondary" variant + */ +export default function Link({ to, isPrimary, variant, children, ...props }: LinkProps) { + const href = useHref(to); + const linkVariant = isPrimary ? "primary" : variant || "secondary"; return ( - + ); } diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index ada3c45abf..d755b19ce4 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -51,7 +51,7 @@ export { default as ServerError } from "./ServerError"; export { default as ExpandableSelector } from "./ExpandableSelector"; export { default as TreeTable } from "./TreeTable"; export { default as CardField } from "./CardField"; -export { default as ButtonLink } from "./ButtonLink"; +export { default as Link } from "./Link"; export { default as EmptyState } from "./EmptyState"; export { default as InstallerOptions } from "./InstallerOptions"; export { default as Drawer } from "./Drawer"; diff --git a/web/src/components/l10n/L10nPage.jsx b/web/src/components/l10n/L10nPage.jsx index 790628a901..8e01bffbfc 100644 --- a/web/src/components/l10n/L10nPage.jsx +++ b/web/src/components/l10n/L10nPage.jsx @@ -21,7 +21,7 @@ import React from "react"; import { Gallery, GalleryItem } from "@patternfly/react-core"; -import { ButtonLink, CardField, Page } from "~/components/core"; +import { Link, CardField, Page } from "~/components/core"; import { PATHS } from "~/routes/l10n"; import { _ } from "~/i18n"; import { useL10n } from "~/queries/l10n"; @@ -55,17 +55,17 @@ export default function L10nPage() { label={_("Language")} value={locale ? `${locale.name} - ${locale.territory}` : _("Not selected yet")} > - + {locale ? _("Change") : _("Select")} - +
- + {keymap ? _("Change") : _("Select")} - +
@@ -74,9 +74,9 @@ export default function L10nPage() { label={_("Time zone")} value={timezone ? (timezone.parts || []).join(" - ") : _("Not selected yet")} > - + {timezone ? _("Change") : _("Select")} - + diff --git a/web/src/components/l10n/L10nPage.test.jsx b/web/src/components/l10n/L10nPage.test.jsx index 808f5ce5aa..3737fb0ebc 100644 --- a/web/src/components/l10n/L10nPage.test.jsx +++ b/web/src/components/l10n/L10nPage.test.jsx @@ -20,7 +20,8 @@ */ import React from "react"; -import { render, screen, within } from "@testing-library/react"; +import { screen, within } from "@testing-library/react"; +import { installerRender } from "~/test-utils"; import L10nPage from "~/components/l10n/L10nPage"; let mockLoadedData; @@ -40,12 +41,6 @@ const timezones = [ { id: "Europe/Madrid", parts: ["Europe", "Madrid"] }, ]; -jest.mock("react-router-dom", () => ({ - ...jest.requireActual("react-router-dom"), - // TODO: mock the link because it needs a working router. - Link: ({ children }) => , -})); - jest.mock("~/queries/l10n", () => ({ useL10n: () => mockLoadedData, })); @@ -62,7 +57,7 @@ beforeEach(() => { }); it("renders a section for configuring the language", () => { - render(); + installerRender(); const region = screen.getByRole("region", { name: "Language" }); within(region).getByText("English - United States"); within(region).getByText("Change"); @@ -74,7 +69,7 @@ describe("if there is no selected language", () => { }); it("renders a button for selecting a language", () => { - render(); + installerRender(); const region = screen.getByRole("region", { name: "Language" }); within(region).getByText("Not selected yet"); within(region).getByText("Select"); @@ -82,7 +77,7 @@ describe("if there is no selected language", () => { }); it("renders a section for configuring the keyboard", () => { - render(); + installerRender(); const region = screen.getByRole("region", { name: "Keyboard" }); within(region).getByText("English"); within(region).getByText("Change"); @@ -94,7 +89,7 @@ describe("if there is no selected keyboard", () => { }); it("renders a button for selecting a keyboard", () => { - render(); + installerRender(); const region = screen.getByRole("region", { name: "Keyboard" }); within(region).getByText("Not selected yet"); within(region).getByText("Select"); @@ -102,7 +97,7 @@ describe("if there is no selected keyboard", () => { }); it("renders a section for configuring the time zone", () => { - render(); + installerRender(); const region = screen.getByRole("region", { name: "Time zone" }); within(region).getByText("Europe - Berlin"); within(region).getByText("Change"); @@ -114,7 +109,7 @@ describe("if there is no selected time zone", () => { }); it("renders a button for selecting a time zone", () => { - render(); + installerRender(); const region = screen.getByRole("region", { name: "Time zone" }); within(region).getByText("Not selected yet"); within(region).getByText("Select"); diff --git a/web/src/components/network/NetworkPage.jsx b/web/src/components/network/NetworkPage.jsx index dd1216d346..e68edf8dac 100644 --- a/web/src/components/network/NetworkPage.jsx +++ b/web/src/components/network/NetworkPage.jsx @@ -23,7 +23,7 @@ import React from "react"; import { CardBody, Grid, GridItem } from "@patternfly/react-core"; -import { ButtonLink, CardField, EmptyState, Page } from "~/components/core"; +import { Link, CardField, EmptyState, Page } from "~/components/core"; import { ConnectionsTable } from "~/components/network"; import { formatIp } from "~/client/network/utils"; import { useNetwork, useNetworkConfigChanges } from "~/queries/network"; @@ -71,9 +71,9 @@ export default function NetworkPage() { + {activeConnection ? _("Change") : _("Connect")} - + } > diff --git a/web/src/components/network/WifiNetworksListPage.jsx b/web/src/components/network/WifiNetworksListPage.jsx index 12e362a298..255524f4a0 100644 --- a/web/src/components/network/WifiNetworksListPage.jsx +++ b/web/src/components/network/WifiNetworksListPage.jsx @@ -47,7 +47,7 @@ import { generatePath } from "react-router-dom"; import { useQueryClient } from "@tanstack/react-query"; import { Icon } from "~/components/layout"; import { WifiConnectionForm } from "~/components/network"; -import { ButtonLink } from "~/components/core"; +import { Link } from "~/components/core"; import { DeviceState } from "~/client/network/model"; import { formatIp } from "~/client/network/utils"; import { useInstallerClient } from "~/context/installer"; @@ -108,12 +108,12 @@ const WifiDrawerPanelBody = ({ network, onCancel }) => { if (network.settings && !network.device) { return ( - await client.network.connectTo(network.settings)}> + await client.network.connectTo(network.settings)}> {_("Connect")} - - + + {_("Edit")} - + @@ -132,12 +132,12 @@ const WifiDrawerPanelBody = ({ network, onCancel }) => { - await client.network.disconnect(network.settings)}> + await client.network.disconnect(network.settings)}> {_("Disconnect")} - - + + {_("Edit")} - +