diff --git a/web/src/components/form/ArrayField.test.tsx b/web/src/components/form/ArrayField.test.tsx new file mode 100644 index 0000000000..bd0af1589a --- /dev/null +++ b/web/src/components/form/ArrayField.test.tsx @@ -0,0 +1,446 @@ +/* + * 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 } from "@testing-library/react"; +import { installerRender } from "~/test-utils"; +import { useAppForm } from "~/hooks/form"; + +type TestFormProps = { + defaultValues?: string[]; + validateOnChange?: (v: string) => string | undefined; + validateOnSubmit?: (v: string) => string | undefined; + skipDuplicates?: boolean; + helperText?: string; + /** Simulates a TanStack Form field-level error returned by onSubmitAsync. */ + fieldError?: string; +}; + +function TestForm({ + defaultValues = [], + validateOnChange, + validateOnSubmit, + skipDuplicates = false, + helperText, + fieldError, +}: TestFormProps) { + const form = useAppForm({ + defaultValues: { tags: defaultValues }, + validators: { + onSubmitAsync: fieldError ? async () => ({ fields: { tags: fieldError } }) : undefined, + }, + }); + + return ( + +
{ + e.preventDefault(); + form.handleSubmit(); + }} + > + + {(field) => ( + + )} + + + +
+
+ ); +} + +describe("ArrayField", () => { + it("renders label and usage", () => { + installerRender(); + screen.getByText("Tags"); + screen.getByText(/Enter or Tab to add/); + }); + + it("renders given existing values", () => { + installerRender(); + screen.getByText("alpha"); + screen.getByText("beta"); + }); + + describe("adding entries", () => { + it("adds a value on Enter", async () => { + const { user } = installerRender(); + await user.type(screen.getByRole("textbox", { name: "Tags" }), "gamma"); + await user.keyboard("{Enter}"); + screen.getByText("gamma"); + }); + + it("adds a value on Tab", async () => { + const { user } = installerRender(); + await user.type(screen.getByRole("textbox", { name: "Tags" }), "delta"); + await user.tab(); + screen.getByText("delta"); + }); + + it("commits the draft on blur", async () => { + const { user } = installerRender(); + await user.type(screen.getByRole("textbox", { name: "Tags" }), "epsilon"); + await user.click(screen.getByRole("button", { name: "Other" })); + screen.getByText("epsilon"); + }); + + it("does not commit an empty draft on blur", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.click(screen.getByRole("button", { name: "Other" })); + expect(screen.getAllByRole("option")).toHaveLength(1); + }); + }); + + describe("removing entries", () => { + it("removes a value via the entry removal button", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("button", { name: "Remove alpha" })); + expect(screen.queryByText("alpha")).not.toBeInTheDocument(); + }); + }); + + describe("editing entries", () => { + it("moves an entry to the draft input when clicked", async () => { + const { user } = installerRender(); + const input = screen.getByRole("textbox", { name: "Tags" }); + await user.click(screen.getByRole("option", { name: "alpha" })); + expect(screen.queryByRole("option", { name: "alpha" })).not.toBeInTheDocument(); + expect(input).toHaveValue("alpha"); + }); + + it("puts the active entry back in the input on Enter", async () => { + const { user } = installerRender(); + const input = screen.getByRole("textbox", { name: "Tags" }); + await user.click(input); + await user.keyboard("{ArrowLeft}{Enter}"); + expect(screen.queryByRole("option", { name: "alpha" })).not.toBeInTheDocument(); + expect(input).toHaveValue("alpha"); + }); + + it("puts the active entry back in the input on Space", async () => { + const { user } = installerRender(); + const input = screen.getByRole("textbox", { name: "Tags" }); + await user.click(input); + await user.keyboard("{ArrowLeft}"); + await user.keyboard(" "); + expect(screen.queryByRole("option", { name: "alpha" })).not.toBeInTheDocument(); + expect(input).toHaveValue("alpha"); + }); + }); + + describe("validateOnChange", () => { + const validateOnChange = (v: string) => + v.startsWith("x") ? "Must not start with x" : undefined; + + it("marks an invalid entry immediately after adding", () => { + installerRender(); + expect(screen.getByRole("option", { name: /invalid/ })).toBeInTheDocument(); + }); + + it("shows the error block when there are invalid entries", () => { + installerRender(); + screen.getByText(/Select entries to edit or remove them/); + }); + + it("removes all invalid entries when clicking the clear button", async () => { + const { user } = installerRender( + , + ); + await user.click(screen.getByRole("button", { name: /remove all invalid entries/i })); + screen.getByText("ok"); + expect(screen.queryByText("xbad")).not.toBeInTheDocument(); + }); + + it("opens an invalid entry for editing on Delete instead of removing it", async () => { + const { user } = installerRender( + , + ); + const input = screen.getByRole("textbox", { name: "Tags" }); + await user.click(input); + await user.keyboard("{ArrowLeft}{Delete}"); + expect(screen.queryByRole("option", { name: "xbad" })).not.toBeInTheDocument(); + expect(input).toHaveValue("xbad"); + }); + }); + + describe("validateOnSubmit", () => { + const validateOnSubmit = (v: string) => + v.startsWith("x") ? "Must not start with x" : undefined; + + it("does not mark entries as invalid before submitting", () => { + installerRender(); + expect(screen.queryByRole("option", { name: /invalid/ })).not.toBeInTheDocument(); + }); + + it("marks entries as invalid after a failed submit", async () => { + const { user } = installerRender( + , + ); + await user.click(screen.getByRole("button", { name: "Submit" })); + await screen.findByRole("option", { name: /invalid/ }); + }); + + it("shows the error block after a failed submit", async () => { + const { user } = installerRender( + , + ); + await user.click(screen.getByRole("button", { name: "Submit" })); + await screen.findByText(/Select entries to edit or remove them/); + }); + + it("removes all invalid entries after a failed submit", async () => { + const { user } = installerRender( + , + ); + await user.click(screen.getByRole("button", { name: "Submit" })); + await user.click(await screen.findByRole("button", { name: /remove all invalid entries/i })); + screen.getByText("ok"); + expect(screen.queryByText("xbad")).not.toBeInTheDocument(); + }); + }); + + describe("helperText", () => { + it("does not show helper text when there are no errors", () => { + installerRender(); + expect(screen.queryByText(/Some hint/)).not.toBeInTheDocument(); + }); + + it("shows helper text alongside the error block when there are invalid entries", () => { + const validateOnChange = (v: string) => (v === "bad" ? "Invalid" : undefined); + installerRender( + , + ); + screen.getByText(/Some hint/); + }); + }); + + describe("skipDuplicates", () => { + it("does not add an entry already in the list", async () => { + const { user } = installerRender(); + await user.type(screen.getByRole("textbox", { name: "Tags" }), "alpha"); + await user.keyboard("{Enter}"); + expect(screen.getAllByText("alpha")).toHaveLength(1); + }); + + it("skips duplicates when pasting", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.paste("alpha beta"); + expect(screen.getAllByRole("option")).toHaveLength(2); + screen.getByText("beta"); + }); + }); + + describe("paste", () => { + it("adds multiple entries from a paste", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.paste("alpha beta gamma"); + screen.getByText("alpha"); + screen.getByText("beta"); + screen.getByText("gamma"); + }); + + it("does not intercept a single-token paste", async () => { + const { user } = installerRender(); + const input = screen.getByRole("textbox", { name: "Tags" }); + await user.click(input); + await user.paste("alpha"); + expect(input).toHaveValue("alpha"); + expect(screen.queryByRole("option", { name: "alpha" })).not.toBeInTheDocument(); + }); + }); + + describe("keyboard navigation", () => { + it("does not activate navigation on Backspace when the draft is not empty", async () => { + const { user } = installerRender(); + await user.type(screen.getByRole("textbox", { name: "Tags" }), "partial"); + await user.keyboard("{Backspace}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "false", + ); + }); + + it("does not activate navigation on ArrowLeft when the draft is not empty", async () => { + const { user } = installerRender(); + await user.type(screen.getByRole("textbox", { name: "Tags" }), "partial"); + await user.keyboard("{ArrowLeft}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "false", + ); + }); + + it("activates the last entry on Backspace when the draft is empty", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "true", + ); + }); + + it("activates the last entry on ArrowLeft when the draft is empty", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{ArrowLeft}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "true", + ); + }); + + it("removes the active entry on Delete", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{ArrowLeft}{Delete}"); + expect(screen.queryByText("alpha")).not.toBeInTheDocument(); + }); + + it("removes the active entry on Backspace", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{ArrowLeft}{Backspace}"); + expect(screen.queryByText("alpha")).not.toBeInTheDocument(); + }); + + it("moves to the previous entry on ArrowLeft", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}"); + expect(screen.getByRole("option", { name: "beta" })).toHaveAttribute("aria-selected", "true"); + await user.keyboard("{ArrowLeft}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "true", + ); + }); + + it("moves to the previous entry on ArrowUp", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}{ArrowUp}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "true", + ); + }); + + it("moves to the next entry on ArrowRight", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}{ArrowLeft}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "true", + ); + await user.keyboard("{ArrowRight}"); + expect(screen.getByRole("option", { name: "beta" })).toHaveAttribute("aria-selected", "true"); + }); + + it("moves to the next entry on ArrowDown", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}{ArrowLeft}{ArrowDown}"); + expect(screen.getByRole("option", { name: "beta" })).toHaveAttribute("aria-selected", "true"); + }); + + it("deactivates navigation on ArrowRight at the last entry", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}{ArrowRight}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "false", + ); + }); + + it("jumps to the first entry on Home", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}{Home}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "true", + ); + }); + + it("jumps to the last entry on End", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{ArrowLeft}{Home}{End}"); + expect(screen.getByRole("option", { name: "beta" })).toHaveAttribute("aria-selected", "true"); + }); + + it("deactivates navigation on Escape", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}{Escape}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "false", + ); + }); + + it("deactivates navigation on Tab", async () => { + const { user } = installerRender(); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.keyboard("{Backspace}"); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "true", + ); + await user.tab(); + expect(screen.getByRole("option", { name: "alpha" })).toHaveAttribute( + "aria-selected", + "false", + ); + }); + }); +}); diff --git a/web/src/components/form/ArrayField.tsx b/web/src/components/form/ArrayField.tsx new file mode 100644 index 0000000000..528bbaf9de --- /dev/null +++ b/web/src/components/form/ArrayField.tsx @@ -0,0 +1,636 @@ +/* + * 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, { useState, useRef } from "react"; +import { fork, sift, unique } from "radashi"; +import { sprintf } from "sprintf-js"; +import { + FormGroup, + Label, + TextInputGroup, + TextInputGroupMain, + FormHelperText, + HelperText, + HelperTextItem, + Button, +} from "@patternfly/react-core"; +import { useFieldContext } from "~/hooks/form-contexts"; +import { _ } from "~/i18n"; + +/** + * Keys owned by the entry navigation handler when an entry is active. + * + * Space is included alongside Enter to match the ARIA listbox pattern + * (https://www.w3.org/WAI/ARIA/apg/patterns/listbox/), where both keys + * activate the focused option. Any key outside this set exits navigation + * without consuming the event, so Tab moves focus away and regular characters + * land in the draft input normally. + */ +const NAVIGATION_KEYS = new Set([ + " ", + "ArrowLeft", + "ArrowUp", + "ArrowRight", + "ArrowDown", + "Home", + "End", + "Enter", + "Delete", + "Backspace", +]); + +/** Applies `normalize` to `value` if provided; otherwise returns `value` unchanged. */ +function normalizeValue(value: string, normalize?: (v: string) => string): string { + return normalize ? normalize(value) : value; +} + +/** + * Trims, normalizes, and optionally validates a raw draft string. + * + * Returns `null` for empty or whitespace-only input so callers can skip + * adding an empty entry. Otherwise returns the normalized value and any + * validation error. + */ +function processDraft( + raw: string, + normalize?: (v: string) => string, + validate?: (v: string) => string | undefined, +): { normalized: string; error: string | undefined } | null { + const trimmed = raw.trim(); + if (!trimmed) return null; + const normalized = normalizeValue(trimmed, normalize); + return { normalized, error: validate?.(normalized) }; +} + +/** Builds the screen-reader announcement for a multi-entry paste. Pure function. */ +function pasteAnnouncement( + added: number, + skipped: number, + valid: string[], + invalid: string[], +): string { + if (added === 0) return sprintf(_("%d duplicates skipped."), skipped); + if (skipped === 0) + return invalid.length === 0 + ? sprintf(_("%d entries added."), valid.length) + : sprintf(_("%d entries added, %d invalid."), added, invalid.length); + return invalid.length === 0 + ? sprintf(_("%d entries added, %d duplicates skipped."), valid.length, skipped) + : sprintf( + _("%d entries added, %d invalid, %d duplicates skipped."), + added, + invalid.length, + skipped, + ); +} + +/** Splits pasted text on whitespace and commas, returning non-empty entries. */ +function parsePasteEntries(text: string): string[] { + return sift(text.split(/[\s,]+/).map((t) => t.trim())); +} + +/** + * Returns entries from `normalized` not already in `existing`, + * also deduplicating within `normalized` itself. + * + * Prepends `existing` before deduplication so `unique` sees existing entries + * first and drops any later occurrence of the same value. Slicing off the + * first `existing.length` elements then yields only the genuinely new entries. + */ +function filterNew(existing: string[], normalized: string[]): string[] { + return unique([...existing, ...normalized]).slice(existing.length); +} + +type EntryProps = { + /** Raw stored value, not necessarily the display form. */ + item: string; + index: number; + /** Whether this entry is currently focused during keyboard navigation. */ + isActive: boolean; + /** Validation error message; undefined means the entry is valid. */ + error?: string; + /** Formats the raw value for display and aria labels. */ + toLabel: (v: string) => string; + onEdit: (index: number) => void; + onRemove: (index: number) => void; + /** Returns a stable DOM id used for aria-activedescendant. */ + valueId: (index: number) => string; +}; + +/** + * A single committed entry, rendered as a listbox option. + * + * Both the visual color and the aria-label carry validation state, so + * sighted and assistive-technology users receive the same information. + */ +function Entry({ item, index, isActive, error, toLabel, onEdit, onRemove, valueId }: EntryProps) { + // preventDefault keeps focus on the input; the edit moves the value back to draft. + const handleMouseDown = (e: React.MouseEvent) => { + e.preventDefault(); + onEdit(index); + }; + + // preventDefault avoids blur; stopPropagation prevents the span from triggering edit. + const handleCloseMouseDown = (e: React.MouseEvent) => { + e.preventDefault(); + e.stopPropagation(); + }; + + const handleRemove = (e: React.MouseEvent) => { + e.stopPropagation(); + onRemove(index); + }; + + const labelText = toLabel(item); + + return ( + + + + ); +} + +type MultiValueFieldProps = { + /** + * Label rendered by PatternFly's FormGroup. + * + * Can be a plain string or a ReactNode (e.g. `LabelText` with a suffix). + * When a ReactNode is passed, also provide `inputAriaLabel` so assistive + * technologies receive a plain-text version of the label. + */ + label: React.ReactNode; + + /** + * Plain-text label for assistive technologies. + * + * Used as the accessible name of the text input and as the base for the + * listbox accessible name. Inferred from `label` when it is a plain string; + * required when `label` is a ReactNode. + */ + inputAriaLabel?: string; + + /** + * Per-entry validator that runs on every commit. + * + * Invalid entries are marked and announced immediately, before the form is + * submitted. Use for format checks that are always safe to run eagerly, + * such as address or URL format. + */ + validateOnChange?: (value: string) => string | undefined; + + /** + * Per-entry validator that runs only after the first failed form submit. + * + * Stays silent until TanStack Form sets a field-level error on this field. + * Use for checks that would be distracting before the user attempts to + * submit, such as cross-field or server-validated rules. + */ + validateOnSubmit?: (value: string) => string | undefined; + + /** + * Normalizes user input before it is committed. + * + * Runs on every added entry, including pasted ones. Use for trimming, + * casing, or any formatting rule applied at commit time. + */ + normalize?: (value: string) => string; + + /** + * Formats a stored value for display and accessible labels. + * + * When omitted, the stored value is used as-is. Useful when the stored + * form differs from the human-readable form, e.g. a code vs. a name. + */ + displayValue?: (value: string) => string; + + /** + * Converts a stored value back to a draft string for editing. + * + * Called when an entry is moved into the text input for modification. + * When omitted, the stored value is used as-is. + */ + toDraft?: (value: string) => string; + + /** + * When true, skips entries that are already in the list. + * + * Applies to both single commits and multi-token pastes. + */ + skipDuplicates?: boolean; + + /** + * Additional guidance shown alongside the error messages. + * + * Only rendered when the field has errors. Use to explain the expected + * format or other context that helps the user fix invalid entries. + */ + helperText?: React.ReactNode; + + /** Disables the text input and all entry interactions. */ + isDisabled?: boolean; +}; + +/** + * A form field for entering and managing a list of string values. + * + * Users type in a text input and commit entries one at a time via Enter, + * Tab, or blur. Each committed entry appears as a labeled token inside the + * field. Entries can be edited by clicking them or selecting them with the + * keyboard, and removed individually via the close button or Backspace/Delete. + * A clear-invalid button removes all invalid entries at once when any are present. + * + * Keyboard navigation follows the ARIA listbox pattern: ArrowLeft/Right/Up/Down + * move between entries, Enter and Space activate the focused one, Home/End jump + * to the first or last, and Escape exits navigation. Pasting a whitespace- or + * comma-separated string adds all tokens at once. + * + * Two validation modes are available: + * - `validateOnChange`: runs on every commit; marks invalid entries right away. + * - `validateOnSubmit`: runs only after TanStack Form sets a field-level error + * (i.e. after the first failed submit attempt); stays silent until then. + * + * Both sighted and assistive-technology users receive equivalent feedback: + * invalid entries carry an aria-label describing the error; add, edit, and + * remove actions are announced via a live region. + * + * Must be used inside a TanStack Form `AppField` context that provides a + * `string[]` field value. + * + * @remarks + * **Keyboard focus model** + * + * This component uses `aria-activedescendant` instead of roving tabIndex. + * + * The MDN guide on keyboard-navigable widgets + * (https://developer.mozilla.org/en-US/docs/Web/Accessibility/Guides/Keyboard-navigable_JavaScript_widgets) + * describes two approaches: roving tabIndex (move DOM focus between elements) + * and `aria-activedescendant` (keep DOM focus on one element and point to the + * active descendant by id). + * + * Roving tabIndex was tried first but did not work reliably here. When an + * `onClick` prop is passed to PatternFly's `Label` component, PF renders the + * label content inside a ` + {"."} + + + )} + + {hasAnyError && helperText && <>{helperText}. } + {_( + "Enter or Tab to add; arrow keys to navigate entries, Escape to exit; Backspace or Delete to remove.", + )} + + + + + ); +} diff --git a/web/src/components/network/ConnectionForm.test.tsx b/web/src/components/network/ConnectionForm.test.tsx index 8484c733c3..91e6aaeb9f 100644 --- a/web/src/components/network/ConnectionForm.test.tsx +++ b/web/src/components/network/ConnectionForm.test.tsx @@ -158,12 +158,18 @@ describe("ConnectionForm", () => { await user.click(screen.getByLabelText("IPv4 Settings")); await user.click(screen.getByRole("option", { name: /^Manual/ })); - await user.type(screen.getByLabelText("IPv4 Addresses"), "192.168.1.100 192.168.1.200/12"); + await user.type( + screen.getByLabelText("IPv4 Addresses"), + "192.168.1.100{Enter}192.168.1.200/12{Enter}", + ); await user.type(screen.getByLabelText("IPv4 Gateway (optional)"), "192.168.1.1"); await user.click(screen.getByLabelText("IPv6 Settings")); await user.click(screen.getByRole("option", { name: /^Manual/ })); - await user.type(screen.getByLabelText("IPv6 Addresses"), "2001:db8::1 2001:db8::2/24"); + await user.type( + screen.getByLabelText("IPv6 Addresses"), + "2001:db8::1{Enter}2001:db8::2/24{Enter}", + ); await user.type(screen.getByLabelText("IPv6 Gateway (optional)"), "::1"); await user.click(screen.getByRole("button", { name: "Accept" })); @@ -196,7 +202,7 @@ describe("ConnectionForm", () => { it("shows the DNS servers field when the checkbox is checked", async () => { const { user } = installerRender(); await user.click(screen.getByLabelText("Use custom DNS")); - screen.getByRole("textbox", { name: "DNS servers" }); + screen.getByLabelText("DNS servers"); }); it("submits with parsed nameservers when checkbox is checked", async () => { @@ -204,8 +210,8 @@ describe("ConnectionForm", () => { await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); await user.click(screen.getByLabelText("Use custom DNS")); await user.type( - screen.getByRole("textbox", { name: "DNS servers" }), - "8.8.8.8 1.1.1.1 2001:db8::1", + screen.getByLabelText("DNS servers"), + "8.8.8.8{Enter}1.1.1.1{Enter}2001:db8::1{Enter}", ); await user.click(screen.getByRole("button", { name: "Accept" })); await waitFor(() => @@ -220,7 +226,7 @@ describe("ConnectionForm", () => { await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); const checkbox = screen.getByRole("checkbox", { name: "Use custom DNS" }); await user.click(checkbox); - await user.type(screen.getByRole("textbox", { name: "DNS servers" }), "8.8.8.8"); + await user.type(screen.getByLabelText("DNS servers"), "8.8.8.8{Enter}"); await user.click(checkbox); expect(checkbox).not.toBeChecked(); await user.click(screen.getByRole("button", { name: "Accept" })); @@ -239,7 +245,7 @@ describe("ConnectionForm", () => { it("shows the DNS search domains field when the checkbox is checked", async () => { const { user } = installerRender(); await user.click(screen.getByLabelText("Use custom DNS search domains")); - screen.getByRole("textbox", { name: "DNS search domains" }); + screen.getByLabelText("DNS search domains"); }); it("submits with parsed dnsSearchList when checkbox is checked", async () => { @@ -247,8 +253,8 @@ describe("ConnectionForm", () => { await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); await user.click(screen.getByLabelText("Use custom DNS search domains")); await user.type( - screen.getByRole("textbox", { name: "DNS search domains" }), - "example.com local.lan", + screen.getByLabelText("DNS search domains"), + "example.com{Enter}local.lan{Enter}", ); await user.click(screen.getByRole("button", { name: "Accept" })); await waitFor(() => @@ -263,7 +269,7 @@ describe("ConnectionForm", () => { await user.type(screen.getByLabelText("Name"), "Testing Connection 1"); const checkbox = screen.getByRole("checkbox", { name: "Use custom DNS search domains" }); await user.click(checkbox); - await user.type(screen.getByRole("textbox", { name: "DNS search domains" }), "example.com"); + await user.type(screen.getByLabelText("DNS search domains"), "example.com{Enter}"); await user.click(checkbox); expect(checkbox).not.toBeChecked(); await user.click(screen.getByRole("button", { name: "Accept" })); diff --git a/web/src/components/network/ConnectionForm.tsx b/web/src/components/network/ConnectionForm.tsx index 1854ac09db..78e5bda7da 100644 --- a/web/src/components/network/ConnectionForm.tsx +++ b/web/src/components/network/ConnectionForm.tsx @@ -33,7 +33,6 @@ import { FormHelperText, HelperText, HelperTextItem, - TextArea, TextInput, } from "@patternfly/react-core"; import Page from "~/components/core/Page"; @@ -47,30 +46,34 @@ import { formOptions } from "@tanstack/react-form"; import { useAppForm, mergeFormDefaults } from "~/hooks/form"; import { useDevices } from "~/hooks/model/system/network"; import { NETWORK } from "~/routes/paths"; -import { buildAddress } from "~/utils/network"; +import { + buildAddress, + isValidIPv4, + isValidIPv6, + isValidIPv4Address, + isValidIPv6Address, + isValidNameserver, + isValidDNSSearchDomain, +} from "~/utils/network"; +import { isEmpty, shake } from "radashi"; import { _ } from "~/i18n"; const IPV4_DEFAULT_PREFIX = 24; const IPV6_DEFAULT_PREFIX = 64; -/** Splits a space/newline separated string into a trimmed, non-empty token array. */ -const parseTokens = (raw: string): string[] => - raw - .split(/[\s\n]+/) - .map((s) => s.trim()) - .filter(Boolean); - -/** 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}`; +/** + * Maps form mode values to their corresponding {@link ConnectionMethod}. + * + * "unset" is intentionally absent: omitting it causes the Connection + * constructor to write no method, delegating the decision to NetworkManager. + * This map can be dropped once the form mode values align with + * {@link ConnectionMethod} enum values. + */ +const MODE_TO_METHOD: Record = { + auto: ConnectionMethod.AUTO, + manual: ConnectionMethod.MANUAL, }; -/** Parses a space/newline separated string of addresses into IPAddress objects. */ -const parseAddresses = (raw: string) => parseTokens(raw).map(withPrefix).map(buildAddress); - /** * Shared form options for ConnectionForm and its `withForm` based * sub-components @@ -84,32 +87,152 @@ export const connectionFormOptions = formOptions({ iface: "", ifaceMac: "", ipv4Mode: "unset", - addresses4: "", + addresses4: [] as string[], gateway4: "", ipv6Mode: "unset", - addresses6: "", + addresses6: [] as string[], gateway6: "", - nameservers: "", - dnsSearchList: "", - useCustomDns: false, - useCustomDnsSearch: false, + nameservers: [] as string[], + dnsSearchList: [] as string[], + customDns: false, + customDnsSearch: false, bindingMode: "none" as ConnectionBindingMode, }, }); +type FormValues = typeof connectionFormOptions.defaultValues; +type FormFieldErrors = Partial>; + /** - * Maps form mode values to their corresponding {@link ConnectionMethod}. + * Returns an error when the given list is active and empty or has invalid entries. + * Returns undefined when inactive or when all entries are valid. + */ +function validateActiveList( + active: boolean, + values: string[], + isValid: (v: string) => boolean, + emptyMsg: string, + invalidMsg: string, +): string | undefined { + if (!active) return undefined; + if (values.length === 0) return emptyMsg; + if (values.some((v) => !isValid(v))) return invalidMsg; +} + +/** + * Returns an error for a gateway value under its protocol mode. * - * "unset" is intentionally absent: omitting it causes the Connection - * constructor to write no method, delegating the decision to NetworkManager. - * This map can be dropped once the form mode values align with - * {@link ConnectionMethod} enum values. + * - `manual`: validates if the gateway is present. + * - `auto`: validates only when there are already valid addresses; an empty + * address list means the gateway will be ignored on submission anyway. */ -const MODE_TO_METHOD: Record = { - auto: ConnectionMethod.AUTO, - manual: ConnectionMethod.MANUAL, +function validateGateway( + mode: string, + gateway: string, + validAddresses: string[], + isValid: (v: string) => boolean, + invalidMsg: string, +): string | undefined { + if (!gateway) return undefined; + if (mode === "manual") return isValid(gateway) ? undefined : invalidMsg; + if (mode === "auto" && validAddresses.length > 0) + return isValid(gateway) ? undefined : invalidMsg; +} + +/** 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}`; }; +/** + * Validates the connection form values. + * + * Returns a map of field errors when validation fails, or undefined when all + * values are valid. Validation is intentionally done here rather than in + * per-field onSubmit validators — see the {@link ConnectionForm} remarks. + */ +function validateConnectionForm(formValues: FormValues): FormFieldErrors | undefined { + const validAddresses4 = formValues.addresses4.filter(isValidIPv4Address); + const validAddresses6 = formValues.addresses6.filter(isValidIPv6Address); + + const fieldErrors = shake({ + name: !formValues.name.trim() ? _("Name is required") : undefined, + addresses4: validateActiveList( + formValues.ipv4Mode === "manual", + formValues.addresses4, + isValidIPv4Address, + _("At least one IPv4 address is required"), + _("Some IPv4 addresses are invalid"), + ), + addresses6: validateActiveList( + formValues.ipv6Mode === "manual", + formValues.addresses6, + isValidIPv6Address, + _("At least one IPv6 address is required"), + _("Some IPv6 addresses are invalid"), + ), + gateway4: validateGateway( + formValues.ipv4Mode, + formValues.gateway4, + validAddresses4, + isValidIPv4, + _("Invalid IPv4 gateway"), + ), + gateway6: validateGateway( + formValues.ipv6Mode, + formValues.gateway6, + validAddresses6, + isValidIPv6, + _("Invalid IPv6 gateway"), + ), + nameservers: validateActiveList( + formValues.customDns, + formValues.nameservers, + isValidNameserver, + _("At least one DNS server is required"), + _("Some DNS server addresses are invalid"), + ), + dnsSearchList: validateActiveList( + formValues.customDnsSearch, + formValues.dnsSearchList, + isValidDNSSearchDomain, + _("At least one DNS search domain is required"), + _("Some DNS search domains are invalid"), + ), + }); + + if (!isEmpty(fieldErrors)) return fieldErrors; +} + +/** + * Builds a {@link Connection} from the validated form values. + */ +function buildConnection(formValues: FormValues): Connection { + const ipv4Addresses = + formValues.ipv4Mode === "manual" || formValues.ipv4Mode === "auto" + ? formValues.addresses4.map(withPrefix).map(buildAddress) + : []; + const ipv6Addresses = + formValues.ipv6Mode === "manual" || formValues.ipv6Mode === "auto" + ? formValues.addresses6.map(withPrefix).map(buildAddress) + : []; + + return new Connection(formValues.name, { + iface: formValues.bindingMode === "iface" ? formValues.iface : "", + macAddress: formValues.bindingMode === "mac" ? formValues.ifaceMac : "", + method4: MODE_TO_METHOD[formValues.ipv4Mode], + gateway4: ipv4Addresses.length > 0 ? formValues.gateway4 : "", + method6: MODE_TO_METHOD[formValues.ipv6Mode], + gateway6: ipv6Addresses.length > 0 ? formValues.gateway6 : "", + addresses: [...ipv4Addresses, ...ipv6Addresses], + nameservers: formValues.customDns ? formValues.nameservers : [], + dnsSearchList: formValues.customDnsSearch ? formValues.dnsSearchList : [], + }); +} + /** * Form for creating a new network connection. * @@ -135,29 +258,12 @@ export default function ConnectionForm() { ifaceMac: devices[0]?.macAddress ?? "", }), validators: { - onSubmitAsync: async ({ value }) => { - const ipv4Addresses = - value.ipv4Mode === "manual" || value.ipv4Mode === "auto" - ? parseAddresses(value.addresses4) - : []; - const ipv6Addresses = - value.ipv6Mode === "manual" || value.ipv6Mode === "auto" - ? parseAddresses(value.addresses6) - : []; + onSubmitAsync: async ({ value: formValues }) => { + const fieldErrors = validateConnectionForm(formValues); + if (fieldErrors) return { fields: fieldErrors }; - const connection = new Connection(value.name, { - iface: value.bindingMode === "iface" ? value.iface : "", - macAddress: value.bindingMode === "mac" ? value.ifaceMac : "", - method4: MODE_TO_METHOD[value.ipv4Mode], - gateway4: ipv4Addresses.length > 0 ? value.gateway4 : "", - method6: MODE_TO_METHOD[value.ipv6Mode], - gateway6: ipv6Addresses.length > 0 ? value.gateway6 : "", - addresses: [...ipv4Addresses, ...ipv6Addresses], - nameservers: value.useCustomDns ? parseTokens(value.nameservers) : [], - dnsSearchList: value.useCustomDnsSearch ? parseTokens(value.dnsSearchList) : [], - }); try { - await updateConnection(connection); + await updateConnection(buildConnection(formValues)); } catch (e) { return { form: e.message }; } @@ -175,6 +281,18 @@ export default function ConnectionForm() {
{ e.preventDefault(); + // Validation is intentionally deferred to submission so users are + // not interrupted while filling the form. All rules live in + // onSubmitAsync rather than per-field onSubmit validators because + // several checks are cross-field (e.g. gateway validity depends on + // the addresses list). TanStack Form only clears field errors set + // by onSubmitAsync when a per-field onSubmit validator runs for + // the same cause — which never happens here — so canSubmit stays + // false after a failed attempt. setErrorMap resets every field's + // errorMap.onSubmit before each new attempt, restoring canSubmit + // so onSubmitAsync is called again. + // @see https://tanstack.com/form/latest/docs/reference/formApi#seterrormap + form.setErrorMap({ onSubmit: { fields: {} } }); form.handleSubmit(); }} > @@ -197,22 +315,33 @@ export default function ConnectionForm() { - {(field) => ( - - field.handleChange(v)} - /> - - )} + {(field) => { + const error = field.state.meta.errors[0] as string | undefined; + return ( + + field.handleChange(v)} + /> + {error && ( + + + {error} + + + )} + + ); + }} - + {(dnsToggle) => ( <> {dnsToggle.state.value && ( - + {(field) => ( - -