From 354b0bbd55106546a461fa192b10540adec70eec Mon Sep 17 00:00:00 2001 From: Jorge Costa Date: Fri, 25 Aug 2023 12:17:36 +0100 Subject: [PATCH 1/2] Fix: InnerBlocks allowed blocks change with different sizes. --- .../use-nested-settings-update.js | 25 +++++++++++-------- .../inner-blocks-allowed-blocks/index.js | 15 ++++++----- .../inner-blocks-allowed-blocks.spec.js | 12 +++++++++ 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js b/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js index 44f99428a31bf8..8eba8a1d2223d4 100644 --- a/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js +++ b/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js @@ -1,9 +1,10 @@ /** * WordPress dependencies */ -import { useLayoutEffect, useMemo } from '@wordpress/element'; +import { useLayoutEffect, useMemo, useState } from '@wordpress/element'; import { useSelect, useDispatch, useRegistry } from '@wordpress/data'; import deprecated from '@wordpress/deprecated'; +import isShallowEqual from '@wordpress/is-shallow-equal'; /** * Internal dependencies @@ -15,6 +16,14 @@ import { getLayoutType } from '../../layouts'; const pendingSettingsUpdates = new WeakMap(); +function useShallowMemo( value ) { + const [ prevValue, setPrevValue ] = useState( value ); + if ( ! isShallowEqual( prevValue, value ) ) { + setPrevValue( value ); + } + return prevValue; +} + /** * This hook is a side effect which updates the block-editor store when changes * happen to inner block settings. The given props are transformed into a @@ -70,16 +79,12 @@ export default function useNestedSettingsUpdate( [ clientId ] ); - // Memoize allowedBlocks and prioritisedInnerBlocks based on the contents - // of the arrays. Implementors often pass a new array on every render, + // Implementors often pass a new array on every render, // and the contents of the arrays are just strings, so the entire array - // can be passed as dependencies. - - const _allowedBlocks = useMemo( - () => allowedBlocks, - // eslint-disable-next-line react-hooks/exhaustive-deps - allowedBlocks - ); + // can be passed as dependencies but We need to include the length of the array, + // otherwise if the arrays change length but the first elements are equal the comparison, + // does not works as expected. + const _allowedBlocks = useShallowMemo( allowedBlocks ); const _prioritizedInserterBlocks = useMemo( () => prioritizedInserterBlocks, diff --git a/packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js b/packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js index c32307b2d49ff1..db5f3e1dc0611b 100644 --- a/packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js +++ b/packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js @@ -9,7 +9,8 @@ }; const allowedBlocksWhenSingleEmptyChild = [ 'core/image', 'core/list' ]; - const allowedBlocksWhenMultipleChildren = [ 'core/gallery', 'core/video' ]; + const allowedBlocksWhenTwoChildren = [ 'core/gallery', 'core/video' ]; + const allowedBlocksWhenTreeOrMoreChildren = [ 'core/gallery', 'core/video', 'core/list' ]; registerBlockType( 'test/allowed-blocks-dynamic', { apiVersion: 3, @@ -25,7 +26,12 @@ }, [ props.clientId ] ); - + let allowedBlocks = allowedBlocksWhenSingleEmptyChild; + if ( props.numberOfChildren === 2 ) { + allowedBlocks = allowedBlocksWhenTwoChildren; + } else if( props.numberOfChildren > 2 ){ + allowedBlocks = allowedBlocksWhenTreeOrMoreChildren; + } return el( 'div', { @@ -33,10 +39,7 @@ 'data-number-of-children': numberOfChildren, }, el( InnerBlocks, { - allowedBlocks: - numberOfChildren < 2 - ? allowedBlocksWhenSingleEmptyChild - : allowedBlocksWhenMultipleChildren, + allowedBlocks, } ) ); }, diff --git a/test/e2e/specs/editor/plugins/inner-blocks-allowed-blocks.spec.js b/test/e2e/specs/editor/plugins/inner-blocks-allowed-blocks.spec.js index a515ef68316e83..cd6f440d2db0ed 100644 --- a/test/e2e/specs/editor/plugins/inner-blocks-allowed-blocks.spec.js +++ b/test/e2e/specs/editor/plugins/inner-blocks-allowed-blocks.spec.js @@ -142,5 +142,17 @@ test.describe( 'Allowed Blocks Setting on InnerBlocks', () => { 'Gallery', 'Video', ] ); + + await blockListBox.getByRole( 'option', { name: 'Gallery' } ).click(); + + await editor.clickBlockToolbarButton( 'Select Allowed Blocks Dynamic' ); + await blockAppender.click(); + + // It should display a different allowed block list. + await expect( blockListBox.getByRole( 'option' ) ).toHaveText( [ + 'Gallery', + 'List', + 'Video', + ] ); } ); } ); From d0d0b1b7063f345426bd38216687fea8be8247a8 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Tue, 5 Sep 2023 15:46:45 +0400 Subject: [PATCH 2/2] Fix e2e test plugin --- .../inner-blocks-allowed-blocks/index.js | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js b/packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js index db5f3e1dc0611b..5bcfed61519833 100644 --- a/packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js +++ b/packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js @@ -2,7 +2,7 @@ const { useSelect } = wp.data; const { registerBlockType } = wp.blocks; const { createElement: el } = wp.element; - const { InnerBlocks } = wp.blockEditor; + const { InnerBlocks, useBlockProps } = wp.blockEditor; const divProps = { className: 'product', style: { outline: '1px solid gray', padding: 5 }, @@ -10,7 +10,7 @@ const allowedBlocksWhenSingleEmptyChild = [ 'core/image', 'core/list' ]; const allowedBlocksWhenTwoChildren = [ 'core/gallery', 'core/video' ]; - const allowedBlocksWhenTreeOrMoreChildren = [ 'core/gallery', 'core/video', 'core/list' ]; + const allowedBlocksWhenTreeOrMoreChildren = [ 'core/gallery', 'core/video', 'core/list' ]; registerBlockType( 'test/allowed-blocks-dynamic', { apiVersion: 3, @@ -26,20 +26,23 @@ }, [ props.clientId ] ); - let allowedBlocks = allowedBlocksWhenSingleEmptyChild; - if ( props.numberOfChildren === 2 ) { - allowedBlocks = allowedBlocksWhenTwoChildren; - } else if( props.numberOfChildren > 2 ){ - allowedBlocks = allowedBlocksWhenTreeOrMoreChildren; - } + const blockProps = useBlockProps({ + ...divProps, + 'data-number-of-children': numberOfChildren, + }); + + let allowedBlocks = allowedBlocksWhenSingleEmptyChild; + if ( numberOfChildren === 2 ) { + allowedBlocks = allowedBlocksWhenTwoChildren; + } else if( numberOfChildren > 2 ){ + allowedBlocks = allowedBlocksWhenTreeOrMoreChildren; + } + return el( 'div', - { - ...divProps, - 'data-number-of-children': numberOfChildren, - }, + blockProps, el( InnerBlocks, { - allowedBlocks, + allowedBlocks } ) ); },