Skip to content

fix(platform): unify form validation timing across all forms (#1943)#1970

Open
larryro wants to merge 2 commits into
mainfrom
tale/xs7es5716dnc19tkd133nj3nyn897w9r
Open

fix(platform): unify form validation timing across all forms (#1943)#1970
larryro wants to merge 2 commits into
mainfrom
tale/xs7es5716dnc19tkd133nj3nyn897w9r

Conversation

@larryro

@larryro larryro commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

What & why

Closes #1943. Typing the first character into a field (e.g. "New password") immediately showed a validation error because most forms used mode: 'onChange', while others used no mode at all (react-hook-form's 'onSubmit' default) — validation timing was inconsistent app-wide.

Changes

  • Shared wrapper — new app/components/ui/forms/use-form.ts re-exports useForm with mode: 'onTouched' as the default (validate after the first blur, then re-validate on change). A caller can still override mode explicitly.
  • Repointed every form — all direct react-hook-form useForm call sites and useFormEditor now use the wrapper; the ad-hoc mode: 'onChange' overrides are removed.
  • Regression guard — an oxlint no-restricted-imports rule blocks importing useForm straight from react-hook-form (with an exception for the wrapper itself), so future forms inherit the behaviour and can't regress.
  • Tests — component tests that asserted on-keystroke errors now blur first; added a unit test for the wrapper covering the no-error-on-first-keystroke behaviour.

Acceptance criteria

  • No field shows a validation error on its first keystroke before blur/submit.
  • Behaviour is identical across settings, dialogs, onboarding, chat, and other platform forms (single shared default).
  • A grep shows no remaining ad-hoc mode: 'onChange'; the lint rule prevents new ones.

Verification

  • bunx oxlint (platform) — clean
  • bunx tsc --noEmit (platform) — clean
  • bunx vitest --run --config vitest.ui.config.ts (platform UI suite, 337 files) — green
  • oxfmt --check — clean

Docs/i18n: N/A — no user-facing strings, message keys, or error wording changed; only validation timing.

Forms validated on every keystroke (mode: 'onChange'), so a field showed
an error on its first character before the user finished typing. Others
used no mode (RHF's 'onSubmit' default), so timing was inconsistent
app-wide.

- Add a shared useForm wrapper (app/components/ui/forms/use-form.ts) that
  defaults mode to 'onTouched': validate after the first blur, then
  re-validate on change. Callers can still override mode explicitly.
- Repoint every direct react-hook-form useForm call and useFormEditor to
  the wrapper and drop the ad-hoc mode: 'onChange' overrides.
- Add an oxlint no-restricted-imports guard so future forms can't import
  useForm straight from react-hook-form and regress the timing.
- Update component tests that asserted on-keystroke errors to blur first,
  and add a wrapper unit test covering the no-error-on-first-keystroke
  behaviour.

Docs/i18n: N/A — no user-facing strings, message keys, or error wording
changed; only validation timing.
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@larryro, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 27 minutes and 38 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 523f42d2-0c7e-474e-bfce-46cb3bc9eddc

📥 Commits

Reviewing files that changed from the base of the PR and between b418075 and 67abd5b.

📒 Files selected for processing (46)
  • services/platform/.oxlintrc.json
  • services/platform/app/components/ui/editor/use-form-editor.ts
  • services/platform/app/components/ui/forms/input.test.tsx
  • services/platform/app/components/ui/forms/use-form.test.tsx
  • services/platform/app/components/ui/forms/use-form.ts
  • services/platform/app/features/agents/components/agent-create-dialog.test.tsx
  • services/platform/app/features/agents/components/agent-create-dialog.tsx
  • services/platform/app/features/automations/components/automation-create-dialog.tsx
  • services/platform/app/features/automations/components/automation-rename-dialog.tsx
  • services/platform/app/features/automations/components/step-create-dialog.tsx
  • services/platform/app/features/automations/triggers/components/schedule-create-dialog.tsx
  • services/platform/app/features/customers/components/customer-edit-dialog.tsx
  • services/platform/app/features/customers/components/customers-import-dialog.tsx
  • services/platform/app/features/documents/components/create-folder-dialog.tsx
  • services/platform/app/features/knowledge-entries/components/knowledge-entry-add-dialog.tsx
  • services/platform/app/features/knowledge-entries/components/knowledge-entry-edit-dialog.tsx
  • services/platform/app/features/organization/components/onboarding/steps/account-step.tsx
  • services/platform/app/features/products/components/product-create-dialog.tsx
  • services/platform/app/features/products/components/product-edit-dialog.tsx
  • services/platform/app/features/products/components/products-import-dialog.tsx
  • services/platform/app/features/projects/components/project-create-dialog.tsx
  • services/platform/app/features/projects/components/project-rename-dialog.tsx
  • services/platform/app/features/settings/account/components/account-form.tsx
  • services/platform/app/features/settings/api-keys/components/api-key-create-dialog.tsx
  • services/platform/app/features/settings/enterprise-sso/components/enterprise-sso-form.test.tsx
  • services/platform/app/features/settings/governance/data-subject-requests/cancel-dialog.tsx
  • services/platform/app/features/settings/governance/data-subject-requests/extend-deadline-dialog.tsx
  • services/platform/app/features/settings/governance/data-subject-requests/file-request-dialog.tsx
  • services/platform/app/features/settings/governance/legal-hold/close-matter-dialog.tsx
  • services/platform/app/features/settings/governance/legal-hold/place-hold-dialog.tsx
  • services/platform/app/features/settings/governance/legal-hold/reject-release-dialog.tsx
  • services/platform/app/features/settings/governance/legal-hold/request-release-dialog.tsx
  • services/platform/app/features/settings/governance/legal-hold/upsert-matter-dialog.tsx
  • services/platform/app/features/settings/organization/components/member-add-dialog.tsx
  • services/platform/app/features/settings/organization/components/member-edit-dialog.tsx
  • services/platform/app/features/settings/providers/components/provider-add-panel.tsx
  • services/platform/app/features/settings/teams/components/team-create-dialog.test.tsx
  • services/platform/app/features/settings/teams/components/team-create-dialog.tsx
  • services/platform/app/features/settings/teams/components/team-edit-dialog.tsx
  • services/platform/app/features/vendors/components/vendor-edit-dialog.tsx
  • services/platform/app/features/vendors/components/vendors-import-dialog.tsx
  • services/platform/app/features/websites/components/website-add-dialog.tsx
  • services/platform/app/features/websites/components/website-edit-dialog.tsx
  • services/platform/app/routes/_auth/log-in.tsx
  • services/platform/app/routes/dashboard/$id/automations/$amId/configuration.tsx
  • services/platform/app/routes/forced-change-password.$id.tsx
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tale/xs7es5716dnc19tkd133nj3nyn897w9r

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@larryro

larryro commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Desk Review — #1943 (unify form validation timing → mode: 'onTouched')

Verdict: READY TO MERGE. The fix correctly resolves the reported bug, the migration is complete, regression is well-guarded, and all checks are green.

What was verified

Correctness (the load-bearing concern). Many migrated forms gate their submit/Continue/Save button on formState.isValid and previously relied on mode: 'onChange' so isValid updates while typing. I confirmed in the react-hook-form 7.72.1 source (dist/index.esm.mjs, the shouldSkipValidation branch) that under mode: 'onTouched', a change event still calls _setValid() whenever isValid is subscribed — so button gating stays live per keystroke; only inline error display is deferred to first blur. That is exactly the intended behavior and the bug fix. useFormEditor (which dropped mode entirely) is unaffected: its isValid/isDirty/isSubmitting gating is mode-independent and doSave validates via handleSubmit.

Migration completeness. No direct useForm import from react-hook-form remains in services/platform (only useFormContext/Controller/useFieldArray/FormProvider, which are fine). No remaining ad-hoc mode: 'onChange' on any RHF form. The no-restricted-imports oxlint rule correctly forbids the direct import and exempts only the wrapper (app/components/ui/forms/use-form.ts), preventing regressions. The wrapper's { mode: 'onTouched', ...props } correctly lets callers override.

CI: all required checks green — 46 pass, 5 conditional skips (fork-PR/docs-container variants), 0 failing, 0 pending. (Lint, Type check, Unit, UI, Storybook, 16× Playwright shards all pass.)

Tests run locally: bunx vitest --run --config vitest.ui.config.ts337 files / 2514 tests passed. Targeted form tests (use-form, input, team-create-dialog, enterprise-sso-form, agent-create-dialog) → 50 passed. New use-form.test.tsx proves no error on first keystroke; updated integration tests assert the error surfaces after blur and that isValid gating stays accurate.

Non-blocking notes (optional, not required for merge)

  • use-form.test.tsx asserts "no error before blur" but delegates the "error appears after blur" assertion to integration tests (team-create-dialog / enterprise-sso both cover it). Adding a blur→error case to the unit test would make it self-contained.
  • Scope is services/platform (matches the issue's proposed change). Forms in services/web are not covered by the wrapper or the lint rule — fine if intentionally out of scope.

Acceptance criteria all met: no error on first keystroke before blur; behavior unified across dialogs/settings/onboarding; no remaining ad-hoc mode: 'onChange'; shared wrapper + lint rule prevent future drift.

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.

Bug: Form validation fires on first keystroke — unify validation timing across all forms

1 participant