Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: Improve form handling #1963

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

ui: Improve form handling #1963

wants to merge 5 commits into from

Conversation

Etsija
Copy link
Contributor

@Etsija Etsija commented Feb 6, 2025

This PR gets rid of most React warnings about form components transferring from uncontrolled to controlled state by:

  1. Providing default values for mandatory form fields.
  2. Handling optional fields in a special way, also allowing proper Zod validation for optional fields, whenever user inputs something to them.

As an example of handling optional fields, the user creation form is modified accordingly. If this approach is acceptable, other forms would be fixed in a follow-up PR.

This is a more proper alternative to #1946.

Please see the commits for details.

@Etsija Etsija force-pushed the improve-form-handling branch 3 times, most recently from 03ec527 to ae88545 Compare February 7, 2025 07:12
@Etsija Etsija marked this pull request as ready for review February 7, 2025 09:50
@Etsija Etsija requested review from lamppu and mmurto as code owners February 7, 2025 09:50
#1940 introduced a component for inputting "password" fiels, so use it
also for creating a new user.

Signed-off-by: Jyrki Keisala <jyrki.keisala@doubleopen.org>
When using controlled forms, default values must be provided to 
non-optional form fields [1], to prevent the React warning (seen in the 
dev. console of the browser):

"Warning: A component is changing an uncontrolled input to be controlled. 
This is likely caused by the value changing from undefined to a defined 
value, which should not happen."

[1]: https://stackoverflow.com/a/77202565

Signed-off-by: Jyrki Keisala <jyrki.keisala@doubleopen.org>
Define a function which makes it possible to add refining schema
validations to optional parameters [1].

[1]: shadcn-ui/ui#690 (comment)

Signed-off-by: Jyrki Keisala <jyrki.keisala@doubleopen.org>
Optional inputs in controlled forms are problematic, because 
(1) Specifying default values for them in the `useForm` hook leads to
    unnecessary payload sent from the form, so rather keep them as
    `undefined`, which means the corresponding parameter is not 
    included in the payload.
(2) Leaving them without default values causes a React warning about a
    component transferring from uncontrolled to controlled state.

Alleviate these problems with a component specifically used for optional 
inputs [1].

[1]: shadcn-ui/ui#410 (comment)
As an example of using `asOptionalField` utility function and 
`OptionalInput` component when handling optional inputs in forms, use
them in the Admin section's user creation form. This form has quite many
optional input fields, which, when left empty, are not included in the 
form payload. When they are non-empty, they are validated with Zod rules
(for example, "email" field).

Signed-off-by: Jyrki Keisala <jyrki.keisala@doubleopen.org>
@Etsija Etsija force-pushed the improve-form-handling branch from ae88545 to 26dfd21 Compare February 10, 2025 05:06
Copy link
Contributor

@mnonnenmacher mnonnenmacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but I would appreciate if @lamppu could also have a look as my React knowledge is limited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants