diff --git a/ui/src/react-admin/modules/content-page/components/ContentBlockForm/ContentBlockForm.tsx b/ui/src/react-admin/modules/content-page/components/ContentBlockForm/ContentBlockForm.tsx index c3a681f8c..c1c342813 100644 --- a/ui/src/react-admin/modules/content-page/components/ContentBlockForm/ContentBlockForm.tsx +++ b/ui/src/react-admin/modules/content-page/components/ContentBlockForm/ContentBlockForm.tsx @@ -22,6 +22,7 @@ import React from 'react'; import CopyToClipboard from 'react-copy-to-clipboard'; import { BlockHeading } from '~content-blocks/BlockHeading/BlockHeading'; import { ToastType } from '~core/config/config.types'; +import { validateFieldGroup } from '~modules/content-page/helpers/validate-content-block-config'; import { findImageInJson } from '~shared/helpers/find-image-in-json'; import { showToast } from '~shared/helpers/show-toast'; import { tText } from '~shared/helpers/translation-functions'; @@ -33,6 +34,7 @@ import type { ContentBlockConfig, ContentBlockErrors, ContentBlockField, + ContentBlockFieldGroup, ContentBlockState, ContentBlockStateType, RepeatedContentBlockComponentState, @@ -90,6 +92,7 @@ const ContentBlockForm: FunctionComponent = ({ stateIndex?: number ) => { const field = config[formGroupType].fields[key] as ContentBlockField; + const fieldGroup = config[formGroupType].fields[key] as ContentBlockFieldGroup; const updateObject = { ...Object.fromEntries((field.fieldsToResetOnChange || []).map((key) => [key, null])), [key]: value, @@ -109,7 +112,20 @@ const ContentBlockForm: FunctionComponent = ({ Array.isArray(stateUpdate) && typeof stateIndex === 'number' ? stateUpdate[stateIndex] : stateUpdate; - if (!(field.isVisible && !field.isVisible(config, stateToCheckVisibility))) { + + if (fieldGroup.type === 'fieldGroup') { + // Mechanism B: re-validate every element in the repeated group so inline + // errors clear/appear as the user edits (validators live on the inner fields) + const newErrors = validateFieldGroup( + configErrors, + config, + key, + fieldGroup.fields, + // biome-ignore lint/suspicious/noExplicitAny: state is a single object here (mechanism B) + (stateUpdate as any)[key] + ); + onError(blockIndex, newErrors); + } else if (!(field.isVisible && !field.isVisible(config, stateToCheckVisibility))) { handleValidation(field, key, value, stateIndex); } onChange(formGroupType, stateUpdate, Array.isArray(components.state) ? stateIndex : undefined); diff --git a/ui/src/react-admin/modules/content-page/components/ContentBlockFormGroup/ContentBlockFormGroup.tsx b/ui/src/react-admin/modules/content-page/components/ContentBlockFormGroup/ContentBlockFormGroup.tsx index 39b72ea61..3a8fb5c05 100644 --- a/ui/src/react-admin/modules/content-page/components/ContentBlockFormGroup/ContentBlockFormGroup.tsx +++ b/ui/src/react-admin/modules/content-page/components/ContentBlockFormGroup/ContentBlockFormGroup.tsx @@ -9,6 +9,7 @@ import type { ContentBlockComponentsConfig, ContentBlockConfig, ContentBlockField, + ContentBlockFieldGroup, ContentBlockState, ContentBlockStateType, } from '../../types/content-block.types'; @@ -41,12 +42,16 @@ const ContentBlockFormGroup: FunctionComponent = ({ }) => (
{Object.keys(formGroup.fields).map((key: string, formGroupIndex: number) => { - let error: string[]; + let error: string[] | undefined; const configErrors = config.errors || {}; const stateKey = key as keyof ContentBlockComponentState | keyof ContentBlockState; const formErrorsForBlock = configErrors[stateKey]; + const isFieldGroup = (formGroup.fields[key] as ContentBlockFieldGroup).type === 'fieldGroup'; - if (typeof stateIndex === 'number') { + if (isFieldGroup) { + // Field group errors are nested per element and rendered inside the group itself + error = undefined; + } else if (typeof stateIndex === 'number') { error = (formErrorsForBlock?.[stateIndex] || []) as string[]; } else { error = formErrorsForBlock as string[]; diff --git a/ui/src/react-admin/modules/content-page/components/ContentBlockRenderer/ContentBlockRenderer.const.tsx b/ui/src/react-admin/modules/content-page/components/ContentBlockRenderer/ContentBlockRenderer.const.tsx index 2b41e4d26..53a9fb6a6 100644 --- a/ui/src/react-admin/modules/content-page/components/ContentBlockRenderer/ContentBlockRenderer.const.tsx +++ b/ui/src/react-admin/modules/content-page/components/ContentBlockRenderer/ContentBlockRenderer.const.tsx @@ -96,6 +96,12 @@ export function GET_BLOCK_COMPONENT( return blocks[type]; } +/** + * @deprecated Legacy allowlist for the array-valued `components.state` repetition + * mechanism (A). Do NOT add new block types here. For new blocks use the `fieldGroup` + + * `repeat` mechanism (B), which is config-driven and needs no allowlist. + * See `components/blocks/README.md`. + */ export const REPEATABLE_CONTENT_BLOCKS = [ ContentBlockType.AnchorLinks, ContentBlockType.Buttons, diff --git a/ui/src/react-admin/modules/content-page/components/FieldGenerator/FieldGenerator.tsx b/ui/src/react-admin/modules/content-page/components/FieldGenerator/FieldGenerator.tsx index 986ec6f87..f4e37ba55 100644 --- a/ui/src/react-admin/modules/content-page/components/FieldGenerator/FieldGenerator.tsx +++ b/ui/src/react-admin/modules/content-page/components/FieldGenerator/FieldGenerator.tsx @@ -63,6 +63,7 @@ export const FieldGenerator: FunctionComponent = ({ = ({ + config, globalState, globalStateIndex, fieldKey, @@ -38,6 +42,9 @@ export const FieldGroup: FunctionComponent = ({ }) => { const { fields } = fieldGroup; + const groupErrors = config.errors?.[fieldKey] as ContentBlockFieldGroupErrors | undefined; + const elementErrors = groupErrors?.[fieldGroupStateIndex]; + const handleFieldGroupStateChange = ( // biome-ignore lint/suspicious/noExplicitAny: todo stateCopy: any, @@ -87,7 +94,7 @@ export const FieldGroup: FunctionComponent = ({ return ( - + diff --git a/ui/src/react-admin/modules/content-page/components/blocks/README.md b/ui/src/react-admin/modules/content-page/components/blocks/README.md new file mode 100644 index 000000000..ebdc28287 --- /dev/null +++ b/ui/src/react-admin/modules/content-page/components/blocks/README.md @@ -0,0 +1,134 @@ +# Content blocks + +Each content block lives in its own folder here and is made up of a small trio of +files: + +| File | Purpose | +|---|---| +| `Block.editorconfig.ts` | Declares the **editor** config: which fields the admin sees, their editor types, validators, defaults, and initial state. This is the file you copy when creating a new block. | +| `Block.tsx` | The **preview/render** component that turns saved state into markup on the public page. | +| `Block.types.ts` | (optional) Component-state / prop types for that block. | + +A block config (`ContentBlockConfig` in +[`../../types/content-block.types.ts`](../../types/content-block.types.ts)) has +two sibling sections: + +- **`components`** — the block's actual content. Its `state` may repeat (see + below). Rendered by the form and passed to the preview component as props. +- **`block`** — block-level "Blok-opties" (background, padding, user groups, …). + Always a single object, never repeated. + +--- + +## Repeated fields: two mechanisms + +There are two entirely different ways to let an admin add/remove multiple +instances of something. They look similar in the UI but are wired up completely +differently. **For new blocks, always use mechanism B.** + +### A — array-valued `components.state` *(legacy — do not use for new blocks)* + +The **whole** `components` config repeats as a unit: `components.state` is an +array, and every field is duplicated per array entry. Used by ~14 older blocks +(Buttons, CTAs, ImageGrid, MediaGrid, Spotlight, …). + +This path is driven by machinery outside the editorconfig: + +- the block type must be added to the hardcoded `REPEATABLE_CONTENT_BLOCKS` + allowlist ([`../ContentBlockRenderer/ContentBlockRenderer.const.tsx`](../ContentBlockRenderer/ContentBlockRenderer.const.tsx)); +- count is bounded by `components.limits.{min,max}`; +- add/remove goes through dedicated reducer actions + (`ADD_COMPONENTS_STATE` / `REMOVE_COMPONENTS_STATE`) plumbed through four + components; +- at render time the array is wrapped as the `elements` prop + ([`../ContentBlockRenderer/ContentBlockRenderer.tsx`](../ContentBlockRenderer/ContentBlockRenderer.tsx)). + +Limitations: only **one** repeatable set per block, and you cannot mix repeated +and non-repeated (scalar) fields at the same level. + +```ts +// BlockButtons.editorconfig.ts — components.state IS the array +components: { + limits: { max: 3 }, + state: [{ label: '', type: 'primary' }], // ButtonsBlockComponentState[] + fields: { /* type, label, icon, buttonAction … repeated per entry */ }, +} +``` + +### B — `fieldGroup` + `repeat` *(preferred)* + +`components.state` is a single object; a **field** inside it holds the array and +declares `type: 'fieldGroup'` + a `repeat` descriptor. Repetition is fully +config-driven and local — no allowlist, no reducer actions. You can have several +independent repeated groups alongside ordinary scalar fields, and it's all +handled in [`../FieldGenerator/FieldGenerator.tsx`](../FieldGenerator/FieldGenerator.tsx). + +```ts +// BlockOverviewWithCarousel.editorconfig.ts — state is an object; +// the `elements` field repeats. +components: { + state: { + title: '', + titleType: '', + elements: [INITIAL_ELEMENT_STATE()], // the repeated array lives in a field + }, + fields: { + title: TEXT_FIELD({ /* … */ }), // scalar field, alongside… + elements: { + label: 'Content item', + type: 'fieldGroup', + fields: { /* mediaItem, image, title, … repeated per element */ }, + repeat: { + defaultState: INITIAL_ELEMENT_STATE(), + addButtonLabel: 'voeg object toe', + deleteButtonLabel: 'verwijder object', + }, + // optional: min, max + }, + }, +} +``` + +> A plain (non-group) field can also repeat: give a `ContentBlockField` a +> `repeat` descriptor to repeat a single editor (e.g. a list of text values). +> This is the same B code path in `FieldGenerator` (`handleField`). + +### A vs B at a glance + +| | A: `components.state` is an array | B: `fieldGroup` + `repeat` | +|---|---|---| +| What repeats | the whole `components` config | one field(-group) inside the config | +| Driven by | `REPEATABLE_CONTENT_BLOCKS` allowlist + reducer actions + `limits` | config only (`repeat`, `min`/`max`) | +| Repeatable sets per block | exactly one | any number | +| Scalar fields alongside | no | yes | +| Stored shape | `components: [ {…}, {…} ]` | `components: { …, elements: [ {…} ] }` | +| Add/remove code | 4-component prop chain + 3 reducer actions | none (local `handleChange`) | +| Render prop | array wrapped as `elements` | the state object, spread | + +### Which do I use? + +Use **B** for every new block. Mechanism A is kept only for the existing blocks +that already rely on it — **do not extend it**: don't add new block types to +`REPEATABLE_CONTENT_BLOCKS`, and don't reach for `components.limits` or the +`ADD_/REMOVE_COMPONENTS_STATE` actions in new work. (They're marked `@deprecated` +for this reason.) + +We deliberately have **not** migrated the legacy blocks to B: their state is +already persisted in the DB as bare arrays, so a switch would require a data +migration of all saved content pages plus prop changes in every affected preview +component. + +--- + +## Key files + +- [`../ContentBlockForm/ContentBlockForm.tsx`](../ContentBlockForm/ContentBlockForm.tsx) — + renders the block form; the A branch is `Array.isArray(formGroup.state)`. +- [`../FieldGenerator/FieldGenerator.tsx`](../FieldGenerator/FieldGenerator.tsx) — + the B branch: `fieldGroup.repeat` (group) and `field.repeat` (single field). +- [`../../types/content-block.types.ts`](../../types/content-block.types.ts) — + `ContentBlockConfig`, `ContentBlockComponentsConfig`, `ContentBlockField`, + `ContentBlockFieldGroup`. +- [`../../helpers/validate-content-block-config.ts`](../../helpers/validate-content-block-config.ts) — + validation branches on `Array.isArray(state)` (mechanism A); mechanism B + validates through the normal per-field path. diff --git a/ui/src/react-admin/modules/content-page/helpers/validate-content-block-config.test.ts b/ui/src/react-admin/modules/content-page/helpers/validate-content-block-config.test.ts new file mode 100644 index 000000000..61f268c11 --- /dev/null +++ b/ui/src/react-admin/modules/content-page/helpers/validate-content-block-config.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from 'vitest'; +import type { + ContentBlockConfig, + ContentBlockFieldGroupErrors, +} from '../types/content-block.types'; +import { validateContentBlockConfig } from './validate-content-block-config'; + +const required = (message: string) => (value: string) => (value ? [] : [message]); + +// A block config modelled on the mechanism-B blocks (e.g. BlockOverviewWithCarousel): +// a scalar `title` field alongside a repeated `elements` field group whose inner +// fields carry the validators. `title` intentionally appears at both levels to guard +// against error-key collisions. +const makeFields = () => ({ + title: { + editorType: 'TextInput' as never, + validator: required('block title required'), + }, + elements: { + type: 'fieldGroup' as const, + label: 'Content item', + repeat: { defaultState: {} }, + fields: { + image: { + editorType: 'FileUpload' as never, + validator: required('image required'), + }, + title: { + editorType: 'TextInput' as never, + validator: required('item title required'), + }, + itemDisplay: { + editorType: 'Select' as never, + validator: required('display required'), + }, + }, + }, +}); + +const makeConfig = (state: unknown): ContentBlockConfig => + ({ components: { state, fields: makeFields() } }) as unknown as ContentBlockConfig; + +describe('validateContentBlockConfig - field groups (mechanism B)', () => { + it('reports an error per invalid inner field, keyed by element index', () => { + const state = { + title: 'ok', + elements: [ + { image: 'img.jpg', title: 'Item 1', itemDisplay: '16:9' }, // valid + { image: '', title: '', itemDisplay: '' }, // all missing + ], + }; + + const errors = validateContentBlockConfig({}, makeConfig(state), makeFields(), state as never); + + const groupErrors = errors.elements as ContentBlockFieldGroupErrors; + expect(groupErrors[0]).toEqual({}); // first element is valid + expect(groupErrors[1]).toEqual({ + image: ['image required'], + title: ['item title required'], + itemDisplay: ['display required'], + }); + }); + + it('does not collide the element title with the block-level title', () => { + const state = { + title: '', // block-level invalid + elements: [{ image: 'img.jpg', title: '', itemDisplay: '16:9' }], // element title invalid + }; + + const errors = validateContentBlockConfig({}, makeConfig(state), makeFields(), state as never); + + expect(errors.title).toEqual(['block title required']); // scalar field untouched + expect((errors.elements as ContentBlockFieldGroupErrors)[0]).toEqual({ + title: ['item title required'], + }); + }); + + it('clears the group key when every element becomes valid', () => { + const state = { + title: 'ok', + elements: [{ image: 'img.jpg', title: 'Item 1', itemDisplay: '16:9' }], + }; + + const errors = validateContentBlockConfig( + { elements: [{ image: ['image required'] }] }, + makeConfig(state), + makeFields(), + state as never + ); + + expect(errors.elements).toBeUndefined(); + }); +}); diff --git a/ui/src/react-admin/modules/content-page/helpers/validate-content-block-config.ts b/ui/src/react-admin/modules/content-page/helpers/validate-content-block-config.ts index d48d21778..8d21729ba 100644 --- a/ui/src/react-admin/modules/content-page/helpers/validate-content-block-config.ts +++ b/ui/src/react-admin/modules/content-page/helpers/validate-content-block-config.ts @@ -5,26 +5,94 @@ import type { ContentBlockConfig, ContentBlockErrors, ContentBlockField, + ContentBlockFieldGroup, + ContentBlockFieldGroupErrors, } from '../types/content-block.types'; +/** + * Validate the inner fields of a repeated field group (mechanism B, e.g. `elements`). + * The group itself has no validator; the validators live on its inner fields, and its + * state is an array with one object per repeated element. Errors are namespaced under + * the group key to avoid collisions with equally named top-level fields. + */ +export function validateFieldGroup( + errors: ContentBlockErrors, + config: ContentBlockConfig, + groupKey: string, + groupFields: Record, + // biome-ignore lint/suspicious/noExplicitAny: element state shape varies per block + groupState: any[] +): ContentBlockErrors { + const groupErrors: ContentBlockFieldGroupErrors = []; + let hasError = false; + + groupState.forEach((elementState, elementIndex: number) => { + const elementErrors: Record = {}; + + Object.keys(groupFields).forEach((innerKey) => { + const innerField = groupFields[innerKey]; + const validator = innerField.validator; + const isVisible = innerField.isVisible; + + if (validator && isFunction(validator)) { + if (!isVisible || isVisible(config, elementState)) { + const messages = validator(elementState?.[innerKey]); + if (messages.length) { + elementErrors[innerKey] = messages; + hasError = true; + } + } + } + }); + + groupErrors[elementIndex] = elementErrors; + }); + + if (hasError) { + return { + ...errors, + [groupKey]: groupErrors, + }; + } + + // No more errors in this group, clear its property from the error object + const updatedErrors = { ...errors }; + delete updatedErrors[groupKey]; + return updatedErrors; +} + export function validateContentBlockConfig( errors: ContentBlockErrors, config: ContentBlockConfig, - fields: Record, + fields: Record, state: ContentBlockComponentState ) { let newErrors = { ...errors }; - const keysToValidate = Object.keys(fields).filter((key) => fields[key].validator); + Object.keys(fields).forEach((key) => { + const field = fields[key]; + + // Mechanism B: repeated field group (e.g. `elements`). Validators live on the + // inner fields, so validate each element against the group's inner fields. + if ((field as ContentBlockFieldGroup).type === 'fieldGroup') { + const groupFields = (field as ContentBlockFieldGroup).fields; + // biome-ignore lint/suspicious/noExplicitAny: state is a single object here (mechanism B) + const groupState = (state as any)?.[key]; + + if (Array.isArray(groupState)) { + newErrors = validateFieldGroup(newErrors, config, key, groupFields, groupState); + } + return; + } - keysToValidate.forEach((key) => { - const validator = fields[key].validator; - const isVisible = fields[key].isVisible; + const validator = (field as ContentBlockField).validator; + const isVisible = (field as ContentBlockField).isVisible; if (validator && isFunction(validator)) { if (Array.isArray(state) && state.length > 0) { + // Validate repeatable fields inside a block state.forEach((singleState: ContentBlockComponentState, stateIndex: number) => { - if (!(isVisible && !isVisible(config, singleState))) { + if (!isVisible || isVisible(config, singleState)) { newErrors = validateContentBlockField( key, validator, @@ -35,7 +103,8 @@ export function validateContentBlockConfig( } }); } else if (Object.hasOwn(state, key)) { - if (!(isVisible && !isVisible(config, state))) { + // Validate the block fields itself + if (!isVisible || isVisible(config, state)) { newErrors = validateContentBlockField( key, validator, diff --git a/ui/src/react-admin/modules/content-page/types/content-block.types.ts b/ui/src/react-admin/modules/content-page/types/content-block.types.ts index 534dfefaf..5135bb255 100644 --- a/ui/src/react-admin/modules/content-page/types/content-block.types.ts +++ b/ui/src/react-admin/modules/content-page/types/content-block.types.ts @@ -127,9 +127,21 @@ export interface PaddingFieldState { // CONTENT BLOCK CONFIG +// Errors for a repeated field group (mechanism B, e.g. `elements`): one entry per +// element index, each mapping an inner field key to its error messages. +export type ContentBlockFieldGroupErrors = Record[]; + // if 1 block, errors is a string[]. If multiple, it is a string[] index by their stateIndex, so string[][]. -export type ContentBlockErrors = { [key: string]: (string | string[])[] }; +// For a repeated field group the value is a ContentBlockFieldGroupErrors instead. +export type ContentBlockErrors = { + [key: string]: (string | string[])[] | ContentBlockFieldGroupErrors; +}; +/** + * @deprecated Legacy: bounds the array-valued `components.state` repetition mechanism (A). + * For new blocks use the `fieldGroup` + `repeat` mechanism (B) and put `min`/`max` on the + * `ContentBlockFieldGroup` instead. See `components/blocks/README.md`. + */ export interface ContentBlockComponentsLimits { min?: number; max?: number; diff --git a/ui/src/react-admin/modules/content-page/types/content-pages.types.ts b/ui/src/react-admin/modules/content-page/types/content-pages.types.ts index f8bd836be..a9ea73c28 100644 --- a/ui/src/react-admin/modules/content-page/types/content-pages.types.ts +++ b/ui/src/react-admin/modules/content-page/types/content-pages.types.ts @@ -142,8 +142,18 @@ export enum ContentEditActionType { ADD_CONTENT_BLOCK_CONFIG = '@@admin-content-edit/ADD_CONTENT_BLOCK_CONFIG', REMOVE_CONTENT_BLOCK_CONFIG = '@@admin-content-edit/REMOVE_CONTENT_BLOCK_CONFIG', REORDER_CONTENT_BLOCK_CONFIG = '@@admin-content-edit/REORDER_CONTENT_BLOCK_CONFIG', + /** + * @deprecated Legacy: only used by the array-valued `components.state` repetition + * mechanism (A). For new blocks use the `fieldGroup` + `repeat` mechanism (B), which + * needs no dedicated add/remove actions. See `components/blocks/README.md`. + */ ADD_COMPONENTS_STATE = '@@admin-content-edit/ADD_COMPONENTS_STATE', SET_COMPONENTS_STATE = '@@admin-content-edit/SET_COMPONENTS_STATE', + /** + * @deprecated Legacy: only used by the array-valued `components.state` repetition + * mechanism (A). For new blocks use the `fieldGroup` + `repeat` mechanism (B), which + * needs no dedicated add/remove actions. See `components/blocks/README.md`. + */ REMOVE_COMPONENTS_STATE = '@@admin-content-edit/REMOVE_COMPONENTS_STATE', SET_BLOCK_STATE = '@@admin-content-edit/SET_BLOCK_STATE', SET_CONTENT_BLOCK_ERROR = '@@admin-content-edit/SET_CONTENT_BLOCK_ERROR', diff --git a/ui/src/react-admin/modules/shared/helpers/validation.ts b/ui/src/react-admin/modules/shared/helpers/validation.ts index 4deff10d8..16648e4e8 100644 --- a/ui/src/react-admin/modules/shared/helpers/validation.ts +++ b/ui/src/react-admin/modules/shared/helpers/validation.ts @@ -20,7 +20,7 @@ export const validateContentBlockField = ( if (errorArray.length) { if (typeof stateIndex === 'number') { - const errorsByKey = [...(oldErrors[fieldKey] || [])]; + const errorsByKey = [...((oldErrors[fieldKey] as (string | string[])[]) || [])]; errorsByKey[stateIndex] = errorArray; return {