From 3824cd7e47e151dc7844ddfc4f7508eb81e8b081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 18 Mar 2026 20:57:37 +0000 Subject: [PATCH 01/14] web: add TanStack Form dependency Adds @tanstack/react-form as a project dependency. TanStack Form will be used to replace the current ad-hoc form state management, starting with the network connection form. Check https://tanstack.com/form/latest --- web/package-lock.json | 101 ++++++++++++++++++++++++++++++++++++++++++ web/package.json | 1 + 2 files changed, 102 insertions(+) diff --git a/web/package-lock.json b/web/package-lock.json index c187749290..87b99eadc4 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -12,6 +12,7 @@ "@patternfly/patternfly": "^6.4.0", "@patternfly/react-core": "^6.4.1", "@patternfly/react-table": "^6.4.1", + "@tanstack/react-form": "^1.28.4", "@tanstack/react-query": "^5.90.21", "axios": "^1.13.6", "fast-sort": "^3.4.1", @@ -4846,6 +4847,47 @@ "url": "https://github.com/sponsors/gregberge" } }, + "node_modules/@tanstack/devtools-event-client": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/@tanstack/devtools-event-client/-/devtools-event-client-0.4.1.tgz", + "integrity": "sha512-GRxmPw4OHZ2oZeIEUkEwt/NDvuEqzEYRAjzUVMs+I0pd4C7k1ySOiuJK2CqF+K/yEAR3YZNkW3ExrpDarh9Vwg==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + } + }, + "node_modules/@tanstack/form-core": { + "version": "1.28.4", + "resolved": "https://registry.npmjs.org/@tanstack/form-core/-/form-core-1.28.4.tgz", + "integrity": "sha512-2eox5ePrJ6kvA1DXD5QHk/GeGr3VFZ0uYR63UgQOe7bUg6h1JfXaIMqTjZK9sdGyE4oRNqFpoW54H0pZM7nObQ==", + "license": "MIT", + "dependencies": { + "@tanstack/devtools-event-client": "^0.4.0", + "@tanstack/pacer-lite": "^0.1.1", + "@tanstack/store": "^0.9.1" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + } + }, + "node_modules/@tanstack/pacer-lite": { + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/@tanstack/pacer-lite/-/pacer-lite-0.1.1.tgz", + "integrity": "sha512-y/xtNPNt/YeyoVxE/JCx+T7yjEzpezmbb+toK8DDD1P4m7Kzs5YR956+7OKexG3f8aXgC3rLZl7b1V+yNUSy5w==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + } + }, "node_modules/@tanstack/query-core": { "version": "5.90.20", "resolved": "https://registry.npmjs.org/@tanstack/query-core/-/query-core-5.90.20.tgz", @@ -4856,6 +4898,46 @@ "url": "https://github.com/sponsors/tannerlinsley" } }, + "node_modules/@tanstack/react-form": { + "version": "1.28.4", + "resolved": "https://registry.npmjs.org/@tanstack/react-form/-/react-form-1.28.4.tgz", + "integrity": "sha512-ZGBwl9JM2u0kol7jAWpqAkr2JSHfXJaLPsFDZWPf+ewpVkwngTTW/rGgtoDe5uVpHoDIpOhzpPCAh6O1SjGEOg==", + "license": "MIT", + "dependencies": { + "@tanstack/form-core": "1.28.4", + "@tanstack/react-store": "^0.9.1" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + }, + "peerDependencies": { + "react": "^17.0.0 || ^18.0.0 || ^19.0.0" + }, + "peerDependenciesMeta": { + "@tanstack/react-start": { + "optional": true + } + } + }, + "node_modules/@tanstack/react-form/node_modules/@tanstack/react-store": { + "version": "0.9.2", + "resolved": "https://registry.npmjs.org/@tanstack/react-store/-/react-store-0.9.2.tgz", + "integrity": "sha512-Vt5usJE5sHG/cMechQfmwvwne6ktGCELe89Lmvoxe3LKRoFrhPa8OCKWs0NliG8HTJElEIj7PLtaBQIcux5pAQ==", + "license": "MIT", + "dependencies": { + "@tanstack/store": "0.9.2", + "use-sync-external-store": "^1.6.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + }, + "peerDependencies": { + "react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0", + "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0" + } + }, "node_modules/@tanstack/react-query": { "version": "5.90.21", "resolved": "https://registry.npmjs.org/@tanstack/react-query/-/react-query-5.90.21.tgz", @@ -4872,6 +4954,16 @@ "react": "^18 || ^19" } }, + "node_modules/@tanstack/store": { + "version": "0.9.2", + "resolved": "https://registry.npmjs.org/@tanstack/store/-/store-0.9.2.tgz", + "integrity": "sha512-K013lUJEFJK2ofFQ/hZKJUmCnpcV00ebLyOyFOWQvyQHUOZp/iYO84BM6aOGiV81JzwbX0APTVmW8YI7yiG5oA==", + "license": "MIT", + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + } + }, "node_modules/@testing-library/dom": { "version": "10.4.1", "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-10.4.1.tgz", @@ -18626,6 +18718,15 @@ "punycode": "^2.1.0" } }, + "node_modules/use-sync-external-store": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/use-sync-external-store/-/use-sync-external-store-1.6.0.tgz", + "integrity": "sha512-Pp6GSwGP/NrPIrxVFAIkOQeyw8lFenOHijQWkUTrDvrF4ALqylP2C/KCkeS9dpUM3KvYRQhna5vt7IL95+ZQ9w==", + "license": "MIT", + "peerDependencies": { + "react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0" + } + }, "node_modules/util-deprecate": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", diff --git a/web/package.json b/web/package.json index 1b8cabd259..c87aab00e4 100644 --- a/web/package.json +++ b/web/package.json @@ -96,6 +96,7 @@ "@patternfly/patternfly": "^6.4.0", "@patternfly/react-core": "^6.4.1", "@patternfly/react-table": "^6.4.1", + "@tanstack/react-form": "^1.28.4", "@tanstack/react-query": "^5.90.21", "axios": "^1.13.6", "fast-sort": "^3.4.1", From d711ab6dd447d4b4649362266ba8756c6eefdb16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 18 Mar 2026 21:00:17 +0000 Subject: [PATCH 02/14] refactor(web): start connection form reimplementation This commit starts the reimplementation of the network connection form using TanStack Form, replacing the previous ad-hoc form state management with a structured, library-backed approach. The form is intentionally minimal in this first iteration, name and interface fields only, to establish the pattern cleanly before adding complexity. The full form (IP settings, bond/bridge/VLAN config, etc) will be built incrementally in subsequent commits. TanStack Form was chosen for: - Declarative field registration with built-in state (dirty, touched, errors) per field, without manual useState plumbing - Type-safe form values inferred from defaultValues - A clean model for handling server errors via onSubmitAsync: the error is surfaced in the UI without throwing, keeping the form interactive so the user can read the message, adjust, and retry - A composable field component model that will allow reusable, form-aware widgets to be introduced as the form grows As patterns emerge across iterations (reusable field components, subform composition, complex validation), they will be extracted and documented as part of this reimplementation effort, or as part of the reimplementation of other forms in the codebase. Whichever comes first. --- .../network/ConnectionForm.test.tsx | 80 ++++++++++ web/src/components/network/ConnectionForm.tsx | 144 ++++++++++++++++++ web/src/hooks/form.tsx | 39 +++++ web/src/routes/network.tsx | 6 +- 4 files changed, 266 insertions(+), 3 deletions(-) create mode 100644 web/src/components/network/ConnectionForm.test.tsx create mode 100644 web/src/components/network/ConnectionForm.tsx create mode 100644 web/src/hooks/form.tsx diff --git a/web/src/components/network/ConnectionForm.test.tsx b/web/src/components/network/ConnectionForm.test.tsx new file mode 100644 index 0000000000..92760a75d3 --- /dev/null +++ b/web/src/components/network/ConnectionForm.test.tsx @@ -0,0 +1,80 @@ +/* + * Copyright (c) [2026] 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, waitFor } from "@testing-library/react"; +import { installerRender } from "~/test-utils"; +import ConnectionForm from "~/components/network/ConnectionForm"; +import { ConnectionType, DeviceState } from "~/types/network"; + +const mockDevice1 = { + name: "enp1s0", + type: ConnectionType.ETHERNET, + state: DeviceState.CONNECTED, +}; + +const mockDevice2 = { + name: "enp2s0", + type: ConnectionType.ETHERNET, + state: DeviceState.DISCONNECTED, +}; + +const mockMutateAsync = jest.fn().mockResolvedValue({}); + +jest.mock("~/hooks/model/config/network", () => ({ + useConnectionMutation: () => ({ mutateAsync: mockMutateAsync }), +})); + +jest.mock("~/hooks/model/system/network", () => ({ + useDevices: () => [mockDevice1, mockDevice2], +})); + +describe("ConnectionForm", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("renders common connections fields", () => { + installerRender(); + screen.getByLabelText("Name"); + screen.getByLabelText("Interface"); + }); + + it("submits with the entered values", async () => { + const { user } = installerRender(); + await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); + await user.click(screen.getByRole("button", { name: "Accept" })); + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ id: "Testing Connection 1", iface: "enp1s0" }), + ), + ); + }); + + it("shows the error returned by server when the mutation fails", async () => { + mockMutateAsync.mockRejectedValueOnce(new Error("Connection failed")); + const { user } = installerRender(); + await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); + await user.click(screen.getByRole("button", { name: "Accept" })); + await screen.findByText("Connection failed"); + }); +}); diff --git a/web/src/components/network/ConnectionForm.tsx b/web/src/components/network/ConnectionForm.tsx new file mode 100644 index 0000000000..66dc1f3848 --- /dev/null +++ b/web/src/components/network/ConnectionForm.tsx @@ -0,0 +1,144 @@ +/* + * Copyright (c) [2026] 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 { useNavigate } from "react-router"; +import { + Alert, + ActionGroup, + Button, + Form, + FormGroup, + FormSelect, + FormSelectOption, + TextInput, +} from "@patternfly/react-core"; +import Page from "~/components/core/Page"; +import { Connection } from "~/types/network"; +import { useConnectionMutation } from "~/hooks/model/config/network"; +import { useAppForm } from "~/hooks/form"; +import { useDevices } from "~/hooks/model/system/network"; +import { NETWORK } from "~/routes/paths"; +import { _ } from "~/i18n"; + +/** + * Form for creating a new network connection. + * + * @remarks + * Server errors are surfaced via TanStack Form's `validators.onSubmitAsync`. + * This is the recommended pattern for forms where the server acts as the + * validator: the call lives in the async validator, which returns `{ form: + * message }` on failure. TanStack then exposes the message via + * `state.errorMap.onSubmit.form` without throwing, keeping the button + * re-enabled for a retry. + * + * @see https://tanstack.com/form/latest/docs/framework/react/guides/validation + * @see https://github.com/TanStack/form/discussions/623#discussioncomment-13026699 + */ +export default function ConnectionForm() { + const navigate = useNavigate(); + const devices = useDevices(); + const { mutateAsync: updateConnection } = useConnectionMutation(); + const form = useAppForm({ + defaultValues: { + name: "", + interface: devices[0]?.name ?? "", + }, + validators: { + onSubmitAsync: async ({ value }) => { + const connection = new Connection(value.name, { iface: value.interface }); + try { + await updateConnection(connection); + } catch (e) { + return { form: e.message }; + } + }, + }, + onSubmit: () => navigate(-1), + }); + + const breadcrumbs = [{ label: _("Network"), path: NETWORK.root }, { label: _("New connection") }]; + + return ( + + +
{ + e.preventDefault(); + form.handleSubmit(); + }} + > + s.errorMap.onSubmit?.form}> + {(serverError) => + serverError && ( + + {serverError} + + ) + } + + + + {(field) => ( + + field.handleChange(v)} + /> + + )} + + + + {(field) => ( + + field.handleChange(v)} + > + {devices.map((d) => ( + + ))} + + + )} + + + + s.isSubmitting}> + {(isSubmitting) => ( + + )} + + + +
+ + + ); +} diff --git a/web/src/hooks/form.tsx b/web/src/hooks/form.tsx new file mode 100644 index 0000000000..9494c638b4 --- /dev/null +++ b/web/src/hooks/form.tsx @@ -0,0 +1,39 @@ +/* + * Copyright (c) [2026] 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 { createFormHook, createFormHookContexts } from "@tanstack/react-form"; + +const { fieldContext, formContext } = createFormHookContexts(); + +/** + * Application-wide TanStack Form hook. + * + * @see https://tanstack.com/form/latest/docs/framework/react/guides/form-composition + */ +const { useAppForm } = createFormHook({ + fieldContext, + formContext, + fieldComponents: {}, + formComponents: {}, +}); + +export { useAppForm }; diff --git a/web/src/routes/network.tsx b/web/src/routes/network.tsx index d447425e82..771f903a55 100644 --- a/web/src/routes/network.tsx +++ b/web/src/routes/network.tsx @@ -21,8 +21,8 @@ */ import React from "react"; +import ConnectionForm from "~/components/network/ConnectionForm"; import BindingSettingsForm from "~/components/network/BindingSettingsForm"; -import IpSettingsForm from "~/components/network/IpSettingsForm"; import NetworkPage from "~/components/network/NetworkPage"; import WifiConnectionForm from "~/components/network/WifiConnectionForm"; import WiredConnectionPage from "~/components/network/WiredConnectionPage"; @@ -40,11 +40,11 @@ const routes = (): Route => ({ { index: true, element: }, { path: PATHS.editConnection, - element: , + element: , }, { path: PATHS.newConnection, - element: , + element: , }, { path: PATHS.editBindingSettings, From 66d5f8e9b7e8d3e39b2fe4d2edabb24862b9396f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 20 Mar 2026 12:44:15 +0000 Subject: [PATCH 03/14] refactor(web): bring back method and gateway fields --- .../network/ConnectionForm.test.tsx | 37 +++++++++++++- web/src/components/network/ConnectionForm.tsx | 50 ++++++++++++++++++- 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/web/src/components/network/ConnectionForm.test.tsx b/web/src/components/network/ConnectionForm.test.tsx index 92760a75d3..4098197ae8 100644 --- a/web/src/components/network/ConnectionForm.test.tsx +++ b/web/src/components/network/ConnectionForm.test.tsx @@ -24,7 +24,7 @@ import React from "react"; import { screen, waitFor } from "@testing-library/react"; import { installerRender } from "~/test-utils"; import ConnectionForm from "~/components/network/ConnectionForm"; -import { ConnectionType, DeviceState } from "~/types/network"; +import { ConnectionMethod, ConnectionType, DeviceState } from "~/types/network"; const mockDevice1 = { name: "enp1s0", @@ -57,6 +57,19 @@ describe("ConnectionForm", () => { installerRender(); screen.getByLabelText("Name"); screen.getByLabelText("Interface"); + screen.getByLabelText("Method"); + }); + + it("defaults to automatic method and does not show gateway", () => { + installerRender(); + expect(screen.getByLabelText("Method")).toHaveValue(ConnectionMethod.AUTO); + expect(screen.queryByLabelText("Gateway")).not.toBeInTheDocument(); + }); + + it("shows the gateway field when method is manual", async () => { + const { user } = installerRender(); + await user.selectOptions(screen.getByLabelText("Method"), ConnectionMethod.MANUAL); + screen.getByLabelText("Gateway"); }); it("submits with the entered values", async () => { @@ -70,6 +83,28 @@ describe("ConnectionForm", () => { ); }); + it("submits with gateway when method is manual", async () => { + const { user } = installerRender(); + await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); + await user.selectOptions(screen.getByLabelText("Method"), ConnectionMethod.MANUAL); + await user.type(screen.getByLabelText("Gateway"), "192.168.1.1"); + await user.click(screen.getByRole("button", { name: "Accept" })); + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ method4: ConnectionMethod.MANUAL, gateway4: "192.168.1.1" }), + ), + ); + }); + + it("does not submit gateway when method is automatic", async () => { + const { user } = installerRender(); + await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); + await user.click(screen.getByRole("button", { name: "Accept" })); + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith(expect.objectContaining({ gateway4: "" })), + ); + }); + it("shows the error returned by server when the mutation fails", async () => { mockMutateAsync.mockRejectedValueOnce(new Error("Connection failed")); const { user } = installerRender(); diff --git a/web/src/components/network/ConnectionForm.tsx b/web/src/components/network/ConnectionForm.tsx index 66dc1f3848..f8eb59a197 100644 --- a/web/src/components/network/ConnectionForm.tsx +++ b/web/src/components/network/ConnectionForm.tsx @@ -33,13 +33,18 @@ import { TextInput, } from "@patternfly/react-core"; import Page from "~/components/core/Page"; -import { Connection } from "~/types/network"; +import { Connection, ConnectionMethod } from "~/types/network"; import { useConnectionMutation } from "~/hooks/model/config/network"; import { useAppForm } from "~/hooks/form"; import { useDevices } from "~/hooks/model/system/network"; import { NETWORK } from "~/routes/paths"; import { _ } from "~/i18n"; +const METHOD_OPTIONS = [ + { value: ConnectionMethod.AUTO, label: _("Automatic (DHCP)") }, + { value: ConnectionMethod.MANUAL, label: _("Manual") }, +]; + /** * Form for creating a new network connection. * @@ -58,14 +63,21 @@ export default function ConnectionForm() { const navigate = useNavigate(); const devices = useDevices(); const { mutateAsync: updateConnection } = useConnectionMutation(); + const form = useAppForm({ defaultValues: { name: "", interface: devices[0]?.name ?? "", + method4: ConnectionMethod.AUTO, + gateway4: "", }, validators: { onSubmitAsync: async ({ value }) => { - const connection = new Connection(value.name, { iface: value.interface }); + const connection = new Connection(value.name, { + iface: value.interface, + method4: value.method4, + gateway4: value.method4 === ConnectionMethod.MANUAL ? value.gateway4 : undefined, + }); try { await updateConnection(connection); } catch (e) { @@ -125,6 +137,40 @@ export default function ConnectionForm() { )} + + {(field) => ( + + field.handleChange(v as ConnectionMethod)} + > + {METHOD_OPTIONS.map((o) => ( + + ))} + + + )} + + + s.values.method4}> + {(method4) => + method4 === ConnectionMethod.MANUAL && ( + + {(field) => ( + + field.handleChange(v)} + /> + + )} + + ) + } + + s.isSubmitting}> {(isSubmitting) => ( From 891869be97b9103f1e84903450d88cb907702e77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 20 Mar 2026 13:09:51 +0000 Subject: [PATCH 04/14] feat(web): add independent IPv4 and IPv6 method and gateway fields Related to https://bugzilla.suse.com/show_bug.cgi?id=1259067 --- .../network/ConnectionForm.test.tsx | 51 ++++---- web/src/components/network/ConnectionForm.tsx | 117 ++++++++++++------ 2 files changed, 111 insertions(+), 57 deletions(-) diff --git a/web/src/components/network/ConnectionForm.test.tsx b/web/src/components/network/ConnectionForm.test.tsx index 4098197ae8..ddaa6fcbd7 100644 --- a/web/src/components/network/ConnectionForm.test.tsx +++ b/web/src/components/network/ConnectionForm.test.tsx @@ -53,23 +53,34 @@ describe("ConnectionForm", () => { jest.clearAllMocks(); }); - it("renders common connections fields", () => { + it("renders common connection fields", () => { installerRender(); screen.getByLabelText("Name"); screen.getByLabelText("Interface"); - screen.getByLabelText("Method"); + screen.getByLabelText("IPv4 Method"); + screen.getByLabelText("IPv6 Method"); }); - it("defaults to automatic method and does not show gateway", () => { + it("defaults both methods to automatic and does not show gateways", () => { installerRender(); - expect(screen.getByLabelText("Method")).toHaveValue(ConnectionMethod.AUTO); - expect(screen.queryByLabelText("Gateway")).not.toBeInTheDocument(); + expect(screen.getByLabelText("IPv4 Method")).toHaveValue(ConnectionMethod.AUTO); + expect(screen.getByLabelText("IPv6 Method")).toHaveValue(ConnectionMethod.AUTO); + expect(screen.queryByLabelText("IPv4 Gateway")).not.toBeInTheDocument(); + expect(screen.queryByLabelText("IPv6 Gateway")).not.toBeInTheDocument(); }); - it("shows the gateway field when method is manual", async () => { + it("shows IPv4 gateway when IPv4 method is manual", async () => { const { user } = installerRender(); - await user.selectOptions(screen.getByLabelText("Method"), ConnectionMethod.MANUAL); - screen.getByLabelText("Gateway"); + await user.selectOptions(screen.getByLabelText("IPv4 Method"), ConnectionMethod.MANUAL); + screen.getByLabelText("IPv4 Gateway"); + expect(screen.queryByLabelText("IPv6 Gateway")).not.toBeInTheDocument(); + }); + + it("shows IPv6 gateway when IPv6 method is manual", async () => { + const { user } = installerRender(); + await user.selectOptions(screen.getByLabelText("IPv6 Method"), ConnectionMethod.MANUAL); + screen.getByLabelText("IPv6 Gateway"); + expect(screen.queryByLabelText("IPv4 Gateway")).not.toBeInTheDocument(); }); it("submits with the entered values", async () => { @@ -83,28 +94,26 @@ describe("ConnectionForm", () => { ); }); - it("submits with gateway when method is manual", async () => { + it("submits with gateways when both methods are manual", async () => { const { user } = installerRender(); await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); - await user.selectOptions(screen.getByLabelText("Method"), ConnectionMethod.MANUAL); - await user.type(screen.getByLabelText("Gateway"), "192.168.1.1"); + await user.selectOptions(screen.getByLabelText("IPv4 Method"), ConnectionMethod.MANUAL); + await user.type(screen.getByLabelText("IPv4 Gateway"), "192.168.1.1"); + await user.selectOptions(screen.getByLabelText("IPv6 Method"), ConnectionMethod.MANUAL); + await user.type(screen.getByLabelText("IPv6 Gateway"), "::1"); await user.click(screen.getByRole("button", { name: "Accept" })); await waitFor(() => expect(mockMutateAsync).toHaveBeenCalledWith( - expect.objectContaining({ method4: ConnectionMethod.MANUAL, gateway4: "192.168.1.1" }), + expect.objectContaining({ + method4: ConnectionMethod.MANUAL, + gateway4: "192.168.1.1", + method6: ConnectionMethod.MANUAL, + gateway6: "::1", + }), ), ); }); - it("does not submit gateway when method is automatic", async () => { - const { user } = installerRender(); - await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); - await user.click(screen.getByRole("button", { name: "Accept" })); - await waitFor(() => - expect(mockMutateAsync).toHaveBeenCalledWith(expect.objectContaining({ gateway4: "" })), - ); - }); - it("shows the error returned by server when the mutation fails", async () => { mockMutateAsync.mockRejectedValueOnce(new Error("Connection failed")); const { user } = installerRender(); diff --git a/web/src/components/network/ConnectionForm.tsx b/web/src/components/network/ConnectionForm.tsx index f8eb59a197..e46e8963b7 100644 --- a/web/src/components/network/ConnectionForm.tsx +++ b/web/src/components/network/ConnectionForm.tsx @@ -31,6 +31,7 @@ import { FormSelect, FormSelectOption, TextInput, + Split, } from "@patternfly/react-core"; import Page from "~/components/core/Page"; import { Connection, ConnectionMethod } from "~/types/network"; @@ -38,11 +39,11 @@ import { useConnectionMutation } from "~/hooks/model/config/network"; import { useAppForm } from "~/hooks/form"; import { useDevices } from "~/hooks/model/system/network"; import { NETWORK } from "~/routes/paths"; -import { _ } from "~/i18n"; +import { _, N_ } from "~/i18n"; const METHOD_OPTIONS = [ - { value: ConnectionMethod.AUTO, label: _("Automatic (DHCP)") }, - { value: ConnectionMethod.MANUAL, label: _("Manual") }, + { value: ConnectionMethod.AUTO, label: N_("Automatic (DHCP)") }, + { value: ConnectionMethod.MANUAL, label: N_("Manual") }, ]; /** @@ -70,13 +71,17 @@ export default function ConnectionForm() { interface: devices[0]?.name ?? "", method4: ConnectionMethod.AUTO, gateway4: "", + method6: ConnectionMethod.AUTO, + gateway6: "", }, validators: { onSubmitAsync: async ({ value }) => { const connection = new Connection(value.name, { iface: value.interface, method4: value.method4, - gateway4: value.method4 === ConnectionMethod.MANUAL ? value.gateway4 : undefined, + gateway4: value.gateway4, + method6: value.method6, + gateway6: value.gateway6, }); try { await updateConnection(connection); @@ -137,39 +142,79 @@ export default function ConnectionForm() { )} - - {(field) => ( - - field.handleChange(v as ConnectionMethod)} - > - {METHOD_OPTIONS.map((o) => ( - - ))} - - - )} - + + + {(field) => ( + + field.handleChange(v as ConnectionMethod)} + > + {METHOD_OPTIONS.map(({ value, label }) => ( + // eslint-disable-next-line agama-i18n/string-literals + + ))} + + + )} + - s.values.method4}> - {(method4) => - method4 === ConnectionMethod.MANUAL && ( - - {(field) => ( - - field.handleChange(v)} - /> - - )} - - ) - } - + s.values.method4}> + {(method4) => + method4 === ConnectionMethod.MANUAL && ( + + {(field) => ( + + field.handleChange(v)} + /> + + )} + + ) + } + + + + + + {(field) => ( + + field.handleChange(v as ConnectionMethod)} + > + {METHOD_OPTIONS.map(({ value, label }) => ( + // eslint-disable-next-line agama-i18n/string-literals + + ))} + + + )} + + + s.values.method6}> + {(method6) => + method6 === ConnectionMethod.MANUAL && ( + + {(field) => ( + + field.handleChange(v)} + /> + + )} + + ) + } + + s.isSubmitting}> From cd9e90076da838307b5b06b1e090be0f69d67c8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 22 Mar 2026 10:29:41 +0000 Subject: [PATCH 05/14] Bring back IP Addresses field as a textarea Replaces the former AddressesDataList with a simple textarea as an intermediate step. The goal is to revisit the multi-value input later with a more fluent and accessible approach. Both IPv4 and IPv6 addresses are handled in a single field. Addresses without a prefix default to /24 for IPv4 and /64 for IPv6. The field label reflects the current requirement based on the selected methods, following the field labelling conventions in src/components/form/conventions.md (temporary file will be moved elsewhere later). --- web/src/components/form/conventions.md | 61 +++++++++++++ .../network/ConnectionForm.test.tsx | 91 ++++++++++++++++--- web/src/components/network/ConnectionForm.tsx | 72 ++++++++++++++- 3 files changed, 209 insertions(+), 15 deletions(-) create mode 100644 web/src/components/form/conventions.md diff --git a/web/src/components/form/conventions.md b/web/src/components/form/conventions.md new file mode 100644 index 0000000000..34e1de45d3 --- /dev/null +++ b/web/src/components/form/conventions.md @@ -0,0 +1,61 @@ +# Form conventions + +This document captures the conventions achieved during the reimplementation +of the application forms, starting with the network connection form. They apply +to all forms across the application. + +## Field visibility + +Show only the fields that are needed. The default state of a form should be as +minimal as possible, with only fields that are always relevant visible on first +render. + +Fields that are only relevant under certain conditions should be hidden until +that condition is met. Whether a conditionally shown field is required or +optional depends on the field itself, not on the fact that it was hidden. + +## Field labels and suffixes + +### No suffix, required field + +The default. A field with no suffix is expected to have a value. + +Before rendering a field without a suffix, consider whether a sensible default +value can be provided. If so, pre-fill it. A field with a default is not +optional, it just has a pre-filled value, and the user may or may not change it. + +### `(optional)` suffix + +Used when a field can legitimately be left blank. The field may be always +visible or conditionally visible, it does not matter. What matters is that +leaving it blank is valid. + +Before reaching for `(optional)`, consider whether the field can simply be +hidden. If a field is only relevant under a specific condition and is always +required under that condition, hiding it until needed is cleaner than showing +it with a suffix. + +`(optional)` is appropriate when the field must be rendered and can be left +blank, for example because hiding it would harm usability or the workflow. + +### Clarifying suffix + +Used in rare cases where the requirement is neither simply "required" nor +simply "optional", for example when a field serves multiple protocols and the +requirement depends on the state of other fields. In these cases a short suffix +describes what is expected, e.g. `(IPv4 required)` or +`(IPv4 and IPv6 required)`. + +This pattern should be used sparingly. If it appears often, it is likely a sign +that the form might be restructured. + +## Summary + +| Situation | Approach | +| ------------------------------------------------------------- | ---------------------------------------------------------- | +| Field always required | Show with no suffix | +| Field only relevant conditionally, always required when shown | Hide until condition is met, show with no suffix | +| Field only relevant conditionally, optional when shown | Hide until condition is met, show with `(optional)` suffix | +| Field must always show, has a sensible default | Show with no suffix, pre-fill the default | +| Field must always show, no default, can be blank | Show with `(optional)` suffix | +| Field must show, requirement depends on other fields | Show with clarifying suffix | diff --git a/web/src/components/network/ConnectionForm.test.tsx b/web/src/components/network/ConnectionForm.test.tsx index ddaa6fcbd7..cd0c67e5f0 100644 --- a/web/src/components/network/ConnectionForm.test.tsx +++ b/web/src/components/network/ConnectionForm.test.tsx @@ -61,6 +61,25 @@ describe("ConnectionForm", () => { screen.getByLabelText("IPv6 Method"); }); + it("submits with the entered values", async () => { + const { user } = installerRender(); + await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); + await user.click(screen.getByRole("button", { name: "Accept" })); + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ id: "Testing Connection 1", iface: "enp1s0" }), + ), + ); + }); + + it("shows the error returned by server when the mutation fails", async () => { + mockMutateAsync.mockRejectedValueOnce(new Error("Connection failed")); + const { user } = installerRender(); + await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); + await user.click(screen.getByRole("button", { name: "Accept" })); + await screen.findByText("Connection failed"); + }); + it("defaults both methods to automatic and does not show gateways", () => { installerRender(); expect(screen.getByLabelText("IPv4 Method")).toHaveValue(ConnectionMethod.AUTO); @@ -83,17 +102,6 @@ describe("ConnectionForm", () => { expect(screen.queryByLabelText("IPv4 Gateway")).not.toBeInTheDocument(); }); - it("submits with the entered values", async () => { - const { user } = installerRender(); - await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); - await user.click(screen.getByRole("button", { name: "Accept" })); - await waitFor(() => - expect(mockMutateAsync).toHaveBeenCalledWith( - expect.objectContaining({ id: "Testing Connection 1", iface: "enp1s0" }), - ), - ); - }); - it("submits with gateways when both methods are manual", async () => { const { user } = installerRender(); await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); @@ -114,11 +122,66 @@ describe("ConnectionForm", () => { ); }); - it("shows the error returned by server when the mutation fails", async () => { - mockMutateAsync.mockRejectedValueOnce(new Error("Connection failed")); + it("submits addresses parsed from the textarea", async () => { const { user } = installerRender(); await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); + await user.type( + screen.getByLabelText("IP Addresses (optional)"), + "192.168.1.1/24 2001:db8::1/64", + ); await user.click(screen.getByRole("button", { name: "Accept" })); - await screen.findByText("Connection failed"); + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ + addresses: [ + { address: "192.168.1.1", prefix: 24 }, + { address: "2001:db8::1", prefix: 64 }, + ], + }), + ), + ); + }); + + it("adds default prefix when address has none", async () => { + const { user } = installerRender(); + await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); + await user.type(screen.getByLabelText("IP Addresses (optional)"), "192.168.1.1 2001:db8::1"); + await user.click(screen.getByRole("button", { name: "Accept" })); + await waitFor(() => + expect(mockMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ + addresses: [ + { address: "192.168.1.1", prefix: 24 }, + { address: "2001:db8::1", prefix: 64 }, + ], + }), + ), + ); + }); + + describe("IP Addresses label", () => { + it("shows '(optional)' when both methods are automatic", () => { + installerRender(); + screen.getByLabelText("IP Addresses (optional)"); + }); + + it("shows '(IPv4 required)' when only IPv4 method is manual", async () => { + const { user } = installerRender(); + await user.selectOptions(screen.getByLabelText("IPv4 Method"), ConnectionMethod.MANUAL); + screen.getByLabelText("IP Addresses (IPv4 required)"); + }); + + it("shows '(IPv6 required)' when only IPv6 method is manual", async () => { + const { user } = installerRender(); + await user.selectOptions(screen.getByLabelText("IPv6 Method"), ConnectionMethod.MANUAL); + screen.getByLabelText("IP Addresses (IPv6 required)"); + }); + + it("shows '(IPv4 and IPv6 required)' when both methods are manual", async () => { + const { user } = installerRender(); + await user.selectOptions(screen.getByLabelText("IPv4 Method"), ConnectionMethod.MANUAL); + await user.selectOptions(screen.getByLabelText("IPv6 Method"), ConnectionMethod.MANUAL); + screen.getByLabelText("IP Addresses (IPv4 and IPv6 required)"); + }); }); }); diff --git a/web/src/components/network/ConnectionForm.tsx b/web/src/components/network/ConnectionForm.tsx index e46e8963b7..f95e705e02 100644 --- a/web/src/components/network/ConnectionForm.tsx +++ b/web/src/components/network/ConnectionForm.tsx @@ -28,13 +28,18 @@ import { Button, Form, FormGroup, + FormHelperText, FormSelect, FormSelectOption, - TextInput, + HelperText, + HelperTextItem, Split, + TextArea, + TextInput, } from "@patternfly/react-core"; import Page from "~/components/core/Page"; import { Connection, ConnectionMethod } from "~/types/network"; +import { buildAddress } from "~/utils/network"; import { useConnectionMutation } from "~/hooks/model/config/network"; import { useAppForm } from "~/hooks/form"; import { useDevices } from "~/hooks/model/system/network"; @@ -46,6 +51,26 @@ const METHOD_OPTIONS = [ { value: ConnectionMethod.MANUAL, label: N_("Manual") }, ]; +const IPV4_DEFAULT_PREFIX = 24; +const IPV6_DEFAULT_PREFIX = 64; + +/** Ensures a CIDR string has a prefix, adding a protocol-appropriate default if missing. */ +const withPrefix = (address: string): string => { + if (address.includes("/")) return address; + return address.includes(":") + ? `${address}/${IPV6_DEFAULT_PREFIX}` + : `${address}/${IPV4_DEFAULT_PREFIX}`; +}; + +/** Parses a space/newline separated string of addresses into IPAddress objects. */ +const parseAddresses = (raw: string) => + raw + .split(/[\s\n]+/) + .map((s) => s.trim()) + .filter(Boolean) + .map(withPrefix) + .map(buildAddress); + /** * Form for creating a new network connection. * @@ -73,6 +98,7 @@ export default function ConnectionForm() { gateway4: "", method6: ConnectionMethod.AUTO, gateway6: "", + addresses: "", }, validators: { onSubmitAsync: async ({ value }) => { @@ -82,6 +108,7 @@ export default function ConnectionForm() { gateway4: value.gateway4, method6: value.method6, gateway6: value.gateway6, + addresses: parseAddresses(value.addresses), }); try { await updateConnection(connection); @@ -216,6 +243,49 @@ export default function ConnectionForm() { + ({ + method4: s.values.method4, + method6: s.values.method6, + })} + > + {({ method4, method6 }) => { + const manual4 = method4 === ConnectionMethod.MANUAL; + const manual6 = method6 === ConnectionMethod.MANUAL; + + const addressesLabel = () => { + if (manual4 && manual6) return _("IP Addresses (IPv4 and IPv6 required)"); + if (manual4) return _("IP Addresses (IPv4 required)"); + if (manual6) return _("IP Addresses (IPv6 required)"); + return _("IP Addresses (optional)"); + }; + const label = addressesLabel(); + + return ( + + {(field) => ( + +