From e69073b50f87f887a178ac3de79e454219e2baf7 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 6 Dec 2022 13:27:33 +0000 Subject: [PATCH] Fix auto Nav menu creation due to page list inner blocks (#46223) * Add exception for page list to ignore its inner blocks * Explain what is going on * Only consider the first block * Remove debugging * Be specific in the referential equality check comments * Ditch Lodash for Fast Deep Equal * Use customised conditional deep equality * Remove unnecessary let * Add comments for context * Fix grammar Co-authored-by: Andrei Draganescu * Target fix to Page List only Co-authored-by: Andrei Draganescu --- .../navigation/edit/unsaved-inner-blocks.js | 67 +++++++++++++++++-- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/navigation/edit/unsaved-inner-blocks.js b/packages/block-library/src/navigation/edit/unsaved-inner-blocks.js index 0fa2a119446f36..a57b221aa86ee7 100644 --- a/packages/block-library/src/navigation/edit/unsaved-inner-blocks.js +++ b/packages/block-library/src/navigation/edit/unsaved-inner-blocks.js @@ -35,6 +35,46 @@ const ALLOWED_BLOCKS = [ 'core/navigation-submenu', ]; +/** + * Conditionally compares two candidates for deep equality. + * Provides an option to skip a given property of an object during comparison. + * + * @param {*} x 1st candidate for comparison + * @param {*} y 2nd candidate for comparison + * @param {Function|undefined} shouldSkip a function which can be used to skip a given property of an object. + * @return {boolean} whether the two candidates are deeply equal. + */ +const isDeepEqual = ( x, y, shouldSkip ) => { + if ( x === y ) { + return true; + } else if ( + typeof x === 'object' && + x !== null && + x !== undefined && + typeof y === 'object' && + y !== null && + y !== undefined + ) { + if ( Object.keys( x ).length !== Object.keys( y ).length ) return false; + + for ( const prop in x ) { + if ( y.hasOwnProperty( prop ) ) { + // Afford skipping a given property of an object. + if ( shouldSkip && shouldSkip( prop, x ) ) { + return true; + } + + if ( ! isDeepEqual( x[ prop ], y[ prop ], shouldSkip ) ) + return false; + } else return false; + } + + return true; + } + + return false; +}; + export default function UnsavedInnerBlocks( { blocks, createNavigationMenu, @@ -51,13 +91,26 @@ export default function UnsavedInnerBlocks( { } }, [ blocks ] ); - // If the current inner blocks object is different in any way - // from the original inner blocks from the post content then the - // user has made changes to the inner blocks. At this point the inner - // blocks can be considered "dirty". - // We also make sure the current innerBlocks had a chance to be set. - const innerBlocksAreDirty = - !! originalBlocks.current && blocks !== originalBlocks.current; + // If the current inner blocks are different from the original inner blocks + // from the post content then the user has made changes to the inner blocks. + // At this point the inner blocks can be considered "dirty". + // Note: referential equality is not sufficient for comparison as the inner blocks + // of the page list are controlled and may be updated async due to syncing with + // entity records. As a result we need to perform a deep equality check skipping + // the page list's inner blocks. + const innerBlocksAreDirty = ! isDeepEqual( + originalBlocks.current, + blocks, + ( prop, x ) => { + // Skip inner blocks of page list during comparison as they + // are **always** controlled and may be updated async due to + // syncing with enitiy records. Left unchecked this would + // inadvertently trigger the dirty state. + if ( x?.name === 'core/page-list' && prop === 'innerBlocks' ) { + return true; + } + } + ); const shouldDirectInsert = useMemo( () =>