DataForm: Fix hidden fields validation

packages/dataviews/CHANGELOG.md

@@ -17,6 +17,7 @@
 
 ### Bug Fix
 
+-   DataForms: Skip validation for fields hidden through the `isVisible` API, so a hidden field with validation rules (e.g. `required`) no longer makes the form invalid. Toggling a field's visibility now clears or restores its validity accordingly. [#81377](https://github.com/WordPress/gutenberg/pull/81377)
 -   DataViews: Pass only the eligible items to a bulk action's `callback`. A bulk action is offered when any one selected item is eligible for it, so the callback could run against items it had declared, through `isEligible`, that it could not handle. [#81198](https://github.com/WordPress/gutenberg/pull/81198)
 -   DataViews: Fix the `between` date filter discarding a manually entered `From`/`To` date on blur. The control now commits an incomplete range with an unfilled bound — which neither filters nor renders a chip — instead of waiting for both dates, so a typed date survives tabbing away and a range can be entered manually at all. [#81150](https://github.com/WordPress/gutenberg/pull/81150)
 -   DataViews: Render the filter chip for an incomplete `between` range as if no value were set — matching how the filter itself does not apply — instead of showing a dangling bound or the literal string "undefined". A `null` bound, produced when an unfilled bound round-trips through JSON persistence, is now treated as unfilled too. [#80830](https://github.com/WordPress/gutenberg/pull/80830)

packages/dataviews/README.md

@@ -1763,6 +1763,8 @@ Function that indicates if the field should be visible.
     -   `item`: the data to be processed
 -   Returns a `boolean` indicating if the field should be visible (`true`) or not (`false`).
 
+A field hidden through `isVisible` is not validated: its validation rules are skipped while it is hidden and re-applied when it becomes visible again.
+
 This can be useful to hide fields based on the state of other fields. For example, a `staticHomepage` field can be hidden depending on the value of the `homepageDisplay` field:
 
 ```js

packages/dataviews/src/dataform/stories/validation.tsx

@@ -109,6 +109,8 @@ const ValidationComponent = ( {
 		dateRange?: string;
 		datetime?: string;
 		time?: string;
+		showConditionalText?: boolean;
+		conditionalText?: string;
 	};
 
 	const [ post, setPost ] = useState< ValidatedItem >( {
@@ -134,6 +136,8 @@ const ValidationComponent = ( {
 		dateRange: undefined,
 		datetime: undefined,
 		time: undefined,
+		showConditionalText: undefined,
+		conditionalText: undefined,
 	} );
 
 	// Cache for getElements functions - ensures promises are only created once
@@ -911,6 +915,22 @@ const ValidationComponent = ( {
 					max: minMax ? '17:00' : undefined,
 				},
 			},
+			{
+				id: 'showConditionalText',
+				type: 'boolean',
+				label: 'Show conditional text',
+			},
+			{
+				id: 'conditionalText',
+				type: 'text',
+				label: 'Conditional text',
+				description:
+					'Always required, but only validated while visible.',
+				isVisible: ( item ) => item.showConditionalText === true,
+				isValid: {
+					required: true,
+				},
+			},
 		];
 	}, [ elements, custom, pattern, minMax, getElements, required ] );
 
@@ -975,6 +995,8 @@ const ValidationComponent = ( {
 					'dateRange',
 					'datetime',
 					'time',
+					'showConditionalText',
+					'conditionalText',
 				],
 			};
 		}
@@ -1013,6 +1035,11 @@ const ValidationComponent = ( {
 				label: 'Date fields',
 				children: [ 'date', 'dateRange', 'datetime', 'time' ],
 			},
+			{
+				id: 'conditionalFields',
+				label: 'Conditionally visible fields',
+				children: [ 'showConditionalText', 'conditionalText' ],
+			},
 		];
 
 		if ( layout === 'panel-dropdown' ) {

packages/dataviews/src/hooks/test/use-form-validity.ts

@@ -1,4 +1,4 @@
-import { renderHook, waitFor } from '@testing-library/react';
+import { act, renderHook, waitFor } from '@testing-library/react';
 import { useFormValidity } from '../use-form-validity';
 import type { Field } from '../../types';
 
@@ -228,6 +228,145 @@ describe( 'useFormValidity', () => {
 		} );
 	} );
 
+	describe( 'isVisible', () => {
+		type TestItem = {
+			id: number;
+			isActive: boolean;
+			name?: string;
+			email?: string;
+		};
+		const fields: Field< TestItem >[] = [
+			{
+				id: 'isActive',
+				type: 'boolean',
+			},
+			{
+				id: 'name',
+				type: 'text',
+				isVisible: ( item ) => item.isActive === true,
+				isValid: {
+					required: true,
+				},
+			},
+			{
+				id: 'email',
+				type: 'text',
+				isValid: {
+					required: true,
+				},
+			},
+		];
+		const form = { fields: [ 'isActive', 'name' ] };
+
+		it( 'hidden fields are not validated', () => {
+			const item: TestItem = { id: 1, isActive: false, name: undefined };
+			const {
+				result: {
+					current: { validity, isValid },
+				},
+			} = renderHook( () => useFormValidity( item, fields, form ) );
+			expect( validity ).toEqual( undefined );
+			expect( isValid ).toBe( true );
+		} );
+
+		it( 'toggling visibility re-runs validation for an unchanged value', () => {
+			const { result, rerender } = renderHook(
+				( { item }: { item: TestItem } ) =>
+					useFormValidity( item, fields, form ),
+				{
+					initialProps: {
+						item: { id: 1, isActive: true, name: undefined },
+					},
+				}
+			);
+			expect( result.current.validity ).toEqual( {
+				name: { required: { type: 'invalid' } },
+			} );
+			expect( result.current.isValid ).toBe( false );
+
+			rerender( { item: { id: 1, isActive: false, name: undefined } } );
+			expect( result.current.validity ).toEqual( undefined );
+			expect( result.current.isValid ).toBe( true );
+
+			rerender( { item: { id: 1, isActive: true, name: undefined } } );
+			expect( result.current.validity ).toEqual( {
+				name: { required: { type: 'invalid' } },
+			} );
+			expect( result.current.isValid ).toBe( false );
+		} );
+
+		it( 'hidden children of combined fields are not validated', () => {
+			const item: TestItem = {
+				id: 1,
+				isActive: false,
+				name: undefined,
+				email: undefined,
+			};
+			const combinedForm = {
+				fields: [ { id: 'combined', children: [ 'name', 'email' ] } ],
+			};
+			const {
+				result: {
+					current: { validity, isValid },
+				},
+			} = renderHook( () =>
+				useFormValidity( item, fields, combinedForm )
+			);
+			expect( validity ).toEqual( {
+				combined: {
+					children: {
+						email: { required: { type: 'invalid' } },
+					},
+				},
+			} );
+			expect( isValid ).toBe( false );
+		} );
+
+		it( 'a stale async result does not resurface after the field is hidden', async () => {
+			let resolveCustom: ( value: string | null ) => void = () => {};
+			const asyncFields: Field< TestItem >[] = [
+				{
+					id: 'isActive',
+					type: 'boolean',
+				},
+				{
+					id: 'name',
+					type: 'text',
+					isVisible: ( item ) => item.isActive === true,
+					isValid: {
+						custom: () =>
+							new Promise< string | null >( ( resolve ) => {
+								resolveCustom = resolve;
+							} ),
+					},
+				},
+			];
+			const { result, rerender } = renderHook(
+				( { item }: { item: TestItem } ) =>
+					useFormValidity( item, asyncFields, form ),
+				{
+					initialProps: {
+						item: { id: 1, isActive: true, name: 'a' },
+					},
+				}
+			);
+			expect( result.current.validity ).toEqual( {
+				name: {
+					custom: { type: 'validating', message: 'Validating…' },
+				},
+			} );
+
+			rerender( { item: { id: 1, isActive: false, name: 'a' } } );
+			expect( result.current.validity ).toEqual( undefined );
+
+			await act( async () => {
+				resolveCustom( 'Invalid value.' );
+			} );
+			expect( result.current.validity ).toEqual( undefined );
+			expect( result.current.isValid ).toBe( true );
+		} );
+	} );
+
 	describe( 'isValid.required', () => {
 		const REQUIRED_MESSAGE = {
 			required: { type: 'invalid' },

packages/dataviews/src/hooks/use-form-validity.ts

@@ -388,11 +388,37 @@ type PromiseHandler< Item > = {
 	item: Item;
 };
 
+function isFormFieldHidden< Item >(
+	formField: FormFieldToValidate< Item >,
+	item: Item
+): boolean {
+	// `DataFormLayout` only applies `isVisible` to leaf fields; combined
+	// fields are always rendered, so they must always be validated.
+	return (
+		formField.children.length === 0 &&
+		!! formField.field?.isVisible &&
+		! formField.field.isVisible( item )
+	);
+}
+
 function validateFormField< Item >(
 	item: Item,
 	formField: FormFieldToValidate< Item >,
 	promiseHandler: PromiseHandler< Item >
 ): FieldValidity | undefined {
+	if ( isFormFieldHidden( formField, item ) ) {
+		// Invalidate in-flight async validations so a stale result cannot
+		// resurface for a field that is no longer rendered.
+		const { customCounterRef, elementsCounterRef } = promiseHandler;
+		if ( customCounterRef.current[ formField.id ] ) {
+			customCounterRef.current[ formField.id ] += 1;
+		}
+		if ( elementsCounterRef.current[ formField.id ] ) {
+			elementsCounterRef.current[ formField.id ] += 1;
+		}
+		return undefined;
+	}
+
 	// Validate the field: isValid.required
 	if (
 		formField.field?.isValid.required &&
@@ -597,19 +623,22 @@ function getFormFieldValue< Item >(
 	item: Item
 ): any {
 	const fieldValue = formField?.field?.getValue( { item } );
+	// `isHidden` is never read directly. It is folded into the snapshot so
+	// that the `fastDeepEqual` change check in `validate` fails when a
+	// field's visibility toggles with an unchanged value, forcing that
+	// field to re-validate instead of being skipped as untouched.
+	const isHidden = isFormFieldHidden( formField, item );
 	if ( formField.children.length === 0 ) {
-		return fieldValue;
+		return { value: fieldValue, isHidden };
 	}
 
 	const childrenValues = formField.children.map( ( child ) =>
 		getFormFieldValue( child, item )
 	);
-	if ( ! childrenValues ) {
-		return fieldValue;
-	}
 
 	return {
 		value: fieldValue,
+		isHidden,
 		children: childrenValues,
 	};
 }