diff --git a/docs/code-style-guide.md b/docs/code-style-guide.md new file mode 100644 index 000000000..9409175a0 --- /dev/null +++ b/docs/code-style-guide.md @@ -0,0 +1,73 @@ +# Code style guide + +Write code so readers can follow its intent from top to bottom. Apply these +conventions to new and changed code alongside the root and package `AGENTS.md` +instructions and the repository's ESLint and Prettier configuration. + +## Keep each step easy to follow + +Use descriptive intermediate variables when nested calls, transformations, or +conditions make a call or return object harder to read. Keep simple expressions +inline; avoid introducing variables that only repeat an existing name. + +```ts +const validationInput = omitDisabledValues(validationValues, disabledFields); +const result = await validation.validateAsync(validationInput); + +// Schema defaults can restore disabled values, so filter the output too. +const submissionValues = omitDisabledValues(result.data ?? {}, disabledFields); + +return { + errors: result.errors ?? {}, + values: submissionValues, +}; +``` + +Move large schemas and conditional configuration out of JSX into named +variables. In tests, separate the operation being tested from its assertion +when combining them would hide the sequence. + +## Order return properties alphabetically + +List properties in returned objects alphabetically so readers can find them +consistently. Match that order in expected result objects where practical. +Compute values before the return when their expressions need explanation. + +Do not reorder spreads or expressions when doing so would change override +precedence or evaluation behavior. Refactor those cases explicitly first. + +## Separate logical stages + +Use blank lines between setup, transformations, validation, and returned +results. Keep closely related statements together. Add a short comment when +the transition or ordering has a non-obvious reason. + +Prefer a direct expression for a simple callback. Use a block when it contains +multiple steps or needs an explanatory comment, following the lint rules for +that file. + +## Preserve useful explanations + +Keep comments that explain intent, constraints, lifecycle ordering, or edge +cases. Update them when behavior changes. Avoid comments that merely restate +the next line of code. + +When relying on a dependency's internal API, explain why it is needed and what +must be checked when upgrading the dependency. Keep that explanation near the +access rather than scattering it across callers. + +Review larger docstrings whenever responsibilities or behavior change. Describe +the current implementation, including important caveats, while preserving +still-relevant context. Do not expand unrelated documentation just because the +file is being edited. + +For public props, use the repository's active-verb JSDoc convention: describe +what the prop does, identify callback triggers, and include useful defaults or +fallbacks. See the root `AGENTS.md` for the full convention. + +## Keep cleanup focused + +Prefer small local improvements over new abstractions for a single expression. +Preserve runtime behavior, public types, and helpful comments during readability +changes. Run focused tests and the checks required by the repository; do not +weaken lint rules or assertions to accommodate a refactor. diff --git a/packages/uniform/README.md b/packages/uniform/README.md index efc363451..918adaa52 100644 --- a/packages/uniform/README.md +++ b/packages/uniform/README.md @@ -1,39 +1,10 @@ # uniform -## Password inputs +React form components backed by React Hook Form, Veto schemas and Pixels UI. -`Input` with `type="password"` masks the value and includes a keyboard-accessible -show/hide button by default. Toggling changes only visibility: the field name, -value, validation, and submission behavior remain the same. Pointer activation -keeps input focus; keyboard users can reach and activate the toggle normally. -Form resets hide the password again. +## Features -```tsx - -``` - -Set `hidePasswordToggle={true}` to keep a password input masked without a toggle. -Custom `endContent` appears alongside the toggle. Disabled inputs disable the -toggle, and `clearable` password inputs offer separate clear and reveal buttons. - -## Sensitive debug values - -Password inputs are automatically redacted as `[REDACTED]` in Uniform's debug -viewer, its JSON copy output, and Uniform's submit debug logs. Showing a password -inside the input does not reveal it in debug output. - -Use `sensitive` on an ordinary `Input` for API tokens and other secrets: - -```tsx - - -``` - -Custom field components can pass `sensitive: true` to `useUniformField`. The -shared hook registers the field path with the form. Nested paths and array items -are supported. Sensitivity is retained for the form's lifetime, including when -an input is unmounted but its value remains in form state. - -Redaction changes diagnostic copies only. Form values, validation, and `onSubmit` -receive the original values. Application callbacks that log submitted data must -handle their own redaction. +- Ready-to-use fields with consistent labels and validation feedback +- Schema validation and dynamic client validation +- Field arrays, value transforms, and reset-aware hooks +- Form debugging with sensitive-value redaction diff --git a/packages/uniform/docs/disabled-fields.md b/packages/uniform/docs/disabled-fields.md new file mode 100644 index 000000000..62c1888e0 --- /dev/null +++ b/packages/uniform/docs/disabled-fields.md @@ -0,0 +1,29 @@ +# Disabled fields + +Uniform omits disabled field values before schema validation and submission. +The stored value is retained and restored when the field is re-enabled. + +The Veto schema remains authoritative: a required disabled field produces a +missing-field error and blocks submission. Mark a field optional when its value +may be omitted: + +```tsx +const validation = veto({ accountCode: string().optional() }); + +
; +``` + +This applies to nested fields and client validation too. Schema defaults and +cross-field rules still run normally; disabling a field does not relax them. +Disabled values are also removed from validated output, so schema defaults +cannot add them to the submitted payload. + +For flat arrays, omitted entries keep their indices during validation. Submission +normalization turns these empty slots into `null` placeholders; later entries +keep their positions. + +Previously, Uniform validated stored disabled values before removing them from +submission. Required schemas must now accept omission, or the field must remain +enabled. Re-enabling restores validation of its retained value. diff --git a/packages/uniform/docs/password-fields.md b/packages/uniform/docs/password-fields.md new file mode 100644 index 000000000..1b244aec4 --- /dev/null +++ b/packages/uniform/docs/password-fields.md @@ -0,0 +1,37 @@ +# Password fields + +`Input` with `type="password"` masks the value and includes a keyboard-accessible +show/hide button by default. Toggling changes only visibility: the field name, +value, validation, and submission behavior remain the same. Pointer activation +keeps input focus; keyboard users can reach and activate the toggle normally. +Form resets hide the password again. + +```tsx + +``` + +Set `hidePasswordToggle={true}` to keep a password input masked without a toggle. +Custom `endContent` appears alongside the toggle. Disabled inputs disable the +toggle, and `clearable` password inputs offer separate clear and reveal buttons. + +## Sensitive debug values + +Password inputs are automatically redacted as `[REDACTED]` in Uniform's debug +viewer, its JSON copy output, and Uniform's submit debug logs. Showing a password +inside the input does not reveal it in debug output. + +Use `sensitive` on an ordinary `Input` for API tokens and other secrets: + +```tsx + + +``` + +Custom field components can pass `sensitive: true` to `useUniformField`. The +shared hook registers the field path with the form. Nested paths and array items +are supported. Sensitivity is retained for the form's lifetime, including when +an input is unmounted but its value remains in form state. + +Redaction changes diagnostic copies only. Form values, validation, and `onSubmit` +receive the original values. Application callbacks that log submitted data must +handle their own redaction. diff --git a/packages/uniform/src/Form/Form.test.tsx b/packages/uniform/src/Form/Form.test.tsx index 6316db60b..946704dda 100644 --- a/packages/uniform/src/Form/Form.test.tsx +++ b/packages/uniform/src/Form/Form.test.tsx @@ -1,12 +1,15 @@ import { describe, expect, it, vi } from 'vitest'; -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { string, veto } from '@fuf-stack/veto'; +import { array, object, objectLoose, string, veto } from '@fuf-stack/veto'; import '@testing-library/jest-dom/vitest'; +import { useClientValidation } from '../hooks/useClientValidation'; +import { useFormContext } from '../hooks/useFormContext'; +import Input from '../Input'; import Form from './Form'; describe('Form', () => { @@ -290,6 +293,410 @@ describe('Form', () => { }); }); + describe('disabled field submission', () => { + it.each([false, true])( + 'omits populated disabled fields and preserves enabled fields (schema: %s)', + async (withSchema) => { + const onSubmit = vi.fn(); + const user = userEvent.setup(); + + const validation = withSchema + ? veto({ + disabledField: string().optional(), + enabledField: string(), + }) + : undefined; + + render( + , + ); + + expect( + screen.getByRole('textbox', { name: 'disabledField' }), + ).toBeDisabled(); + expect( + screen.getByRole('textbox', { name: 'disabledField' }), + ).toHaveValue('retained'); + await user.click(screen.getByRole('button', { name: 'Submit' })); + + await waitFor(() => + expect(onSubmit).toHaveBeenCalledExactlyOnceWith( + { enabledField: 'included' }, + expect.anything(), + ), + ); + }, + ); + + it('reports a missing required disabled field even when its stored value is valid', async () => { + const onSubmit = vi.fn(); + const user = userEvent.setup(); + const validation = veto({ field: string().min(3) }); + const validate = vi.spyOn(validation, 'validateAsync'); + const form = (disabled: boolean) => ( + + ); + const { rerender } = render(form(true)); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(screen.getByText('Field is required')).toBeVisible(), + ); + expect(validate).toHaveBeenLastCalledWith({}); + expect(onSubmit).not.toHaveBeenCalled(); + expect(screen.getByRole('textbox', { name: 'field' })).toHaveValue( + 'valid', + ); + + rerender(form(false)); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(onSubmit).toHaveBeenCalledExactlyOnceWith( + { field: 'valid' }, + expect.anything(), + ), + ); + }); + + it('submits the retained user value after a field is re-enabled', async () => { + const onSubmit = vi.fn(); + const user = userEvent.setup(); + const form = (disabled: boolean) => ( + + ); + const { rerender } = render(form(false)); + const input = screen.getByRole('textbox', { name: 'field' }); + + await user.clear(input); + await user.type(input, 'updated'); + await user.tab(); + rerender(form(true)); + expect(input).toBeDisabled(); + expect(input).toHaveValue('updated'); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(onSubmit).toHaveBeenCalledExactlyOnceWith({}, expect.anything()), + ); + + rerender(form(false)); + expect(input).toBeEnabled(); + expect(input).toHaveValue('updated'); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(2)); + expect(onSubmit).toHaveBeenNthCalledWith( + 2, + { field: 'updated' }, + expect.anything(), + ); + }); + + it('omits an optional disabled value and validates its retained value when re-enabled', async () => { + const onSubmit = vi.fn(); + const user = userEvent.setup(); + const validation = veto({ + field: string().min(3, 'Value is too short').optional(), + }); + const form = (disabled: boolean) => ( + + ); + const { rerender } = render(form(true)); + const input = screen.getByRole('textbox', { name: 'field' }); + + expect(input).toBeDisabled(); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(onSubmit).toHaveBeenCalledExactlyOnceWith({}, expect.anything()), + ); + expect(screen.queryByText('Value is too short')).not.toBeInTheDocument(); + + rerender(form(false)); + expect(input).toBeEnabled(); + expect(input).toHaveValue('ab'); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(screen.getByText('Value is too short')).toBeVisible(), + ); + expect(onSubmit).toHaveBeenCalledTimes(1); + + rerender(form(true)); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(2)); + expect(onSubmit).toHaveBeenNthCalledWith(2, {}, expect.anything()); + await waitFor(() => + expect( + screen.queryByText('Value is too short'), + ).not.toBeInTheDocument(), + ); + + rerender(form(false)); + await user.type(input, 'c'); + await user.tab(); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(3)); + expect(onSubmit).toHaveBeenNthCalledWith( + 3, + { field: 'abc' }, + expect.anything(), + ); + }); + }); + + describe('disabled validation paths', () => { + it('omits a disabled sibling when only the enabled field is validated', async () => { + const user = userEvent.setup(); + const validation = veto({ + excluded: string().min(3).optional(), + enabled: string(), + }); + const validate = vi.spyOn(validation, 'validateAsync'); + const triggered = vi.fn(); + const ValidateEnabled = () => { + const { trigger } = useFormContext(); + return ( + + ); + }; + + render( + , + ); + + await user.click( + screen.getByRole('button', { name: 'Validate enabled' }), + ); + + await waitFor(() => expect(triggered).toHaveBeenCalledWith(true)); + // trigger('enabled') can succeed even if the resolver reports sibling errors; + // inspect the actual validation input to prove the sibling was omitted. + expect(validate).toHaveBeenLastCalledWith({ enabled: 'valid' }); + expect(screen.getByRole('textbox', { name: 'excluded' })).toHaveValue( + 'x', + ); + }); + + it.each([false, true])( + 'submits disabled flat-array entries as null placeholders (schema: %s)', + async (withSchema) => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + const validation = withSchema + ? veto({ tags: array(string().min(3).optional()) }) + : undefined; + + render( + , + ); + + await user.click(screen.getByRole('button', { name: 'Submit' })); + + await waitFor(() => + expect(onSubmit).toHaveBeenCalledExactlyOnceWith( + { tags: [null, null, 'valid'] }, + expect.anything(), + ), + ); + }, + ); + + it('retains exclusion across reset and validates the reset value when re-enabled', async () => { + const onSubmit = vi.fn(); + const user = userEvent.setup(); + const Reset = () => { + const { reset } = useFormContext(); + return ( + + ); + }; + const validation = veto({ + field: string().min(6, 'Value is too short').optional(), + }); + const form = (disabled: boolean) => ( + + ); + const { rerender } = render(form(true)); + await user.click(screen.getByRole('button', { name: 'Reset' })); + expect(screen.getByRole('textbox', { name: 'field' })).toHaveValue( + 'reset', + ); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(onSubmit).toHaveBeenCalledExactlyOnceWith({}, expect.anything()), + ); + rerender(form(false)); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(screen.getByText('Value is too short')).toBeVisible(), + ); + expect(onSubmit).toHaveBeenCalledTimes(1); + }); + + it.each([undefined, 'invalid'])( + 'omits optional disabled values before base and client validation (disabled value: %s)', + async (disabledValue) => { + const clientRule = vi.fn(async () => false); + const ClientRules = () => { + useClientValidation(true, () => + objectLoose({ + excluded: string() + .refine(clientRule, 'Client rule ran') + .nullish(), + }), + ); + return null; + }; + const onSubmit = vi.fn(); + const user = userEvent.setup(); + const validation = veto({ + excluded: string().optional(), + enabled: string().min(3, 'Enabled value is too short'), + }); + + render( + , + ); + const enabled = screen.getByRole('textbox', { name: 'enabled' }); + await user.type(enabled, 'ab'); + await user.tab(); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(screen.getByText('Enabled value is too short')).toBeVisible(), + ); + expect(onSubmit).not.toHaveBeenCalled(); + expect(clientRule).not.toHaveBeenCalled(); + + await user.type(enabled, 'c'); + await user.tab(); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(onSubmit).toHaveBeenCalledExactlyOnceWith( + { enabled: 'abc' }, + expect.anything(), + ), + ); + expect(clientRule).not.toHaveBeenCalled(); + }, + ); + + it('excludes nested disabled fields without disabling the same field in other array rows', async () => { + const onSubmit = vi.fn(); + const user = userEvent.setup(); + const validation = veto({ + rows: array( + object({ + name: string().min(3, 'Name is too short').optional(), + code: string(), + }), + ), + }); + + render( + , + ); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(screen.getByText('Name is too short')).toBeVisible(), + ); + expect(onSubmit).not.toHaveBeenCalled(); + await user.type( + screen.getByRole('textbox', { name: 'rows.1.name' }), + 'yz', + ); + await user.tab(); + await user.click(screen.getByRole('button', { name: 'Submit' })); + await waitFor(() => + expect(onSubmit).toHaveBeenCalledExactlyOnceWith( + { rows: [{ code: 'first' }, { name: 'xyz', code: 'second' }] }, + expect.anything(), + ), + ); + }); + }); + describe('validation', () => { it('should validate according to validation schema', async () => { const handleSubmit = vi.fn(); diff --git a/packages/uniform/src/Form/subcomponents/FormContext.tsx b/packages/uniform/src/Form/subcomponents/FormContext.tsx index 9cc50d41a..61796727b 100644 --- a/packages/uniform/src/Form/subcomponents/FormContext.tsx +++ b/packages/uniform/src/Form/subcomponents/FormContext.tsx @@ -6,7 +6,13 @@ import type { import type { BaseSyntheticEvent, ReactNode } from 'react'; import type { FieldValues, Path, SubmitHandler } from 'react-hook-form'; -import React, { useCallback, useMemo, useRef, useState } from 'react'; +import React, { + useCallback, + useLayoutEffect, + useMemo, + useRef, + useState, +} from 'react'; import { FormProvider as HookFormProvider, useForm } from 'react-hook-form'; import createDebug from 'debug'; @@ -241,6 +247,7 @@ interface FormProviderProps { * FormProvider component provides: * - Veto validation schema context * - Resolver-backed validation error state + * - RHF control binding for disabled-value omission without changing schema rules * - Client validation schema management * - Submit handler with automatic data conversion and submission control with preventSubmit * - Form Debug Mode state @@ -271,7 +278,7 @@ const FormProvider: React.FC