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..7830c3ff 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,18 +677,29 @@ 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 ) ) { + // 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, } ); } + hasWrittenFormDataRef.current = true; + // Use original attributes (with hasAcfError) when updating const updatedAttributes = { ...attributes, @@ -714,6 +771,7 @@ export const BlockEdit = ( props ) => { showValidationErrors={ showValidationErrors } theSerializedAcfData={ theSerializedAcfData } setTheSerializedAcfData={ setTheSerializedAcfData } + captureInitialFormData={ captureInitialFormData } acfFormRef={ acfFormRef } blockFormHtml={ blockFormHtml } blockPreviewHtml={ blockPreviewHtml } @@ -770,6 +828,7 @@ function BlockEditInner( props ) { showValidationErrors, theSerializedAcfData, setTheSerializedAcfData, + captureInitialFormData, acfFormRef, blockFormHtml, blockPreviewHtml, @@ -1005,14 +1064,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 +1308,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..4e48a45d 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,177 @@ 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 ); + } ); + + 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 () => { + 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(); + } ); +} );