Skip to content

Commit 02a28ff

Browse files
getdavegetdave
authored
Add tests for Nav block uncontrolled blocks dirty state checking (#46329)
* Extract function and add basic tests * Augment the tests * Normalize test descriptors * Fix spelling * Add test for missing condition
1 parent f65cc68 commit 02a28ff

File tree

3 files changed

+173
-52
lines changed

3 files changed

+173
-52
lines changed

‎packages/block-library/src/navigation/edit/are-blocks-dirty.js‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
export function areBlocksDirty( originalBlocks, blocks ) {
2+
return ! isDeepEqual( originalBlocks, blocks, ( prop, x ) => {
3+
// Skip inner blocks of page list during comparison as they
4+
// are **always** controlled and may be updated async due to
5+
// syncing with entity records. Left unchecked this would
6+
// inadvertently trigger the dirty state.
7+
if ( x?.name === 'core/page-list' && prop === 'innerBlocks' ) {
8+
return true;
9+
}
10+
} );
11+
}
12+
13+
/**
14+
* Conditionally compares two candidates for deep equality.
15+
* Provides an option to skip a given property of an object during comparison.
16+
*
17+
* @param {*} x 1st candidate for comparison
18+
* @param {*} y 2nd candidate for comparison
19+
* @param {Function|undefined} shouldSkip a function which can be used to skip a given property of an object.
20+
* @return {boolean} whether the two candidates are deeply equal.
21+
*/
22+
const isDeepEqual = ( x, y, shouldSkip ) => {
23+
if ( x === y ) {
24+
return true;
25+
} else if (
26+
typeof x === 'object' &&
27+
x !== null &&
28+
x !== undefined &&
29+
typeof y === 'object' &&
30+
y !== null &&
31+
y !== undefined
32+
) {
33+
if ( Object.keys( x ).length !== Object.keys( y ).length ) return false;
34+
35+
for ( const prop in x ) {
36+
if ( y.hasOwnProperty( prop ) ) {
37+
// Afford skipping a given property of an object.
38+
if ( shouldSkip && shouldSkip( prop, x ) ) {
39+
return true;
40+
}
41+
42+
if ( ! isDeepEqual( x[ prop ], y[ prop ], shouldSkip ) )
43+
return false;
44+
} else return false;
45+
}
46+
47+
return true;
48+
}
49+
50+
return false;
51+
};

‎packages/block-library/src/navigation/edit/test/are-blocks-dirty.js‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/**
2+
* Internal dependencies
3+
*/
4+
import { areBlocksDirty } from '../are-blocks-dirty';
5+
6+
describe( 'areBlocksDirty', () => {
7+
it( 'should be clean if the blocks are the same', () => {
8+
expect(
9+
areBlocksDirty(
10+
[ { name: 'core/paragraph', content: 'I am not dirty.' } ],
11+
[ { name: 'core/paragraph', content: 'I am not dirty.' } ]
12+
)
13+
).toBe( false );
14+
} );
15+
16+
it( `should be dirty if the blocks' attributes are different`, () => {
17+
expect(
18+
areBlocksDirty(
19+
[ { name: 'core/paragraph', content: 'I am not dirty.' } ],
20+
[ { name: 'core/paragraph', content: 'I am actually dirty.' } ]
21+
)
22+
).toBe( true );
23+
} );
24+
25+
it( `should be dirty if the blocks' attributes don't match`, () => {
26+
expect(
27+
areBlocksDirty(
28+
[ { name: 'core/paragraph' } ],
29+
[ { name: 'core/paragraph', dropCap: false } ]
30+
)
31+
).toBe( true );
32+
} );
33+
34+
it( `should be dirty if the blocks' inner blocks are dirty`, () => {
35+
expect(
36+
areBlocksDirty(
37+
[
38+
{
39+
name: 'core/social-links',
40+
innerBlocks: [
41+
{
42+
name: 'core/social-link',
43+
url: 'www.wordpress.org',
44+
},
45+
],
46+
},
47+
],
48+
[
49+
{
50+
name: 'core/social-links',
51+
innerBlocks: [
52+
{
53+
name: 'core/social-link',
54+
service: 'wordpress',
55+
url: 'www.wordpress.org',
56+
},
57+
{
58+
name: 'core/social-link',
59+
service: 'wordpress',
60+
url: 'make.wordpress.org',
61+
},
62+
],
63+
},
64+
]
65+
)
66+
).toBe( true );
67+
} );
68+
69+
describe( 'Controlled Page List block specific exceptions', () => {
70+
it( 'should be clean if only page list inner blocks have changed', () => {
71+
expect(
72+
areBlocksDirty(
73+
[
74+
{ name: 'core/paragraph' },
75+
{
76+
name: 'core/page-list',
77+
innerBlocks: [],
78+
},
79+
],
80+
[
81+
{ name: 'core/paragraph' },
82+
{
83+
name: 'core/page-list',
84+
innerBlocks: [ { name: 'core/page-list-item' } ],
85+
},
86+
]
87+
)
88+
).toBe( false );
89+
} );
90+
91+
it( 'should be dirty if other blocks have changed alongside page list inner blocks', () => {
92+
expect(
93+
areBlocksDirty(
94+
[
95+
{
96+
name: 'core/paragraph',
97+
content: 'This is some text',
98+
},
99+
{
100+
name: 'core/page-list',
101+
innerBlocks: [],
102+
},
103+
],
104+
[
105+
{
106+
name: 'core/paragraph',
107+
content: 'This is some text that has changed',
108+
},
109+
{
110+
name: 'core/page-list',
111+
innerBlocks: [ { name: 'core/page-list-item' } ],
112+
},
113+
]
114+
)
115+
).toBe( true );
116+
} );
117+
} );
118+
} );

‎packages/block-library/src/navigation/edit/unsaved-inner-blocks.js‎

Lines changed: 4 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { useContext, useEffect, useRef, useMemo } from '@wordpress/element';
1111
* Internal dependencies
1212
*/
1313
import useNavigationMenu from '../use-navigation-menu';
14+
import { areBlocksDirty } from './are-blocks-dirty';
1415

1516
const EMPTY_OBJECT = {};
1617
const DRAFT_MENU_PARAMS = [
@@ -35,46 +36,6 @@ const ALLOWED_BLOCKS = [
3536
'core/navigation-submenu',
3637
];
3738

38-
/**
39-
* Conditionally compares two candidates for deep equality.
40-
* Provides an option to skip a given property of an object during comparison.
41-
*
42-
* @param {*} x 1st candidate for comparison
43-
* @param {*} y 2nd candidate for comparison
44-
* @param {Function|undefined} shouldSkip a function which can be used to skip a given property of an object.
45-
* @return {boolean} whether the two candidates are deeply equal.
46-
*/
47-
const isDeepEqual = ( x, y, shouldSkip ) => {
48-
if ( x === y ) {
49-
return true;
50-
} else if (
51-
typeof x === 'object' &&
52-
x !== null &&
53-
x !== undefined &&
54-
typeof y === 'object' &&
55-
y !== null &&
56-
y !== undefined
57-
) {
58-
if ( Object.keys( x ).length !== Object.keys( y ).length ) return false;
59-
60-
for ( const prop in x ) {
61-
if ( y.hasOwnProperty( prop ) ) {
62-
// Afford skipping a given property of an object.
63-
if ( shouldSkip && shouldSkip( prop, x ) ) {
64-
return true;
65-
}
66-
67-
if ( ! isDeepEqual( x[ prop ], y[ prop ], shouldSkip ) )
68-
return false;
69-
} else return false;
70-
}
71-
72-
return true;
73-
}
74-
75-
return false;
76-
};
77-
7839
export default function UnsavedInnerBlocks( {
7940
blocks,
8041
createNavigationMenu,
@@ -98,18 +59,9 @@ export default function UnsavedInnerBlocks( {
9859
// of the page list are controlled and may be updated async due to syncing with
9960
// entity records. As a result we need to perform a deep equality check skipping
10061
// the page list's inner blocks.
101-
const innerBlocksAreDirty = ! isDeepEqual(
102-
originalBlocks.current,
103-
blocks,
104-
( prop, x ) => {
105-
// Skip inner blocks of page list during comparison as they
106-
// are **always** controlled and may be updated async due to
107-
// syncing with enitiy records. Left unchecked this would
108-
// inadvertently trigger the dirty state.
109-
if ( x?.name === 'core/page-list' && prop === 'innerBlocks' ) {
110-
return true;
111-
}
112-
}
62+
const innerBlocksAreDirty = areBlocksDirty(
63+
originalBlocks?.current,
64+
blocks
11365
);
11466

11567
const shouldDirectInsert = useMemo(

0 commit comments

Comments
 (0)