Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/dataviews/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

### Enhancements

- Field API: move validation to the field type. [#73642](https://github.com/WordPress/gutenberg/pull/73642)
- DataForm: add support for `min`/`max` and `minLength`/`maxLength` validation for relevant controls. [#73465](https://github.com/WordPress/gutenberg/pull/73465)
- Field API: display formats for `number` and `integer` types. [#73644](https://github.com/WordPress/gutenberg/pull/73644)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ import { Flex } from '@wordpress/components';
/**
* Internal dependencies
*/
import type { View, NormalizedFilter, NormalizedField } from '../../types';
import type {
View,
NormalizedFilter,
NormalizedField,
NormalizedRules,
} from '../../types';
import { getCurrentValue } from './utils';

interface UserInputWidgetProps {
Expand Down Expand Up @@ -58,10 +63,7 @@ export default function InputWidget( {
return {
...currentField,
// Deactivate validation for filters.
isValid: {
required: false,
custom: () => null,
},
isValid: {} satisfies NormalizedRules< any >,
// Configure getValue/setValue as if Item was a plain object.
getValue: ( { item }: { item: any } ) =>
item[ currentField.id ],
Expand Down
10 changes: 7 additions & 3 deletions packages/dataviews/src/dataform-controls/textarea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,20 @@ export default function Textarea< Item >( {

return (
<ValidatedTextareaControl
required={ !! isValid?.required }
required={ !! isValid.required }
customValidity={ getCustomValidity( isValid, validity ) }
label={ label }
placeholder={ placeholder }
value={ value ?? '' }
help={ description }
onChange={ onChangeControl }
rows={ rows }
minLength={ isValid?.minLength }
maxLength={ isValid?.maxLength }
minLength={
isValid.minLength ? isValid.minLength.constraint : undefined
}
maxLength={
isValid.maxLength ? isValid.maxLength.constraint : undefined
}
__next40pxDefaultSize
__nextHasNoMarginBottom
hideLabelFromVision={ hideLabelFromVision }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/**
* Internal dependencies
*/
import type { Rules, FieldValidity } from '../../types';
import type { NormalizedRules, FieldValidity } from '../../types';

export default function getCustomValidity< Item >(
isValid: Rules< Item >,
isValid: NormalizedRules< Item >,
validity: FieldValidity | undefined
) {
let customValidity;
Expand All @@ -16,6 +16,14 @@ export default function getCustomValidity< Item >(
: undefined;
} else if ( isValid?.pattern && validity?.pattern ) {
customValidity = validity.pattern;
} else if ( isValid?.min && validity?.min ) {
customValidity = validity.min;
} else if ( isValid?.max && validity?.max ) {
customValidity = validity.max;
} else if ( isValid?.minLength && validity?.minLength ) {
customValidity = validity.minLength;
} else if ( isValid?.maxLength && validity?.maxLength ) {
customValidity = validity.maxLength;
Comment on lines +19 to +26

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is something we missed doing in #73465

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function is a bit weird, how does one combine multiple validation rules. Also should this be called getFieldValidity or getCombinedValidity.

It's also possible that I don't really understand what this function does.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This function just returns the message to be displayed in the control. It's a single message. It could be refactored to return early if it found a message as well.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What happens if the validation fails on multiple constraints, shouldn't we combine the message? Should we clarify in the name that it returns a message

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There's only one message per field, we return early when some constraint failed. This early return prevents triggering multiple network requests (custom validation) when they are not used.

} else if ( isValid?.elements && validity?.elements ) {
customValidity = validity.elements;
} else if ( validity?.custom ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export default function ValidatedText< Item >( {

return (
<ValidatedInputControl
required={ !! isValid?.required }
required={ !! isValid.required }
customValidity={ getCustomValidity( isValid, validity ) }
label={ label }
placeholder={ placeholder }
Expand All @@ -67,9 +67,13 @@ export default function ValidatedText< Item >( {
type={ type }
prefix={ prefix }
suffix={ suffix }
pattern={ isValid?.pattern }
minLength={ isValid?.minLength }
maxLength={ isValid?.maxLength }
pattern={ isValid.pattern ? isValid.pattern.constraint : undefined }
minLength={
isValid.minLength ? isValid.minLength.constraint : undefined
}
maxLength={
isValid.maxLength ? isValid.maxLength.constraint : undefined
}
__next40pxDefaultSize
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export default function ValidatedNumber< Item >( {

return (
<ValidatedNumberControl
required={ !! isValid?.required }
required={ !! isValid.required }
customValidity={ getCustomValidity( isValid, validity ) }
label={ label }
help={ description }
Expand All @@ -166,8 +166,8 @@ export default function ValidatedNumber< Item >( {
__next40pxDefaultSize
hideLabelFromVision={ hideLabelFromVision }
step={ step }
min={ isValid?.min }
max={ isValid?.max }
min={ isValid.min ? isValid.min.constraint : undefined }
max={ isValid.max ? isValid.max.constraint : undefined }
/>
);
}
19 changes: 14 additions & 5 deletions packages/dataviews/src/dataform-layouts/panel/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,20 @@ function ModalContent< Item >( {
[ field ]
);

const { validity } = useFormValidity(
modalData,
fields as Field< any >[],
form
);
const fieldsAsFieldType: Field< Item >[] = fields.map( ( f ) => ( {
...f,
Edit: f.Edit === null ? undefined : f.Edit,
isValid: {
required: f.isValid.required?.constraint,
elements: f.isValid.elements?.constraint,
min: f.isValid.min?.constraint,
max: f.isValid.max?.constraint,
pattern: f.isValid.pattern?.constraint,
minLength: f.isValid.minLength?.constraint,
maxLength: f.isValid.maxLength?.constraint,
},
} ) );
const { validity } = useFormValidity( modalData, fieldsAsFieldType, form );

const onApply = () => {
onChange( changes );
Expand Down
45 changes: 26 additions & 19 deletions packages/dataviews/src/field-types/array.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,43 @@ import { __ } from '@wordpress/i18n';
/**
* Internal dependencies
*/
import type { DataViewRenderFieldProps, Rules, SortDirection } from '../types';
import type {
DataViewRenderFieldProps,
NormalizedField,
SortDirection,
} from '../types';
import type { FieldType } from '../types/private';
import {
OPERATOR_IS_ALL,
OPERATOR_IS_ANY,
OPERATOR_IS_NONE,
OPERATOR_IS_NOT_ALL,
} from '../constants';
import isValidRequiredForArray from './utils/is-valid-required-for-array';
import isValidElements from './utils/is-valid-elements';

function render( { item, field }: DataViewRenderFieldProps< any > ) {
const value = field.getValue( { item } ) || [];
return value.join( ', ' );
}

const isValid: Rules< any > = {
elements: true,
custom: ( item: any, normalizedField ) => {
const value = normalizedField.getValue( { item } );
function isValidCustom< Item >( item: Item, field: NormalizedField< Item > ) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This isValidCustom function is the same as isValid.custom before. It's just unwrapped. The same for every other field type.

const value = field.getValue( { item } );

if (
! [ undefined, '', null ].includes( value ) &&
! Array.isArray( value )
) {
return __( 'Value must be an array.' );
}
if (
! [ undefined, '', null ].includes( value ) &&
! Array.isArray( value )
) {
return __( 'Value must be an array.' );
}

// Only allow strings for now. Can be extended to other types in the future.
if ( ! value.every( ( v: any ) => typeof v === 'string' ) ) {
return __( 'Every value must be a string.' );
}
// Only allow strings for now. Can be extended to other types in the future.
if ( ! value.every( ( v: any ) => typeof v === 'string' ) ) {
return __( 'Every value must be a string.' );
}

return null;
},
};
return null;
}

const sort = ( a: any, b: any, direction: SortDirection ) => {
// Sort arrays by length, then alphabetically by joined string
Expand All @@ -63,7 +66,6 @@ export default {
render,
Edit: 'array',
sort,
isValid,
enableSorting: true,
enableGlobalSearch: false,
defaultOperators: [ OPERATOR_IS_ANY, OPERATOR_IS_NONE ],
Expand All @@ -74,4 +76,9 @@ export default {
OPERATOR_IS_NOT_ALL,
],
getFormat: () => ( {} ),
validate: {
required: isValidRequiredForArray,
elements: isValidElements,
custom: isValidCustom,
},
} satisfies FieldType< any >;
37 changes: 22 additions & 15 deletions packages/dataviews/src/field-types/boolean.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,16 @@ import { __ } from '@wordpress/i18n';
/**
* Internal dependencies
*/
import type { DataViewRenderFieldProps, Rules, SortDirection } from '../types';
import type {
DataViewRenderFieldProps,
NormalizedField,
SortDirection,
} from '../types';
import type { FieldType } from '../types/private';
import RenderFromElements from './utils/render-from-elements';
import { OPERATOR_IS, OPERATOR_IS_NOT } from '../constants';
import isValidElements from './utils/is-valid-elements';
import isValidRequiredForBool from './utils/is-valid-required-for-bool';

function render( { item, field }: DataViewRenderFieldProps< any > ) {
if ( field.hasElements ) {
Expand All @@ -27,21 +33,18 @@ function render( { item, field }: DataViewRenderFieldProps< any > ) {
return null;
}

const isValid: Rules< any > = {
elements: true,
custom: ( item: any, normalizedField ) => {
const value = normalizedField.getValue( { item } );
function isValidCustom< Item >( item: Item, field: NormalizedField< Item > ) {
const value = field.getValue( { item } );

if (
! [ undefined, '', null ].includes( value ) &&
! [ true, false ].includes( value )
) {
return __( 'Value must be true, false, or undefined' );
}
if (
! [ undefined, '', null ].includes( value ) &&
! [ true, false ].includes( value )
) {
return __( 'Value must be true, false, or undefined' );
}

return null;
},
};
return null;
}

const sort = ( a: any, b: any, direction: SortDirection ) => {
const boolA = Boolean( a );
Expand All @@ -65,7 +68,11 @@ export default {
render,
Edit: 'checkbox',
sort,
isValid,
validate: {
required: isValidRequiredForBool,
elements: isValidElements,
custom: isValidCustom,
},
enableSorting: true,
enableGlobalSearch: false,
defaultOperators: [ OPERATOR_IS, OPERATOR_IS_NOT ],
Expand Down
37 changes: 22 additions & 15 deletions packages/dataviews/src/field-types/color.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import { __ } from '@wordpress/i18n';
/**
* Internal dependencies
*/
import type { DataViewRenderFieldProps, Rules, SortDirection } from '../types';
import type {
DataViewRenderFieldProps,
NormalizedField,
SortDirection,
} from '../types';
import type { FieldType } from '../types/private';
import RenderFromElements from './utils/render-from-elements';
import {
Expand All @@ -20,6 +24,8 @@ import {
OPERATOR_IS_NONE,
OPERATOR_IS_NOT,
} from '../constants';
import isValidElements from './utils/is-valid-elements';
import isValidRequired from './utils/is-valid-required';

function render( { item, field }: DataViewRenderFieldProps< any > ) {
if ( field.hasElements ) {
Expand Down Expand Up @@ -50,21 +56,18 @@ function render( { item, field }: DataViewRenderFieldProps< any > ) {
);
}

const isValid: Rules< any > = {
elements: true,
custom: ( item: any, normalizedField ) => {
const value = normalizedField.getValue( { item } );
function isValidCustom< Item >( item: Item, field: NormalizedField< Item > ) {
const value = field.getValue( { item } );

if (
! [ undefined, '', null ].includes( value ) &&
! colord( value ).isValid()
) {
return __( 'Value must be a valid color.' );
}
if (
! [ undefined, '', null ].includes( value ) &&
! colord( value ).isValid()
) {
return __( 'Value must be a valid color.' );
}

return null;
},
};
return null;
}

const sort = ( a: any, b: any, direction: SortDirection ) => {
// Convert colors to HSL for better sorting
Expand Down Expand Up @@ -99,7 +102,6 @@ export default {
render,
Edit: 'color',
sort,
isValid,
enableSorting: true,
enableGlobalSearch: false,
defaultOperators: [ OPERATOR_IS_ANY, OPERATOR_IS_NONE ],
Expand All @@ -110,4 +112,9 @@ export default {
OPERATOR_IS_NONE,
],
getFormat: () => ( {} ),
validate: {
required: isValidRequired,
elements: isValidElements,
custom: isValidCustom,
},
} satisfies FieldType< any >;
Loading
Loading