Skip to content

Commit

Permalink
Add tests for Nav block uncontrolled blocks dirty state checking (Wor…
Browse files Browse the repository at this point in the history
…dPress#46329)

* Extract function and add basic tests

* Augment the tests

* Normalize test descriptors

* Fix spelling

* Add test for missing condition
  • Loading branch information
getdave authored and mpkelly committed Dec 7, 2022
1 parent fca4207 commit 63bfa7a
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 52 deletions.
51 changes: 51 additions & 0 deletions packages/block-library/src/navigation/edit/are-blocks-dirty.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
export function areBlocksDirty( originalBlocks, blocks ) {
return ! isDeepEqual( originalBlocks, 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 entity records. Left unchecked this would
// inadvertently trigger the dirty state.
if ( x?.name === 'core/page-list' && prop === 'innerBlocks' ) {
return true;
}
} );
}

/**
* 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;
};
118 changes: 118 additions & 0 deletions packages/block-library/src/navigation/edit/test/are-blocks-dirty.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/**
* Internal dependencies
*/
import { areBlocksDirty } from '../are-blocks-dirty';

describe( 'areBlocksDirty', () => {
it( 'should be clean if the blocks are the same', () => {
expect(
areBlocksDirty(
[ { name: 'core/paragraph', content: 'I am not dirty.' } ],
[ { name: 'core/paragraph', content: 'I am not dirty.' } ]
)
).toBe( false );
} );

it( `should be dirty if the blocks' attributes are different`, () => {
expect(
areBlocksDirty(
[ { name: 'core/paragraph', content: 'I am not dirty.' } ],
[ { name: 'core/paragraph', content: 'I am actually dirty.' } ]
)
).toBe( true );
} );

it( `should be dirty if the blocks' attributes don't match`, () => {
expect(
areBlocksDirty(
[ { name: 'core/paragraph' } ],
[ { name: 'core/paragraph', dropCap: false } ]
)
).toBe( true );
} );

it( `should be dirty if the blocks' inner blocks are dirty`, () => {
expect(
areBlocksDirty(
[
{
name: 'core/social-links',
innerBlocks: [
{
name: 'core/social-link',
url: 'www.wordpress.org',
},
],
},
],
[
{
name: 'core/social-links',
innerBlocks: [
{
name: 'core/social-link',
service: 'wordpress',
url: 'www.wordpress.org',
},
{
name: 'core/social-link',
service: 'wordpress',
url: 'make.wordpress.org',
},
],
},
]
)
).toBe( true );
} );

describe( 'Controlled Page List block specific exceptions', () => {
it( 'should be clean if only page list inner blocks have changed', () => {
expect(
areBlocksDirty(
[
{ name: 'core/paragraph' },
{
name: 'core/page-list',
innerBlocks: [],
},
],
[
{ name: 'core/paragraph' },
{
name: 'core/page-list',
innerBlocks: [ { name: 'core/page-list-item' } ],
},
]
)
).toBe( false );
} );

it( 'should be dirty if other blocks have changed alongside page list inner blocks', () => {
expect(
areBlocksDirty(
[
{
name: 'core/paragraph',
content: 'This is some text',
},
{
name: 'core/page-list',
innerBlocks: [],
},
],
[
{
name: 'core/paragraph',
content: 'This is some text that has changed',
},
{
name: 'core/page-list',
innerBlocks: [ { name: 'core/page-list-item' } ],
},
]
)
).toBe( true );
} );
} );
} );
56 changes: 4 additions & 52 deletions packages/block-library/src/navigation/edit/unsaved-inner-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { useContext, useEffect, useRef, useMemo } from '@wordpress/element';
* Internal dependencies
*/
import useNavigationMenu from '../use-navigation-menu';
import { areBlocksDirty } from './are-blocks-dirty';

const EMPTY_OBJECT = {};
const DRAFT_MENU_PARAMS = [
Expand All @@ -35,46 +36,6 @@ 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 @@ -98,18 +59,9 @@ export default function UnsavedInnerBlocks( {
// 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 innerBlocksAreDirty = areBlocksDirty(
originalBlocks?.current,
blocks
);

const shouldDirectInsert = useMemo(
Expand Down

0 comments on commit 63bfa7a

Please sign in to comment.