Skip to content

Commit

Permalink
Fix auto Nav menu creation due to page list inner blocks (#46223)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>

* Target fix to Page List only

Co-authored-by: Andrei Draganescu <[email protected]>
  • Loading branch information
getdave and draganescu authored Dec 6, 2022
1 parent 392df59 commit e69073b
Showing 1 changed file with 60 additions and 7 deletions.
67 changes: 60 additions & 7 deletions packages/block-library/src/navigation/edit/unsaved-inner-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
() =>
Expand Down

0 comments on commit e69073b

Please sign in to comment.