refactor(web): reimplement network connection form using TanStack Form (part 1)#3300
Conversation
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
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.
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).
Gateway fields are only shown when the corresponding method is manual, which might suggest they are always required when visible. However, according to the NetworkManager documentation, the gateway is not mandatory even in manual mode: it is only meaningful when addresses are set, and can be replaced by an explicit default route. Following the conventions in src/components/form/conventions.md, fields that are conditionally shown but can still be left blank should carry the (optional) suffix to make that clear to the user. See: https://networkmanager.dev/docs/api/latest/settings-ip4-config.html
Introduces a reusable component for rendering form field labels with an optional suffix styled to be visually dimmer than the main label text. This makes the suffix clearly secondary for sighted users while keeping the full string accessible to screen readers as a single label.
Introduces a "Use custom DNS servers" checkbox that reveals a textarea for entering nameservers. When unchecked, an empty array is submitted. When checked, the field is shown and its value is submitted. The field value is preserved in form state when the checkbox is unchecked, so re-checking restores what the user previously typed. Validation will be added in a later commit. This establishes the checkbox opt-in as a pattern for advanced or rarely needed fields, documented in src/components/form/conventions.md.
Formerly named FormLabel but renamed in this commit to LabelText because it is a more accurate name due the fact that the component renders text content passed to a FormGroup label prop, not a label element itself. Adds a hidden prop that visually hides the text content while keeping it in the DOM. This is used for fields that have no visible label due to their context, such as the DNS textarea inside checkbox body. Keeping real DOM text is preferred over aria-label because it is able to be translated by browser tools and does not depend on ARIA support.
Adds a "Use custom DNS search domains" checkbox that reveals a textarea for entering search domains, following the same checkbox opt-in pattern as DNS servers.
Reverts the use of LabelText hidden for the DNS servers and DNS search domains textareas. PatternFly's FormGroup reserves visual space for the label area whenever a label prop is provided, even when its content is visually hidden, leaving an unwanted gap in the layout. Working around that with CSS overrides would be hacky and fragile for little practical benefit in an application that manages its own translations via _(). aria-label is used instead. It is widely supported and works correctly in this context.
| /** Splits a space/newline separated string into a trimmed, non-empty token array. */ | ||
| const parseTokens = (raw: string): string[] => | ||
| raw | ||
| .split(/[\s\n]+/) |
There was a problem hiding this comment.
\s should match \n, not? https://www.w3schools.com/jsref/jsref_regexp_whitespace.asp
There was a problem hiding this comment.
I have tested it. You're right. If you don't mind, I will drop the \n in following PRs
| if (address.includes("/")) return address; | ||
| return address.includes(":") | ||
| ? `${address}/${IPV6_DEFAULT_PREFIX}` | ||
| : `${address}/${IPV4_DEFAULT_PREFIX}`; |
There was a problem hiding this comment.
to be honest, if we need it, I would prefer to see such defaults and manipulation rather on backend than on js form file.
There was a problem hiding this comment.
I have no strong opinion. I can only say two things to that regard
- this is temporary here for reasons already explained in the description, but would be moved to a more appropriated file.
- we cannot force users to enter the IP prefix as we're currently doing. It make no sense, Agama should fallback to the default one if none is given. What is more, the "prefix" dedicated textinput has been dropped and will not be part of the user interface.
| return <LabelText suffix={_("(IPv4 required)")}>{_("IP Addresses")}</LabelText>; | ||
| if (manual6) | ||
| return <LabelText suffix={_("(IPv6 required)")}>{_("IP Addresses")}</LabelText>; | ||
| return <LabelText suffix={_("(optional)")}>{_("IP Addresses")}</LabelText>; |
There was a problem hiding this comment.
hmm, so here it is only about suffix, so why not define suffix here and then just have return <LabelText suffix={suffix}>{_("IP Addresses")}</LabelText>;
It was kind of hard to review if all texts are same or not.
There was a problem hiding this comment.
Fine with me. Noted for follow up PRs.
The Checkbox body prop renders its content inside a span element, which is invalid HTML for block content like a FormGroup with a TextArea. Replaces it with a NestedContent sibling rendered conditionally when the checkbox is checked, which is both semantically correct and consistent with how nested content is handled elsewhere in the application.
|
LGTM specially taking into account that this is the first PR for the re-implementation, great work! |
…s (part 2) (#3338) **TL;DR:** Refine connection form for cleaner device and IP settings interface, with clearer option labels and a more logical field order. --- This PR continues the reimplementation of the network connection form, built on top of the TanStack Form foundation introduced in part 1. It focuses on two areas: refining the IP settings and device binding UX, and improving the form architecture to align with [TanStack Form's composition](https://tanstack.com/form/latest/docs/framework/react/guides/form-composition) model. ## What is included ### IP settings redesign The IP settings block is extracted into a standalone `IpSettings` component, rendered once per protocol. The _settings_ selector offers three options: _Automatic_, _Manual_, and _Advanced_. _Manual_ and _Advanced_ reveal address and gateway fields while _Automatic_ keeps the form clean. Option descriptions tries to use no user-targeting language as well as no implementation-detail jargon. ### Device binding redesign The interface binding section is extracted into `BindingModeSelector` and `DeviceSelector` components. Options use plain language (_Any_, _Chosen by name_, _Chosen by MAC_) rather than technical terms as in the current (to be drop) `BindingSettingsForm`. DeviceSelector's label is visually hidden since the binding selector already provides visual context. ### TanStack Form composition These components were first implemented making use of useFormContext(), which is deliberately untyped since it is designed for generic leaf field components where the form type is not known at definition time. But later rewritten to use `withForm()` instead, since it gives each component a typed form instance, removes all "as any" casts, and catches field name errors at compile time. ## What is deferred to follow-up PRs Same as part 1, still pending: - Field validation (client-side). - Enhanced multi-value input for addresses, DNS servers, and DNS search domains. - Auto-generated connection name. - ~~Type-specific subforms (bond, bridge, VLAN).~~ Actually out of the scope of this serie of PRs - Edition mode. - Addressing comments #3300 (comment) --- Related to https://trello.com/c/rUEeqOkf (protected link)
Prepare to release version 20. * #3294 * #3295 * #3296 * #3297 * #3298 * #3299 * #3300 * #3301 * #3302 * #3303 * #3304 * #3305 * #3306 * #3307 * #3308 * #3309 * #3310 * #3311 * #3312 * #3313 * #3315 * #3316 * #3317 * #3318 * #3319 * #3320 * #3322 * #3323 * #3325 * #3326 * #3327 * #3329 * #3330 * #3331 * #3333 * #3334 * #3336 * #3338 * #3339 * #3342 * #3343 * #3349 * #3351 * #3352 * #3353 * #3354 * #3356 * #3357 * #3358 * #3359 * #3360 * #3361 * #3362 * #3363 * #3364 * #3365 * #3366 * #3367 * #3368 * #3371 * #3372 * #3373 * #3375 * #3376 * #3378 * #3379 * #3380 * #3381 * #3382 * #3385 * #3386
This PR 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.
What is included
In order to avoid an endless PR and make it easier to follow for review, the whole reimplementation will be addressed in multiple PRs against a feature branch. This first PR covers the following fields:
One example of the form in action. Here, the user selected "Manual" for the IPv4 method, which reveals the IPv4 Gateway field as optional and mark IP addresses as expecting at least one IPv4. The form has several other states worth exploring it by yourself. The easiest way is to check out the branch and play with it.
Keep in mind this is an intermediate step. Some things are intentionally simplified for now, such as field order (Interface is expected to come before Name, still under discussion) and the textarea inputs that will eventually
become a more fluid multi-value experience.
For the reasoning behind field visibility and label choices, see conventions.md.
Additionally, two files have been included as part of this PR:
src/components/form/LabelText: a reusable component that renders text content for a FormGroup label prop, with an optional styled suffix (e.g. "(optional)") and a hidden prop for fields that need an accessible label without visible text.src/components/form/conventions.md: a set of form conventions documented as patterns arose, covering field visibility patterns, label suffixes, and accessibility guidance. It should be moved elsewhere after reaching the master branch, but kept in the newly created form namespace for now. The idea is to keep it up to date as the project evolves and form patterns that suit it well arise.What is deferred to follow-up PRs
onSubmitAsync. Empty required fields, invalid addresses, and required DNS servers when checked will be added incrementally.useAppFormand shared across forms.Notes for reviewers
The conventions document (
src/components/form/conventions.md) is worth reading first, as it explains the reasoning behind field visibility and label choices, and will apply to all future form work across the application (on agreement, of course).Changelog entry postponed for the final PR from feature branch against master branch.
Related to https://trello.com/c/rUEeqOkf (protected link)