From 1050dbb26470ae3d6511edf0e7e7f54dc2dc5bcd Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Sat, 24 Oct 2026 01:15:28 +0600 Subject: [PATCH 1/3] fix(blocks): stop V3 blocks marking post as changed on load - Set hasAcfError only when it changes, as a non persistent update that sends only that key. - Hash a copy of the attributes so the store object is not changed. - Skip writing form data back while the form still has its loaded values, compared with the form's own first serialization. - Count native input and change events in the block form as user interaction. Stored block data is keyed by field name and the form serializes by field key, so the first form serialization always looked like a change and was written back, marking the post as dirty with no edit. Fixes #556 --- .../js/pro/blocks-v3/components/block-edit.js | 95 ++++++--- .../js/pro/blocks-v3/components/block-form.js | 20 +- tests/js/blocks-v3/block-edit.test.js | 190 +++++++++++++++++- tests/js/blocks-v3/block-form.test.js | 104 ++++++++++ 4 files changed, 370 insertions(+), 39 deletions(-) diff --git a/assets/src/js/pro/blocks-v3/components/block-edit.js b/assets/src/js/pro/blocks-v3/components/block-edit.js index bf373a11..6327a3ec 100644 --- a/assets/src/js/pro/blocks-v3/components/block-edit.js +++ b/assets/src/js/pro/blocks-v3/components/block-edit.js @@ -169,6 +169,22 @@ function deserializeGoogleMapValues( $form, serializedData, clientId ) { } ); } +/** + * Serializes the block form into the JSON string stored as theSerializedAcfData. + * + * @param {Object} $form The block form jQuery element. + * @param {string} clientId The block client ID. + * @return {string|null} JSON string, or null when the form has no data. + */ +function serializeBlockForm( $form, clientId ) { + const serializedData = acf.serialize( $form, `acf-block_${ clientId }` ); + if ( ! serializedData ) { + return null; + } + deserializeGoogleMapValues( $form, serializedData, clientId ); + return JSON.stringify( serializedData ); +} + /** * Main BlockEdit component wrapper * Manages block data fetching and initial setup @@ -264,6 +280,27 @@ export const BlockEdit = ( props ) => { const acfFormRef = useRef( null ); const previewRef = useRef( null ); const debounceRef = useRef( null ); + // Serialized form data before the user changed anything. + const initialFormDataRef = useRef( null ); + const hasWrittenFormDataRef = useRef( false ); + + /** + * Stores the form's current data as its loaded state, until the first real edit is written + * + * @param {jQuery} $form - The block form + */ + const captureInitialFormData = useCallback( + ( $form ) => { + if ( hasWrittenFormDataRef.current ) { + return; + } + const serializedData = serializeBlockForm( $form, clientId ); + if ( serializedData ) { + initialFormDataRef.current = serializedData; + } + }, + [ clientId ] + ); // Initialize acf.blockEdit namespace for jsx-parser to use if ( ! acf.blockEdit ) { @@ -414,9 +451,10 @@ export const BlockEdit = ( props ) => { * @returns {string} - MD5 hash of serialized attributes */ function generateAttributesHash( attrs, ctx ) { - delete attrs.hasAcfError; - attrs._acf_context = sortObjectKeys( ctx ); - return md5( JSON.stringify( sortObjectKeys( attrs ) ) ); + // Work on a copy, attrs can be the block editor store's own object. + const { hasAcfError, ...hashAttrs } = attrs; + hashAttrs._acf_context = sortObjectKeys( ctx ); + return md5( JSON.stringify( sortObjectKeys( hashAttrs ) ) ); } /** @@ -579,10 +617,18 @@ export const BlockEdit = ( props ) => { // Update hasAcfError attribute based on validation errors useEffect( () => { - setAttributes( - validationErrors ? { hasAcfError: true } : { hasAcfError: false } - ); - }, [ validationErrors, setAttributes ] ); + const hasError = !! validationErrors; + if ( hasError === !! attributes.hasAcfError ) { + return; + } + + // hasAcfError is not saved with the block, so this must not mark the post as changed. + // Send only this key, so the next real edit is not merged into this change. + wp.data + .dispatch( 'core/block-editor' ) + .__unstableMarkNextChangeAsNotPersistent(); + setAttributes( { hasAcfError: hasError } ); + }, [ validationErrors, attributes.hasAcfError, setAttributes ] ); // Listen for validation error events from other blocks useEffect( () => { @@ -631,9 +677,13 @@ export const BlockEdit = ( props ) => { } ); } + // Stored data is keyed by field name and the form by field key, so on load + // the form is compared with its own first state instead. if ( theSerializedAcfData === - JSON.stringify( attributesWithoutError.data ) + JSON.stringify( attributesWithoutError.data ) || + ( ! hasWrittenFormDataRef.current && + theSerializedAcfData === initialFormDataRef.current ) ) { return void fetchBlockData( { theAttributes: attributesWithoutError, @@ -643,6 +693,8 @@ export const BlockEdit = ( props ) => { } ); } + hasWrittenFormDataRef.current = true; + // Use original attributes (with hasAcfError) when updating const updatedAttributes = { ...attributes, @@ -714,6 +766,7 @@ export const BlockEdit = ( props ) => { showValidationErrors={ showValidationErrors } theSerializedAcfData={ theSerializedAcfData } setTheSerializedAcfData={ setTheSerializedAcfData } + captureInitialFormData={ captureInitialFormData } acfFormRef={ acfFormRef } blockFormHtml={ blockFormHtml } blockPreviewHtml={ blockPreviewHtml } @@ -770,6 +823,7 @@ function BlockEditInner( props ) { showValidationErrors, theSerializedAcfData, setTheSerializedAcfData, + captureInitialFormData, acfFormRef, blockFormHtml, blockPreviewHtml, @@ -1005,14 +1059,12 @@ function BlockEditInner( props ) { setContentEditableChangeInProgress( false ); fieldInput.value = content; - const $form = $( acfFormRef?.current ); - const serializedData = acf.serialize( - $form, - `acf-block_${ clientId }` + const serializedData = serializeBlockForm( + $( acfFormRef?.current ), + clientId ); if ( serializedData ) { - deserializeGoogleMapValues( $form, serializedData, clientId ); - setTheSerializedAcfData( JSON.stringify( serializedData ) ); + setTheSerializedAcfData( serializedData ); } else { setUserHasInteractedWithForm( false ); } @@ -1251,20 +1303,15 @@ function BlockEditInner( props ) { $={ $ } clientId={ clientId } blockFormHtml={ blockFormHtml } + onMount={ captureInitialFormData } + onUntouchedChange={ captureInitialFormData } onChange={ function ( $form ) { - const serializedData = acf.serialize( + const serializedData = serializeBlockForm( $form, - `acf-block_${ clientId }` + clientId ); if ( serializedData ) { - deserializeGoogleMapValues( - $form, - serializedData, - clientId - ); - setTheSerializedAcfData( - JSON.stringify( serializedData ) - ); + setTheSerializedAcfData( serializedData ); } } } validationErrors={ validationErrors } diff --git a/assets/src/js/pro/blocks-v3/components/block-form.js b/assets/src/js/pro/blocks-v3/components/block-form.js index f5b22527..444155aa 100644 --- a/assets/src/js/pro/blocks-v3/components/block-form.js +++ b/assets/src/js/pro/blocks-v3/components/block-form.js @@ -16,6 +16,7 @@ import { lockPostSaving, unlockPostSaving } from '../utils/post-locking'; * @param {string} props.blockFormHtml - HTML markup for the ACF form * @param {Function} props.onMount - Callback when the form mounts * @param {Function} props.onChange - Callback when form data changes + * @param {Function} props.onUntouchedChange - Callback when the form changes before any user interaction * @param {Array} props.validationErrors - Array of validation error objects * @param {boolean} props.showValidationErrors - Whether to display validation errors * @param {React.RefObject} props.acfFormRef - Ref to the form container element @@ -29,6 +30,7 @@ export const BlockForm = ( { blockFormHtml, onMount, onChange, + onUntouchedChange, validationErrors, showValidationErrors, acfFormRef, @@ -39,17 +41,24 @@ export const BlockForm = ( { const [ formHtml, setFormHtml ] = useState( blockFormHtml ); const [ pendingChange, setPendingChange ] = useState( false ); const debounceTimer = useRef( null ); + const reportedUntouchedChange = useRef( null ); const [ userInteracted, setUserInteracted ] = useState( false ); const [ hasRemounted, setHasRemounted ] = useState( false ); // Trigger onChange when there's a pending change useEffect( () => { - if ( - pendingChange && - ( userHasInteractedWithForm || userInteracted ) - ) { + if ( ! pendingChange ) { + return; + } + + if ( userHasInteractedWithForm || userInteracted ) { onChange( pendingChange ); setPendingChange( false ); + } else if ( reportedUntouchedChange.current !== pendingChange ) { + // No interaction yet, so this is the form setting itself up. + // Report it once, not on every render, so a later edit is never taken as setup. + reportedUntouchedChange.current = pendingChange; + onUntouchedChange?.( pendingChange ); } }, [ pendingChange, @@ -57,6 +66,7 @@ export const BlockForm = ( { userInteracted, setPendingChange, onChange, + onUntouchedChange, ] ); // Update form HTML when blockFormHtml prop changes @@ -177,6 +187,8 @@ export const BlockForm = ( { } const handleChange = () => { + // Edits like a context menu paste fire no click or keydown first. + setUserInteracted( true ); onChange( $form ); }; diff --git a/tests/js/blocks-v3/block-edit.test.js b/tests/js/blocks-v3/block-edit.test.js index 1b866922..cca50c53 100644 --- a/tests/js/blocks-v3/block-edit.test.js +++ b/tests/js/blocks-v3/block-edit.test.js @@ -8,7 +8,7 @@ /* global acf, wp, MouseEvent */ import React from 'react'; -import { render, screen, act, waitFor } from '@testing-library/react'; +import { render, screen, act } from '@testing-library/react'; import '@testing-library/jest-dom'; // Mock md5 @@ -40,14 +40,19 @@ jest.mock( } ) ); +// Latest props passed to BlockForm, so tests can call its callbacks. +let mockBlockFormProps = null; jest.mock( '../../../assets/src/js/pro/blocks-v3/components/block-form', () => ( { - BlockForm: ( { blockFormHtml } ) => ( -
- { blockFormHtml ? 'Form loaded' : 'No form' } -
- ), + BlockForm: ( props ) => { + mockBlockFormProps = props; + return ( +
+ { props.blockFormHtml ? 'Form loaded' : 'No form' } +
+ ); + }, } ) ); @@ -225,11 +230,14 @@ global.acf = { }; // Mock wp.data +const mockMarkNextChangeAsNotPersistent = jest.fn(); global.wp = { data: { dispatch: jest.fn( () => ( { lockPostSaving: jest.fn(), unlockPostSaving: jest.fn(), + __unstableMarkNextChangeAsNotPersistent: + mockMarkNextChangeAsNotPersistent, } ) ), select: jest.fn( () => ( { getBlockParents: jest.fn( () => [] ), @@ -339,7 +347,7 @@ describe( 'BlockEdit Component', () => { expect( typeof acf.blockEdit.getBlockFieldInfo ).toBe( 'function' ); } ); - test( 'calls setAttributes when validation errors change', async () => { + test( 'does not set hasAcfError on load when there are no errors', async () => { const setAttributes = jest.fn(); await act( async () => { @@ -350,12 +358,172 @@ describe( 'BlockEdit Component', () => { /> ); } ); + await act( async () => { + jest.runAllTimers(); + } ); + + expect( setAttributes ).not.toHaveBeenCalled(); + expect( mockMarkNextChangeAsNotPersistent ).not.toHaveBeenCalled(); + } ); + + test( 'sets hasAcfError as a non persistent change with only that key', async () => { + const setAttributes = jest.fn(); + mockAjax.mockImplementationOnce( () => ( { + done( callback ) { + callback( { + data: { + form: '
Form HTML
', + preview: '
Preview HTML
', + validation: { + valid: false, + errors: [ { input: 'x', message: 'Required' } ], + }, + }, + } ); + return this; + }, + fail() { + return this; + }, + abort: jest.fn(), + } ) ); + + await act( async () => { + render( + + ); + } ); + await act( async () => { + jest.advanceTimersByTime( 250 ); + } ); + + expect( setAttributes ).toHaveBeenCalledWith( { hasAcfError: true } ); + expect( mockMarkNextChangeAsNotPersistent ).toHaveBeenCalled(); + } ); + + test( 'does not modify the attributes object when hashing', async () => { + const attributes = { + name: 'acf/test-block', + data: { field_1: 'value_1' }, + hasAcfError: false, + }; + + await act( async () => { + render( + + ); + } ); + await act( async () => { + jest.runAllTimers(); + } ); + + expect( attributes ).not.toHaveProperty( '_acf_context' ); + expect( attributes ).toHaveProperty( 'hasAcfError', false ); + } ); + + describe( 'writing form data back to the block', () => { + // Stored data is keyed by field name, the form serializes by field key. + const storedAttributes = { + name: 'acf/test-block', + data: { title: 'Hello', _title: 'field_1' }, + }; + const $form = { find: () => ( { each: () => {} } ) }; + + async function renderWithForm( setAttributes ) { + await act( async () => { + render( + + ); + } ); + await act( async () => { + jest.advanceTimersByTime( 250 ); + } ); + + // Mount the form, the same way the first inline editing click does. + const onInlineEditingElementSelected = + acf.parseJSX.mock.calls.at( -1 )[ 1 ]; + await act( async () => { + onInlineEditingElementSelected( 'inline-uid' ); + } ); + } + + async function sendFormChange( values ) { + acf.serialize.mockReturnValue( values ); + await act( async () => { + mockBlockFormProps.onChange( $form ); + } ); + await act( async () => { + jest.advanceTimersByTime( 250 ); + } ); + } + + afterEach( () => { + acf.serialize.mockReturnValue( { field_1: 'value_1' } ); + } ); + + test( 'skips the write when the form still has its loaded values', async () => { + const setAttributes = jest.fn(); + await renderWithForm( setAttributes ); + + acf.serialize.mockReturnValue( { field_1: 'Hello' } ); + await act( async () => { + mockBlockFormProps.onMount( $form ); + } ); + + await sendFormChange( { field_1: 'Hello' } ); + + expect( setAttributes ).not.toHaveBeenCalled(); + } ); + + test( 'uses the latest untouched form state as the loaded values', async () => { + const setAttributes = jest.fn(); + await renderWithForm( setAttributes ); + + acf.serialize.mockReturnValue( { field_1: 'Hello' } ); + await act( async () => { + mockBlockFormProps.onMount( $form ); + } ); + + // A field changed its own markup while setting up. + acf.serialize.mockReturnValue( { field_1: 'Hello\r\n' } ); + await act( async () => { + mockBlockFormProps.onUntouchedChange( $form ); + } ); + + await sendFormChange( { field_1: 'Hello\r\n' } ); + + expect( setAttributes ).not.toHaveBeenCalled(); + } ); + + test( 'writes a real edit, and later edits back to the loaded values', async () => { + const setAttributes = jest.fn(); + await renderWithForm( setAttributes ); + + acf.serialize.mockReturnValue( { field_1: 'Hello' } ); + await act( async () => { + mockBlockFormProps.onMount( $form ); + } ); + + await sendFormChange( { field_1: 'Hello world' } ); + + expect( setAttributes ).toHaveBeenLastCalledWith( + expect.objectContaining( { + data: { field_1: 'Hello world' }, + } ) + ); + + await sendFormChange( { field_1: 'Hello' } ); - // setAttributes should be called with hasAcfError - await waitFor( () => { - expect( setAttributes ).toHaveBeenCalledWith( + expect( setAttributes ).toHaveBeenLastCalledWith( expect.objectContaining( { - hasAcfError: expect.any( Boolean ), + data: { field_1: 'Hello' }, } ) ); } ); diff --git a/tests/js/blocks-v3/block-form.test.js b/tests/js/blocks-v3/block-form.test.js index bec300b9..f0c12b58 100644 --- a/tests/js/blocks-v3/block-form.test.js +++ b/tests/js/blocks-v3/block-form.test.js @@ -506,3 +506,107 @@ describe( 'BlockForm Attribute Handling', () => { ).not.toThrow(); } ); } ); + +describe( 'BlockForm Changes Before Interaction', () => { + const renderForm = ( props ) => { + const acfFormRef = { current: null }; + render( + + ); + return acfFormRef; + }; + + // Change the form markup and let the 300ms debounce run. + const mutateForm = async ( acfFormRef ) => { + await act( async () => { + acfFormRef.current.appendChild( document.createElement( 'span' ) ); + } ); + await act( async () => { + jest.advanceTimersByTime( 350 ); + } ); + }; + + beforeEach( () => { + jest.clearAllMocks(); + jest.useFakeTimers(); + } ); + + afterEach( () => { + jest.useRealTimers(); + } ); + + test( 'reports form changes before any interaction as untouched', async () => { + const onChange = jest.fn(); + const onUntouchedChange = jest.fn(); + const acfFormRef = renderForm( { onChange, onUntouchedChange } ); + + await mutateForm( acfFormRef ); + + expect( onUntouchedChange ).toHaveBeenCalled(); + expect( onChange ).not.toHaveBeenCalled(); + } ); + + test( 'reports an untouched change only once across renders', async () => { + const onUntouchedChange = jest.fn(); + const acfFormRef = { current: null }; + const props = { + $: mockJQuery, + clientId: 'test-client-id', + blockFormHtml: + '
', + onMount: jest.fn(), + onUntouchedChange, + validationErrors: null, + showValidationErrors: false, + acfFormRef, + userHasInteractedWithForm: false, + attributes: { name: 'acf/test-block', data: {} }, + hideFieldsInSidebar: false, + }; + const { rerender } = render( + + ); + + await mutateForm( acfFormRef ); + expect( onUntouchedChange ).toHaveBeenCalledTimes( 1 ); + + // A parent render passes a new onChange function each time. + await act( async () => { + rerender( ); + } ); + + expect( onUntouchedChange ).toHaveBeenCalledTimes( 1 ); + } ); + + test( 'treats a native input event as interaction', async () => { + const onChange = jest.fn(); + const onUntouchedChange = jest.fn(); + const acfFormRef = renderForm( { onChange, onUntouchedChange } ); + + await act( async () => { + acfFormRef.current + .querySelector( 'input' ) + .dispatchEvent( new Event( 'input' ) ); + } ); + + expect( onChange ).toHaveBeenCalledTimes( 1 ); + + onUntouchedChange.mockClear(); + await mutateForm( acfFormRef ); + + expect( onChange ).toHaveBeenCalledTimes( 2 ); + expect( onUntouchedChange ).not.toHaveBeenCalled(); + } ); +} ); From ef6f31e22e138f94a84bb5776529a4b2de18ebca Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Sat, 24 Oct 2026 01:41:50 +0600 Subject: [PATCH 2/3] fix(blocks): validate form values when the write is skipped A new block has no stored data, so when the form still had its loaded values the fetch sent no field data and the server treated it as a first preview, skipping validation. Required fields on new blocks then no longer blocked publishing. Send the form values with the fetch, without storing them, so the server validates them like before. --- assets/src/js/pro/blocks-v3/components/block-edit.js | 7 ++++++- tests/js/blocks-v3/block-edit.test.js | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/assets/src/js/pro/blocks-v3/components/block-edit.js b/assets/src/js/pro/blocks-v3/components/block-edit.js index 6327a3ec..7830c3ff 100644 --- a/assets/src/js/pro/blocks-v3/components/block-edit.js +++ b/assets/src/js/pro/blocks-v3/components/block-edit.js @@ -685,8 +685,13 @@ export const BlockEdit = ( props ) => { ( ! hasWrittenFormDataRef.current && theSerializedAcfData === initialFormDataRef.current ) ) { + // Send the form values even when they are not stored, so the server + // validates them. A new block has no stored data to validate. return void fetchBlockData( { - theAttributes: attributesWithoutError, + theAttributes: { + ...attributesWithoutError, + data: parsedData, + }, theClientId: clientId, theContext: context, isSelected: isSelected, diff --git a/tests/js/blocks-v3/block-edit.test.js b/tests/js/blocks-v3/block-edit.test.js index cca50c53..4e48a45d 100644 --- a/tests/js/blocks-v3/block-edit.test.js +++ b/tests/js/blocks-v3/block-edit.test.js @@ -477,9 +477,14 @@ describe( 'BlockEdit Component', () => { mockBlockFormProps.onMount( $form ); } ); + mockAjax.mockClear(); await sendFormChange( { field_1: 'Hello' } ); expect( setAttributes ).not.toHaveBeenCalled(); + + // The form values still go to the server, so it can validate them. + const { block } = mockAjax.mock.calls.at( -1 )[ 0 ].data; + expect( JSON.parse( block ).data ).toEqual( { field_1: 'Hello' } ); } ); test( 'uses the latest untouched form state as the loaded values', async () => { From 83b2e80c4963f9b61e2eed6dbb32a1c0d0f70ae1 Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Tue, 27 Oct 2026 05:11:59 +0600 Subject: [PATCH 3/3] fix(blocks): keep undo and redo working for V3 block fields - When the block data changes from outside the form (undo, redo or a script), load a new form for that data and re-mount it, instead of writing the old form values back. - Compare stored data with the form by value, mapping field names to keys through the "_name" entries or the block's fields. - Keep an edit typed while the new form is loading, and put focus back in the field that had it. - Only count edits made in a re-mounted form, and run the "unmount" action when the form goes away so fields like WYSIWYG clean up. - Write _acf_changed with undoIgnore so it does not add an empty undo step. The V3 form never followed the data attribute. After Ctrl+Z it wrote its own values back about 200 ms later, which undid the undo and cleared the redo stack. Fixes #577 --- assets/src/js/_acf-fields.js | 6 +- .../js/pro/blocks-v3/components/block-edit.js | 254 ++++++++++++++++-- .../js/pro/blocks-v3/components/block-form.js | 15 +- tests/js/blocks-v3/block-edit.test.js | 251 ++++++++++++++++- tests/js/blocks-v3/block-form.test.js | 62 +++++ 5 files changed, 561 insertions(+), 27 deletions(-) diff --git a/assets/src/js/_acf-fields.js b/assets/src/js/_acf-fields.js index 5ecef9aa..bc02ec58 100644 --- a/assets/src/js/_acf-fields.js +++ b/assets/src/js/_acf-fields.js @@ -403,7 +403,11 @@ if ( acf.isGutenbergPostEditor() ) { try { - wp.data.dispatch('core/editor').editPost({ meta: { _acf_changed: 1 } }); + wp.data.dispatch( 'core/editor' ).editPost( + { meta: { _acf_changed: 1 } }, + // Not a change to undo, so keep it out of the undo history. + { undoIgnore: true } + ); } catch ( error ) { console.log( 'ACF: Failed to update _acf_changed meta', error ); } diff --git a/assets/src/js/pro/blocks-v3/components/block-edit.js b/assets/src/js/pro/blocks-v3/components/block-edit.js index 7830c3ff..a710212a 100644 --- a/assets/src/js/pro/blocks-v3/components/block-edit.js +++ b/assets/src/js/pro/blocks-v3/components/block-edit.js @@ -185,6 +185,46 @@ function serializeBlockForm( $form, clientId ) { return JSON.stringify( serializedData ); } +/** + * Checks if the stored block data has the same values as the form data. + * + * Stored data can be keyed by field name (with "_name" entries holding the field key), + * the form data is always keyed by field key. Only top level fields are mapped. + * + * @param {Object|undefined} data The block's data attribute. + * @param {Object} formData The parsed form data. + * @param {Array|null} fields The block's fields, used to map names to keys. + * @return {boolean} True when both hold the same values. + */ +export function isSameBlockData( data, formData, fields ) { + if ( ! data || ! formData ) { + return false; + } + + const keyedData = {}; + Object.entries( data ).forEach( ( [ name, value ] ) => { + // Skip the "_name" entries, they only point to the field key. + if ( + name.startsWith( '_' ) && + typeof value === 'string' && + value.startsWith( 'field_' ) + ) { + return; + } + + const key = + data[ `_${ name }` ] || + fields?.find( ( field ) => field.name === name )?.key || + name; + keyedData[ key ] = value; + } ); + + return ( + JSON.stringify( sortObjectKeys( keyedData ) ) === + JSON.stringify( sortObjectKeys( formData ) ) + ); +} + /** * Main BlockEdit component wrapper * Manages block data fetching and initial setup @@ -276,6 +316,8 @@ export const BlockEdit = ( props ) => { const blockEditorInspectorSidebarOpen = useBlockEditorInspectorSidebarOpen(); const [ mountPortal, setMountPortal ] = useState( false ); + // Changing this re-mounts the form with the latest form HTML. + const [ formKey, setFormKey ] = useState( 0 ); const acfFormRef = useRef( null ); const previewRef = useRef( null ); @@ -283,6 +325,13 @@ export const BlockEdit = ( props ) => { // Serialized form data before the user changed anything. const initialFormDataRef = useRef( null ); const hasWrittenFormDataRef = useRef( false ); + // Form data and stored data as of the last sync, to tell form edits from outside changes. + const lastFormDataRef = useRef( null ); + const lastDataRef = useRef( JSON.stringify( attributes.data ?? null ) ); + // Set while the form waits for HTML matching the stored data, holds the focused input. + const formReloadRef = useRef( null ); + // Set when the new form HTML is in, until the form has mounted. + const reloadedFormRef = useRef( null ); /** * Stores the form's current data as its loaded state, until the first real edit is written @@ -302,6 +351,138 @@ export const BlockEdit = ( props ) => { [ clientId ] ); + /** + * Takes the form's current data as in sync with the stored data, so it is not written back + * + * @param {jQuery} $form - The block form + */ + const syncFormData = useCallback( + ( $form ) => { + const serializedData = serializeBlockForm( $form, clientId ); + lastFormDataRef.current = serializedData; + setTheSerializedAcfData( serializedData ); + }, + [ clientId ] + ); + + /** + * Runs when the form mounts. After a reload, takes the new form as the synced state + * and puts the focus back into the input that had it. + * + * @param {jQuery} $form - The block form + */ + const onFormMount = useCallback( + ( $form ) => { + captureInitialFormData( $form ); + + const reloaded = reloadedFormRef.current; + if ( ! reloaded ) { + return; + } + reloadedFormRef.current = null; + syncFormData( $form ); + + if ( ! reloaded.focus ) { + return; + } + // Only take the focus back when it was lost with the old form. + const doc = $form[ 0 ].ownerDocument; + if ( doc.activeElement && doc.activeElement !== doc.body ) { + return; + } + const input = [ + ...$form[ 0 ].querySelectorAll( 'input, textarea, select' ), + ].find( ( element ) => element.name === reloaded.focus.name ); + if ( ! input ) { + return; + } + input.focus(); + try { + input.setSelectionRange( + reloaded.focus.start, + reloaded.focus.end + ); + } catch ( err ) {} + }, + [ captureInitialFormData, syncFormData ] + ); + + /** + * Runs when the form changes before the user touched it. A reloaded form takes + * these changes (fields setting themselves up) as part of its synced state. + * + * @param {jQuery} $form - The block form + */ + const onFormUntouchedChange = useCallback( + ( $form ) => { + captureInitialFormData( $form ); + if ( formKey > 0 ) { + syncFormData( $form ); + } + }, + [ captureInitialFormData, syncFormData, formKey ] + ); + + /** + * Checks if the serialized form holds a user edit, not its synced or loaded state + * + * @param {string} serializedData - Serialized form data + * @return {boolean} - True for a user edit + */ + function isFormEdit( serializedData ) { + return ( + serializedData !== lastFormDataRef.current && + ( hasWrittenFormDataRef.current || + serializedData !== initialFormDataRef.current ) + ); + } + + /** + * Handles form changes. An edit made while a reload is on its way wins over the reload. + * + * @param {jQuery} $form - The block form + */ + function onFormChange( $form ) { + const serializedData = serializeBlockForm( $form, clientId ); + if ( ! serializedData ) { + return; + } + if ( isFormEdit( serializedData ) ) { + formReloadRef.current = null; + } + setTheSerializedAcfData( serializedData ); + } + + /** + * Asks for form HTML that matches the stored data, keeping track of the focused input. + */ + function requestFormReload() { + const form = acfFormRef.current; + const active = form?.ownerDocument.activeElement; + formReloadRef.current = { + focus: + active && active.name && form.contains( active ) + ? { + name: active.name, + start: active.selectionStart, + end: active.selectionEnd, + } + : null, + }; + } + + /** + * Re-mounts the form with the fetched HTML, if a reload is still wanted. + */ + function applyFormReload() { + if ( ! formReloadRef.current ) { + return; + } + reloadedFormRef.current = formReloadRef.current; + formReloadRef.current = null; + setFormKey( ( key ) => key + 1 ); + } + // Initialize acf.blockEdit namespace for jsx-parser to use if ( ! acf.blockEdit ) { acf.blockEdit = {}; @@ -353,6 +534,7 @@ export const BlockEdit = ( props ) => { if ( preloadedData ) { handlePreloadedData( preloadedData ); + applyFormReload(); unlockPostSavingByName( 'acf-fetching-block' ); setIsFetchingBlock( false ); return; @@ -394,6 +576,7 @@ export const BlockEdit = ( props ) => { setIsFetchingBlock( false ); setBlockFormHtml( response.data.form ); + applyFormReload(); // Handle new field metadata for inline editing if ( response.data.fields ) { @@ -668,7 +851,25 @@ export const BlockEdit = ( props ) => { debounceRef.current = setTimeout( () => { const parsedData = JSON.parse( theSerializedAcfData ); + if ( isFormEdit( theSerializedAcfData ) ) { + // A form edit wins over a reload that is still on its way. + formReloadRef.current = null; + } + const formChanged = + theSerializedAcfData !== lastFormDataRef.current; + lastFormDataRef.current = theSerializedAcfData; + + const dataJson = JSON.stringify( + attributesWithoutError.data ?? null + ); + const dataChanged = dataJson !== lastDataRef.current; + lastDataRef.current = dataJson; + if ( ! parsedData ) { + // The form was not edited, but it still has to show the new data. + if ( dataChanged ) { + requestFormReload(); + } return void fetchBlockData( { theAttributes: attributesWithoutError, theClientId: clientId, @@ -677,11 +878,14 @@ export const BlockEdit = ( props ) => { } ); } - // Stored data is keyed by field name and the form by field key, so on load - // the form is compared with its own first state instead. + // On load the form is also compared with its own first state, as stored + // data may not have the "_name" entries needed to map names to keys. if ( - theSerializedAcfData === - JSON.stringify( attributesWithoutError.data ) || + isSameBlockData( + attributesWithoutError.data, + parsedData, + blockFieldInfo + ) || ( ! hasWrittenFormDataRef.current && theSerializedAcfData === initialFormDataRef.current ) ) { @@ -698,7 +902,23 @@ export const BlockEdit = ( props ) => { } ); } + // The form was not edited, so the data was changed from outside (undo, redo + // or another script). Writing the form back would cancel that change, so + // load a form for the new data instead. + if ( ! formChanged ) { + if ( dataChanged ) { + requestFormReload(); + } + return void fetchBlockData( { + theAttributes: attributesWithoutError, + theClientId: clientId, + theContext: context, + isSelected: isSelected, + } ); + } + hasWrittenFormDataRef.current = true; + lastDataRef.current = JSON.stringify( parsedData ); // Use original attributes (with hasAcfError) when updating const updatedAttributes = { @@ -771,7 +991,10 @@ export const BlockEdit = ( props ) => { showValidationErrors={ showValidationErrors } theSerializedAcfData={ theSerializedAcfData } setTheSerializedAcfData={ setTheSerializedAcfData } - captureInitialFormData={ captureInitialFormData } + onFormMount={ onFormMount } + onFormChange={ onFormChange } + onFormUntouchedChange={ onFormUntouchedChange } + formKey={ formKey } acfFormRef={ acfFormRef } blockFormHtml={ blockFormHtml } blockPreviewHtml={ blockPreviewHtml } @@ -828,7 +1051,10 @@ function BlockEditInner( props ) { showValidationErrors, theSerializedAcfData, setTheSerializedAcfData, - captureInitialFormData, + onFormMount, + onFormChange, + onFormUntouchedChange, + formKey, acfFormRef, blockFormHtml, blockPreviewHtml, @@ -1305,26 +1531,20 @@ function BlockEditInner( props ) { createPortal( <> 0 } attributes={ attributes } hideFieldsInSidebar={ ( blockType?.auto_inline_editing && diff --git a/assets/src/js/pro/blocks-v3/components/block-form.js b/assets/src/js/pro/blocks-v3/components/block-form.js index 444155aa..2379d6a0 100644 --- a/assets/src/js/pro/blocks-v3/components/block-form.js +++ b/assets/src/js/pro/blocks-v3/components/block-form.js @@ -21,6 +21,7 @@ import { lockPostSaving, unlockPostSaving } from '../utils/post-locking'; * @param {boolean} props.showValidationErrors - Whether to display validation errors * @param {React.RefObject} props.acfFormRef - Ref to the form container element * @param {boolean} props.userHasInteractedWithForm - Whether user has interacted with the form + * @param {boolean} props.isReloaded - Whether the form was re-mounted to show changed block data * @param {Object} props.attributes - Block attributes * @returns {JSX.Element} - Rendered form component */ @@ -35,6 +36,7 @@ export const BlockForm = ( { showValidationErrors, acfFormRef, userHasInteractedWithForm, + isReloaded, attributes, hideFieldsInSidebar, } ) => { @@ -51,7 +53,9 @@ export const BlockForm = ( { return; } - if ( userHasInteractedWithForm || userInteracted ) { + // A reloaded form only counts edits made in it, as the editor + // interaction that led to the reload (like Ctrl+Z) is not one. + if ( userInteracted || ( userHasInteractedWithForm && ! isReloaded ) ) { onChange( pendingChange ); setPendingChange( false ); } else if ( reportedUntouchedChange.current !== pendingChange ) { @@ -63,6 +67,7 @@ export const BlockForm = ( { }, [ pendingChange, userHasInteractedWithForm, + isReloaded, userInteracted, setPendingChange, onChange, @@ -299,6 +304,14 @@ export const BlockForm = ( { userHasInteractedWithForm, ] ); + // Let the fields clean up (like WYSIWYG editors) when the form goes away. + useEffect( () => { + const $form = $( acfFormRef.current ); + return () => { + acf.doAction( 'unmount', $form ); + }; + }, [] ); + return (
( { BlockForm: ( props ) => { mockBlockFormProps = props; + require( 'react' ).useEffect( () => { + mockBlockFormMounts++; + }, [] ); return (
{ props.blockFormHtml ? 'Form loaded' : 'No form' } @@ -247,7 +252,10 @@ global.wp = { }; // Import after all mocks -import { BlockEdit } from '../../../assets/src/js/pro/blocks-v3/components/block-edit'; +import { + BlockEdit, + isSameBlockData, +} from '../../../assets/src/js/pro/blocks-v3/components/block-edit'; describe( 'BlockEdit Component', () => { const defaultProps = { @@ -432,15 +440,40 @@ describe( 'BlockEdit Component', () => { }; const $form = { find: () => ( { each: () => {} } ) }; + // Sets the block's attributes from outside, like undo and redo do. + let setBlockAttributes = null; + + /** + * Keeps the attributes in state, so a write reaches BlockEdit like in the editor. + * + * @param {Object} props Props. + * @param {Function} props.setAttributes Spy for the attribute updates. + */ + function StatefulBlockEdit( { setAttributes } ) { + const [ attributes, setState ] = React.useState( storedAttributes ); + setBlockAttributes = setState; + const update = React.useCallback( + ( newAttributes ) => { + setAttributes( newAttributes ); + setState( ( previous ) => ( { + ...previous, + ...newAttributes, + } ) ); + }, + [ setAttributes ] + ); + return ( + + ); + } + async function renderWithForm( setAttributes ) { await act( async () => { - render( - - ); + render( ); } ); await act( async () => { jest.advanceTimersByTime( 250 ); @@ -464,6 +497,25 @@ describe( 'BlockEdit Component', () => { } ); } + async function changeDataFromOutside( data ) { + await act( async () => { + setBlockAttributes( ( previous ) => ( { ...previous, data } ) ); + } ); + await act( async () => { + jest.advanceTimersByTime( 250 ); + } ); + } + + async function mountReloadedForm( values ) { + acf.serialize.mockReturnValue( values ); + await act( async () => { + mockBlockFormProps.onMount( $form ); + } ); + await act( async () => { + jest.advanceTimersByTime( 250 ); + } ); + } + afterEach( () => { acf.serialize.mockReturnValue( { field_1: 'value_1' } ); } ); @@ -532,6 +584,189 @@ describe( 'BlockEdit Component', () => { } ) ); } ); + + test( 'loads a new form instead of writing back after an undo', async () => { + const setAttributes = jest.fn(); + await renderWithForm( setAttributes ); + await mountReloadedForm( { field_1: 'Hello' } ); + await sendFormChange( { field_1: 'HelloXYZ' } ); + expect( setAttributes ).toHaveBeenCalledTimes( 1 ); + + setAttributes.mockClear(); + mockAjax.mockClear(); + const mountsBefore = mockBlockFormMounts; + + // Undo puts back the stored data, the form still has the edit. + await changeDataFromOutside( storedAttributes.data ); + + expect( setAttributes ).not.toHaveBeenCalled(); + const { block } = mockAjax.mock.calls.at( -1 )[ 0 ].data; + expect( JSON.parse( block ).data ).toEqual( storedAttributes.data ); + expect( mockBlockFormMounts ).toBe( mountsBefore + 1 ); + + // The new form has the undone value, which is not written again. + await mountReloadedForm( { field_1: 'Hello' } ); + expect( setAttributes ).not.toHaveBeenCalled(); + } ); + + test( 'keeps redo and later edits working after an undo', async () => { + const setAttributes = jest.fn(); + await renderWithForm( setAttributes ); + await mountReloadedForm( { field_1: 'Hello' } ); + await sendFormChange( { field_1: 'HelloXYZ' } ); + + await changeDataFromOutside( storedAttributes.data ); + await mountReloadedForm( { field_1: 'Hello' } ); + + setAttributes.mockClear(); + const mountsBefore = mockBlockFormMounts; + + // Redo. + await changeDataFromOutside( { field_1: 'HelloXYZ' } ); + expect( setAttributes ).not.toHaveBeenCalled(); + expect( mockBlockFormMounts ).toBe( mountsBefore + 1 ); + await mountReloadedForm( { field_1: 'HelloXYZ' } ); + expect( setAttributes ).not.toHaveBeenCalled(); + + // A new edit after the redo is written. + await sendFormChange( { field_1: 'HelloXYZ!' } ); + expect( setAttributes ).toHaveBeenLastCalledWith( + expect.objectContaining( { + data: { field_1: 'HelloXYZ!' }, + } ) + ); + } ); + + test( 'keeps an edit typed while the new form is loading', async () => { + const setAttributes = jest.fn(); + await renderWithForm( setAttributes ); + await mountReloadedForm( { field_1: 'Hello' } ); + await sendFormChange( { field_1: 'HelloXYZ' } ); + + // Hold the next fetch, so the reload is still on its way. + let finishFetch = null; + mockAjax.mockImplementationOnce( () => ( { + done( callback ) { + finishFetch = () => + callback( { + data: { + form: '
Form HTML
', + preview: '
Preview HTML
', + validation: { valid: true }, + }, + } ); + return this; + }, + fail() { + return this; + }, + abort: jest.fn(), + } ) ); + + await changeDataFromOutside( storedAttributes.data ); + const mountsBefore = mockBlockFormMounts; + + acf.serialize.mockReturnValue( { field_1: 'HelloXYZa' } ); + await act( async () => { + mockBlockFormProps.onChange( $form ); + } ); + await act( async () => { + finishFetch(); + } ); + + expect( mockBlockFormMounts ).toBe( mountsBefore ); + + await act( async () => { + jest.advanceTimersByTime( 250 ); + } ); + expect( setAttributes ).toHaveBeenLastCalledWith( + expect.objectContaining( { + data: { field_1: 'HelloXYZa' }, + } ) + ); + } ); + + test( 'loads a new form when an untouched form gets outside data', async () => { + const setAttributes = jest.fn(); + await renderWithForm( setAttributes ); + mockAjax.mockClear(); + const mountsBefore = mockBlockFormMounts; + + await changeDataFromOutside( { field_1: 'External' } ); + + expect( setAttributes ).not.toHaveBeenCalled(); + const { block } = mockAjax.mock.calls.at( -1 )[ 0 ].data; + expect( JSON.parse( block ).data ).toEqual( { + field_1: 'External', + } ); + expect( mockBlockFormMounts ).toBe( mountsBefore + 1 ); + } ); + + test( 'does not load a new form when other attributes change', async () => { + const setAttributes = jest.fn(); + await renderWithForm( setAttributes ); + await mountReloadedForm( { field_1: 'Hello' } ); + await sendFormChange( { field_1: 'HelloXYZ' } ); + const mountsBefore = mockBlockFormMounts; + + await act( async () => { + setBlockAttributes( ( previous ) => ( { + ...previous, + align: 'wide', + } ) ); + } ); + await act( async () => { + jest.advanceTimersByTime( 250 ); + } ); + + expect( mockBlockFormMounts ).toBe( mountsBefore ); + } ); + } ); + + describe( 'isSameBlockData', () => { + test( 'maps field names to keys with the "_name" entries', () => { + expect( + isSameBlockData( + { title: 'Hello', _title: 'field_1' }, + { field_1: 'Hello' }, + null + ) + ).toBe( true ); + } ); + + test( 'maps field names to keys with the block fields', () => { + expect( + isSameBlockData( { title: 'Hello' }, { field_1: 'Hello' }, [ + { name: 'title', key: 'field_1' }, + ] ) + ).toBe( true ); + } ); + + test( 'compares data already keyed by field key', () => { + expect( + isSameBlockData( + { field_2: 'b', field_1: 'a' }, + { field_1: 'a', field_2: 'b' }, + null + ) + ).toBe( true ); + } ); + + test( 'finds different values', () => { + expect( + isSameBlockData( + { title: 'Hello', _title: 'field_1' }, + { field_1: 'HelloXYZ' }, + null + ) + ).toBe( false ); + } ); + + test( 'returns false without data', () => { + expect( + isSameBlockData( undefined, { field_1: 'Hello' }, null ) + ).toBe( false ); + } ); } ); test( 'handles isSelected prop change', async () => { diff --git a/tests/js/blocks-v3/block-form.test.js b/tests/js/blocks-v3/block-form.test.js index f0c12b58..51ff4f0f 100644 --- a/tests/js/blocks-v3/block-form.test.js +++ b/tests/js/blocks-v3/block-form.test.js @@ -609,4 +609,66 @@ describe( 'BlockForm Changes Before Interaction', () => { expect( onChange ).toHaveBeenCalledTimes( 2 ); expect( onUntouchedChange ).not.toHaveBeenCalled(); } ); + + test( 'a reloaded form ignores interaction from before it was mounted', async () => { + const onChange = jest.fn(); + const onUntouchedChange = jest.fn(); + const acfFormRef = renderForm( { + onChange, + onUntouchedChange, + userHasInteractedWithForm: true, + isReloaded: true, + } ); + + await mutateForm( acfFormRef ); + + expect( onUntouchedChange ).toHaveBeenCalled(); + expect( onChange ).not.toHaveBeenCalled(); + } ); + + test( 'a reloaded form reports edits made in it', async () => { + const onChange = jest.fn(); + const acfFormRef = renderForm( { + onChange, + onUntouchedChange: jest.fn(), + userHasInteractedWithForm: true, + isReloaded: true, + } ); + + await act( async () => { + acfFormRef.current + .querySelector( 'input' ) + .dispatchEvent( new Event( 'input' ) ); + } ); + + expect( onChange ).toHaveBeenCalledTimes( 1 ); + } ); +} ); + +describe( 'BlockForm Unmount', () => { + test( 'runs the unmount action so fields can clean up', () => { + const { unmount } = render( + + ); + acf.doAction.mockClear(); + + unmount(); + + expect( acf.doAction ).toHaveBeenCalledWith( + 'unmount', + expect.anything() + ); + } ); } );