Skip to content

refactor(ui): adopt react-hook-form for account & settings forms#6618

Closed
luannmoreira wants to merge 2 commits into
masterfrom
refactor/rhf-account-settings-forms
Closed

refactor(ui): adopt react-hook-form for account & settings forms#6618
luannmoreira wants to merge 2 commits into
masterfrom
refactor/rhf-account-settings-forms

Conversation

@luannmoreira

Copy link
Copy Markdown
Member

What

Migrates the Account & settings form group to react-hook-form: the Profile
edit and change-password drawers, the Settings rename-namespace drawer, and the
admin SAML configuration drawer. Next step of the incremental migration tracked
in shellhub-io/team#152, following the SignUp and auth-form migrations.

Why

Each of these forms hand-threaded useState + onChange + per-field error
computation. Reusing the established RHF field wrappers and resolver-adapter
pattern removes that boilerplate and gives the SAML drawer real validation
structure and its first tests.

Changes

  • fields/rhf/FormTextareaField: new useController-bound wrapper over a raw
    <textarea> (INPUT styling + className override, aria-invalid /
    aria-describedby), for the SAML certificate field. Joins the existing three
    wrappers.
  • Profile: EditProfileDrawer and ChangePasswordDrawer moved to useForm
    with per-form resolvers (editProfileResolver, changePasswordResolver)
    reusing the existing validators (relocated into pages/profile/validate.ts
    unchanged, preserving messages). Recovery-email remains a cross-field rule —
    changing the primary email re-triggers it via trigger("recoveryEmail"). The
    profile payload is still built from changed fields only (dirtyFields).
  • Settings: EditNameDrawer bound via useController over the existing
    NamespaceNameField, delegating to validateNamespaceName (wrapped ?? true
    so a valid name is not treated as invalid).
  • SAML: SamlConfigDrawer moved to useForm with samlResolver replicating
    the metadata-URL-vs-manual conditional rules; useWatch drives the mode UI and
    buildBody was refactored to a pure function of the form values.
  • Validation UX preserved: errors surface only once a field diverges from its
    saved value; empty required fields gate the submit button rather than showing an
    error. Form-level server errors use RHF root errors; drawer-local UI state
    (SAML showAdvanced, change-password success) resets on open via
    useResetOnOpen.
  • Out of scope (not validated forms, untouched): the Settings session-recording
    and device-auto-accept toggles, the delete-confirm and leave dialogs, and the
    Profile delete-account dialogs.

Testing

vitest in the ui container — full console suite green (2701 tests). New
coverage: FormTextareaField, the three resolvers, and the previously-untested
SamlConfigDrawer (metadata to manual toggle, per-mode required-field gating, and
the submit payload for both modes). Worth probing: the recovery-email cross-field
path (change only the primary email and confirm the recovery-email error appears)
and that prefilled edit drawers show no error until a field is changed.

Continues the incremental react-hook-form migration (shellhub-io/team#152)
after the SignUp pilot, converting the account & settings forms: the Profile
edit and change-password drawers, the Settings rename-namespace drawer, and
the admin SAML configuration drawer.

Each form reuses the shared RHF field wrappers and a per-form resolver adapter
over the existing validators. Adds a FormTextareaField wrapper for the SAML
certificate field. Validation UX is preserved (errors surface only once a
field diverges from its saved value), including the recovery-email cross-field
rule, the changed-fields-only profile payload, and the SAML metadata/manual
mode semantics. Form-level server errors now use RHF root errors, and
reset-on-open keeps drawer-local UI state in sync via useResetOnOpen.

The previously untested SAML drawer gains coverage for its mode toggle,
per-mode required-field gating, and submit payloads.
@luannmoreira luannmoreira requested a review from a team as a code owner July 7, 2026 17:45
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Claude finished @luannmoreira's task in 7m 59s —— View job


Code Review Complete

Reviewed 17 files across code quality, security, testing, language patterns, and architecture. 1 inline issue posted (1 high).

The migration itself is clean: buildBody for SAML is byte-for-byte identical to the old request shape, the dirtyFields-based profile payload correctly reproduces the old per-field diffing, ChangePasswordDrawer and EditNameDrawer preserve their required-field gating, and the resolver adapters return the correct { values, errors } shape. No security regressions (SAML cert/metadata handling and password flow are unchanged; textarea binds through React with no raw HTML).

The one real finding is a validation regression in editProfileResolver where clearing a required field (Name/Username/Email) leaves the Save button enabled — see the inline comment for details and two fix options.


If you've addressed the feedback and want a new review, tag @shellhub-io/admin and a team member can trigger it.

Comment thread ui/apps/console/src/pages/profile/editProfileResolver.ts Outdated
Validate name/username/email unconditionally in editProfileResolver so a
cleared required field surfaces its "… is required" error and blocks Save,
instead of silently submitting a blank value. recoveryEmail stays optional.

Also drop an unnecessary type assertion in Profile.test.tsx that failed the
console lint step.
@luannmoreira

Copy link
Copy Markdown
Member Author

Superseded by #6619, which reorganizes this work into the umbrella react-hook-form migration PR (team#152) with one commit per topic (Profile, Settings, SAML config). Same diff; continuing there.

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.

1 participant