From 177cb4dfb39106ff6dd2b82dbabe43126f8b1cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 20 Jul 2018 12:56:20 +0200 Subject: [PATCH 01/12] Refs #7957 - Inserter: take into account referenced blocks When deriving whether a block should be disabled or not, the inserter should take into account the blocks referenced by the existing top-level shared blocks. --- packages/editor/src/store/selectors.js | 27 +++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index ffabf1c82f7019..ccf0621b2b8253 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -564,6 +564,31 @@ export const getBlocks = createSelector( ] ); +/** + * Returns an array containing both top-level blocks + * and all blocks referenced by the existing top-level shared blocks. + * + * @param {Object} state Global application state. + * + * @return {Array} All top-level and referenced blocks. + */ +export const getBlocksTopLevelAndReferenced = createSelector( + ( state ) => { + const topLevelBlocks = getBlocks( state ); + const referencedBlocksIds = topLevelBlocks.map( + ( block ) => block.name === 'core/block' ? + get( getSharedBlock( state, block.attributes.ref ), [ 'clientId' ], null ) : + null, + ).filter( ( id ) => !! id ); + const referencedBlocks = getBlocksByClientId( state, referencedBlocksIds ); + return [ ...topLevelBlocks, ...referencedBlocks ]; + }, + ( state ) => [ + state.editor.present.blockOrder, + state.editor.present.blocksByClientId, + ] +); + /** * Returns the total number of blocks, or the total number of blocks with a specific name in a post. * The number returned includes nested blocks. @@ -1546,7 +1571,7 @@ export const getInserterItems = createSelector( let isDisabled = false; if ( ! hasBlockSupport( blockType.name, 'multiple', true ) ) { - isDisabled = some( getBlocks( state ), { name: blockType.name } ); + isDisabled = some( getBlocksTopLevelAndReferenced( state ), { name: blockType.name } ); } const isContextual = isArray( blockType.parent ); From 9f08042745c1134b903f1b8f24c15b07cf4e62b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 20 Jul 2018 13:22:22 +0200 Subject: [PATCH 02/12] Add test for getBlocksTopLevelAndReferenced selector --- packages/editor/src/store/test/selectors.js | 34 +++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index a1877d42fea629..02a2c035d60c52 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -44,6 +44,7 @@ const { getBlockName, getBlock, getBlocks, + getBlocksTopLevelAndReferenced, getBlockCount, hasSelectedBlock, getSelectedBlock, @@ -1731,6 +1732,39 @@ describe( 'selectors', () => { } ); } ); + describe( 'getBlocksTopLevelAndReferenced', () => { + it( 'should return the top level blocks and any block referenced by a existing top-level shared block', () => { + const state = { + currentPost: {}, + editor: { + present: { + blocksByClientId: { + 'uuid-2': { clientId: 'uuid-2', name: 'core/image', attributes: {} }, + 'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph', attributes: {} }, + 'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph', attributes: {} }, + 'uuid-8': { clientId: 'uuid-8', name: 'core/block', attributes: { ref: 1 } }, + }, + blockOrder: { + '': [ 'uuid-6', 'uuid-8' ], + }, + edits: {}, + }, + }, + sharedBlocks: { + data: { + 1: { clientId: 'uuid-2', title: 'SharedImage' }, + 3: { clientId: 'uuid-4', title: 'SharedParagraph' }, + }, + }, + }; + expect( getBlocksTopLevelAndReferenced( state ) ).toEqual( [ + { clientId: 'uuid-6', name: 'core/paragraph', attributes: {}, innerBlocks: [] }, + { clientId: 'uuid-8', name: 'core/block', attributes: { ref: 1 }, innerBlocks: [] }, + { clientId: 'uuid-2', name: 'core/image', attributes: {}, innerBlocks: [] }, + ] ); + } ); + } ); + describe( 'getBlockCount', () => { it( 'should return the number of top-level blocks in the post', () => { const state = { From 9c2b1b2cf505543b24010627448aedde629540fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 20 Jul 2018 13:55:04 +0200 Subject: [PATCH 03/12] Test that nested blocks are unfolded --- packages/editor/src/store/test/selectors.js | 31 ++++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 02a2c035d60c52..f1a9ed16b45c95 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -44,7 +44,7 @@ const { getBlockName, getBlock, getBlocks, - getBlocksTopLevelAndReferenced, + getBlocksUnfolded, getBlockCount, hasSelectedBlock, getSelectedBlock, @@ -1732,7 +1732,7 @@ describe( 'selectors', () => { } ); } ); - describe( 'getBlocksTopLevelAndReferenced', () => { + describe( 'getBlocksUnfolded', () => { it( 'should return the top level blocks and any block referenced by a existing top-level shared block', () => { const state = { currentPost: {}, @@ -1743,9 +1743,21 @@ describe( 'selectors', () => { 'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph', attributes: {} }, 'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph', attributes: {} }, 'uuid-8': { clientId: 'uuid-8', name: 'core/block', attributes: { ref: 1 } }, + 'uuid-10': { clientId: 'uuid-10', name: 'core/columns', attributes: { } }, + 'uuid-12': { clientId: 'uuid-12', name: 'core/column', attributes: { } }, + 'uuid-14': { clientId: 'uuid-14', name: 'core/column', attributes: { } }, + 'uuid-16': { clientId: 'uuid-16', name: 'core/quote', attributes: { } }, }, blockOrder: { - '': [ 'uuid-6', 'uuid-8' ], + '': [ 'uuid-6', 'uuid-8', 'uuid-10' ], + 'uuid-2': [ ], + 'uuid-4': [ ], + 'uuid-6': [ ], + 'uuid-8': [ ], + 'uuid-10': [ 'uuid-12', 'uuid-14' ], + 'uuid-12': [ 'uuid-16' ], + 'uuid-14': [ ], + 'uuid-16': [ ], }, edits: {}, }, @@ -1757,10 +1769,21 @@ describe( 'selectors', () => { }, }, }; - expect( getBlocksTopLevelAndReferenced( state ) ).toEqual( [ + expect( getBlocksUnfolded( state ) ).toEqual( [ { clientId: 'uuid-6', name: 'core/paragraph', attributes: {}, innerBlocks: [] }, { clientId: 'uuid-8', name: 'core/block', attributes: { ref: 1 }, innerBlocks: [] }, + { clientId: 'uuid-10', name: 'core/columns', attributes: { }, innerBlocks: [ + { clientId: 'uuid-12', name: 'core/column', attributes: { }, innerBlocks: [ + { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, + ] }, + { clientId: 'uuid-14', name: 'core/column', attributes: { }, innerBlocks: [] }, + ] }, { clientId: 'uuid-2', name: 'core/image', attributes: {}, innerBlocks: [] }, + { clientId: 'uuid-12', name: 'core/column', attributes: { }, innerBlocks: [ + { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, + ] }, + { clientId: 'uuid-14', name: 'core/column', attributes: { }, innerBlocks: [] }, + { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, ] ); } ); } ); From 55bc814ab4f639542eff1cc25525aa9fe988ae06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 20 Jul 2018 16:33:37 +0200 Subject: [PATCH 04/12] Take into account also children of nested blocks --- packages/editor/src/store/selectors.js | 35 ++++++++++++++------- packages/editor/src/store/test/selectors.js | 25 ++++++++++----- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index ccf0621b2b8253..60185253eb0009 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -564,24 +564,35 @@ export const getBlocks = createSelector( ] ); +const unfoldBlockIds = ( state, block ) => { + const clientIds = []; + if ( block.name === 'core/block' ) { + clientIds.push( get( getSharedBlock( state, block.attributes.ref ), [ 'clientId' ], null ) ); + } + if ( block.innerBlocks.length > 0 ) { + block.innerBlocks.map( ( innerBlock ) => { + clientIds.push( innerBlock.clientId ); + clientIds.push( ...unfoldBlockIds( state, innerBlock ) ); + } ); + } + return clientIds.filter( ( id ) => !! id ); +}; + /** - * Returns an array containing both top-level blocks - * and all blocks referenced by the existing top-level shared blocks. + * Returns an array containing the top-level blocks, + * all blocks referenced by the existing top-level shared blocks, + * and all the inner blocks of existing top-level nested blocks. * * @param {Object} state Global application state. * - * @return {Array} All top-level and referenced blocks. + * @return {Array} All top-level, referenced, and inner blocks. */ -export const getBlocksTopLevelAndReferenced = createSelector( +export const getBlocksUnfolded = createSelector( ( state ) => { const topLevelBlocks = getBlocks( state ); - const referencedBlocksIds = topLevelBlocks.map( - ( block ) => block.name === 'core/block' ? - get( getSharedBlock( state, block.attributes.ref ), [ 'clientId' ], null ) : - null, - ).filter( ( id ) => !! id ); - const referencedBlocks = getBlocksByClientId( state, referencedBlocksIds ); - return [ ...topLevelBlocks, ...referencedBlocks ]; + const clientIds = []; + topLevelBlocks.forEach( ( block ) => clientIds.push( ...unfoldBlockIds( state, block ) ) ); + return [ ...topLevelBlocks, ...getBlocksByClientId( state, clientIds ) ]; }, ( state ) => [ state.editor.present.blockOrder, @@ -1571,7 +1582,7 @@ export const getInserterItems = createSelector( let isDisabled = false; if ( ! hasBlockSupport( blockType.name, 'multiple', true ) ) { - isDisabled = some( getBlocksTopLevelAndReferenced( state ), { name: blockType.name } ); + isDisabled = some( getBlocksUnfolded( state ), { name: blockType.name } ); } const isContextual = isArray( blockType.parent ); diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index f1a9ed16b45c95..1f959aa110eae7 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -1743,10 +1743,12 @@ describe( 'selectors', () => { 'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph', attributes: {} }, 'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph', attributes: {} }, 'uuid-8': { clientId: 'uuid-8', name: 'core/block', attributes: { ref: 1 } }, - 'uuid-10': { clientId: 'uuid-10', name: 'core/columns', attributes: { } }, - 'uuid-12': { clientId: 'uuid-12', name: 'core/column', attributes: { } }, - 'uuid-14': { clientId: 'uuid-14', name: 'core/column', attributes: { } }, - 'uuid-16': { clientId: 'uuid-16', name: 'core/quote', attributes: { } }, + 'uuid-10': { clientId: 'uuid-10', name: 'core/columns', attributes: {} }, + 'uuid-12': { clientId: 'uuid-12', name: 'core/column', attributes: {} }, + 'uuid-14': { clientId: 'uuid-14', name: 'core/column', attributes: {} }, + 'uuid-16': { clientId: 'uuid-16', name: 'core/quote', attributes: {} }, + 'uuid-18': { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 } }, + 'uuid-20': { clientId: 'uuid-20', name: 'core/gallery', attributes: {} }, }, blockOrder: { '': [ 'uuid-6', 'uuid-8', 'uuid-10' ], @@ -1756,8 +1758,10 @@ describe( 'selectors', () => { 'uuid-8': [ ], 'uuid-10': [ 'uuid-12', 'uuid-14' ], 'uuid-12': [ 'uuid-16' ], - 'uuid-14': [ ], + 'uuid-14': [ 'uuid-18' ], 'uuid-16': [ ], + 'uuid-18': [ ], + 'uuid-20': [ ], }, edits: {}, }, @@ -1766,6 +1770,7 @@ describe( 'selectors', () => { data: { 1: { clientId: 'uuid-2', title: 'SharedImage' }, 3: { clientId: 'uuid-4', title: 'SharedParagraph' }, + 5: { clientId: 'uuid-20', title: 'SharedGallery' }, }, }, }; @@ -1776,14 +1781,20 @@ describe( 'selectors', () => { { clientId: 'uuid-12', name: 'core/column', attributes: { }, innerBlocks: [ { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, ] }, - { clientId: 'uuid-14', name: 'core/column', attributes: { }, innerBlocks: [] }, + { clientId: 'uuid-14', name: 'core/column', attributes: { }, innerBlocks: [ + { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 }, innerBlocks: [] }, + ] }, ] }, { clientId: 'uuid-2', name: 'core/image', attributes: {}, innerBlocks: [] }, { clientId: 'uuid-12', name: 'core/column', attributes: { }, innerBlocks: [ { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, ] }, - { clientId: 'uuid-14', name: 'core/column', attributes: { }, innerBlocks: [] }, { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, + { clientId: 'uuid-14', name: 'core/column', attributes: { }, innerBlocks: [ + { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 }, innerBlocks: [] }, + ] }, + { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 }, innerBlocks: [] }, + { clientId: 'uuid-20', name: 'core/gallery', attributes: {}, innerBlocks: [] }, ] ); } ); } ); From 43310ba69fb5e2d9cbe2131bcdc13b10528ccd18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Mon, 23 Jul 2018 13:52:25 +0200 Subject: [PATCH 05/12] Expose getBlockIdsUnfolded instead of getBlocksUnfolded By doing this, we reduce the number of API methods to retrieve blocks to: - getBlocks: top level blocks - getBlocksByClientId: blocks by id And we expose getBlockIdsUnfolded as an utility to retrieve the ids of top-level, referenced, and nested blocks. --- packages/editor/src/store/selectors.js | 17 +++++---- packages/editor/src/store/test/selectors.js | 39 ++++++++------------- 2 files changed, 24 insertions(+), 32 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index 60185253eb0009..ab0c8eedad762d 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -579,20 +579,23 @@ const unfoldBlockIds = ( state, block ) => { }; /** - * Returns an array containing the top-level blocks, + * Returns an array containing the ids of the top-level blocks, * all blocks referenced by the existing top-level shared blocks, * and all the inner blocks of existing top-level nested blocks. * * @param {Object} state Global application state. * - * @return {Array} All top-level, referenced, and inner blocks. + * @return {Array} All ids of top-level, referenced, and inner blocks. */ -export const getBlocksUnfolded = createSelector( +export const getBlockIdsUnfolded = createSelector( ( state ) => { - const topLevelBlocks = getBlocks( state ); const clientIds = []; - topLevelBlocks.forEach( ( block ) => clientIds.push( ...unfoldBlockIds( state, block ) ) ); - return [ ...topLevelBlocks, ...getBlocksByClientId( state, clientIds ) ]; + const topLevelBlocks = getBlocks( state ); + topLevelBlocks.forEach( ( block ) => { + clientIds.push( block.clientId ); + clientIds.push( ...unfoldBlockIds( state, block ) ); + } ); + return clientIds; }, ( state ) => [ state.editor.present.blockOrder, @@ -1582,7 +1585,7 @@ export const getInserterItems = createSelector( let isDisabled = false; if ( ! hasBlockSupport( blockType.name, 'multiple', true ) ) { - isDisabled = some( getBlocksUnfolded( state ), { name: blockType.name } ); + isDisabled = some( getBlocksByClientId( state, getBlockIdsUnfolded( state ) ), { name: blockType.name } ); } const isContextual = isArray( blockType.parent ); diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 1f959aa110eae7..37be565161a50e 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -44,7 +44,7 @@ const { getBlockName, getBlock, getBlocks, - getBlocksUnfolded, + getBlockIdsUnfolded, getBlockCount, hasSelectedBlock, getSelectedBlock, @@ -1732,8 +1732,8 @@ describe( 'selectors', () => { } ); } ); - describe( 'getBlocksUnfolded', () => { - it( 'should return the top level blocks and any block referenced by a existing top-level shared block', () => { + describe( 'getBlockIdsUnfolded', () => { + it( 'should return the ids for top-level blocks, any block referenced by a existing top-level shared block, and children of nested blocks.', () => { const state = { currentPost: {}, editor: { @@ -1774,27 +1774,16 @@ describe( 'selectors', () => { }, }, }; - expect( getBlocksUnfolded( state ) ).toEqual( [ - { clientId: 'uuid-6', name: 'core/paragraph', attributes: {}, innerBlocks: [] }, - { clientId: 'uuid-8', name: 'core/block', attributes: { ref: 1 }, innerBlocks: [] }, - { clientId: 'uuid-10', name: 'core/columns', attributes: { }, innerBlocks: [ - { clientId: 'uuid-12', name: 'core/column', attributes: { }, innerBlocks: [ - { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, - ] }, - { clientId: 'uuid-14', name: 'core/column', attributes: { }, innerBlocks: [ - { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 }, innerBlocks: [] }, - ] }, - ] }, - { clientId: 'uuid-2', name: 'core/image', attributes: {}, innerBlocks: [] }, - { clientId: 'uuid-12', name: 'core/column', attributes: { }, innerBlocks: [ - { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, - ] }, - { clientId: 'uuid-16', name: 'core/quote', attributes: { }, innerBlocks: [] }, - { clientId: 'uuid-14', name: 'core/column', attributes: { }, innerBlocks: [ - { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 }, innerBlocks: [] }, - ] }, - { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 }, innerBlocks: [] }, - { clientId: 'uuid-20', name: 'core/gallery', attributes: {}, innerBlocks: [] }, + expect( getBlockIdsUnfolded( state ) ).toEqual( [ + 'uuid-6', + 'uuid-8', + 'uuid-2', + 'uuid-10', + 'uuid-12', + 'uuid-16', + 'uuid-14', + 'uuid-18', + 'uuid-20', ] ); } ); } ); @@ -3284,7 +3273,7 @@ describe( 'selectors', () => { editor: { present: { blocksByClientId: { - block1: { name: 'core/test-block-b' }, + block1: { clientId: 'block1', name: 'core/test-block-b' }, }, blockOrder: { '': [ 'block1' ], From 14490acb5c71f7d2790f196b47f8b70806ec7376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 26 Jul 2018 13:04:01 +0200 Subject: [PATCH 06/12] Rename --- packages/editor/src/store/selectors.js | 26 ++++++++++++++------- packages/editor/src/store/test/selectors.js | 6 ++--- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index ab0c8eedad762d..410540d2ea3762 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -564,7 +564,17 @@ export const getBlocks = createSelector( ] ); -const unfoldBlockIds = ( state, block ) => { +/** + * Given a block, returns an array containing all clientIds + * of referenced blocks (if it's a shared block) + * and innerBlocks. + * + * @param {Object} state Global application state. + * @param {Object} block The block whose ids we want to unfold. + * + * @return {Array} ids of referenced and inner blocks. + */ +const unfoldClientIds = ( state, block ) => { const clientIds = []; if ( block.name === 'core/block' ) { clientIds.push( get( getSharedBlock( state, block.attributes.ref ), [ 'clientId' ], null ) ); @@ -572,28 +582,28 @@ const unfoldBlockIds = ( state, block ) => { if ( block.innerBlocks.length > 0 ) { block.innerBlocks.map( ( innerBlock ) => { clientIds.push( innerBlock.clientId ); - clientIds.push( ...unfoldBlockIds( state, innerBlock ) ); + clientIds.push( ...unfoldClientIds( state, innerBlock ) ); } ); } return clientIds.filter( ( id ) => !! id ); }; /** - * Returns an array containing the ids of the top-level blocks, - * all blocks referenced by the existing top-level shared blocks, + * Returns an array containing the clientIds of the top-level blocks, + * all blocks referenced by any shared block, * and all the inner blocks of existing top-level nested blocks. * * @param {Object} state Global application state. * - * @return {Array} All ids of top-level, referenced, and inner blocks. + * @return {Array} ids of top-level, referenced, and inner blocks. */ -export const getBlockIdsUnfolded = createSelector( +export const getClientIdsUnfolded = createSelector( ( state ) => { const clientIds = []; const topLevelBlocks = getBlocks( state ); topLevelBlocks.forEach( ( block ) => { clientIds.push( block.clientId ); - clientIds.push( ...unfoldBlockIds( state, block ) ); + clientIds.push( ...unfoldClientIds( state, block ) ); } ); return clientIds; }, @@ -1585,7 +1595,7 @@ export const getInserterItems = createSelector( let isDisabled = false; if ( ! hasBlockSupport( blockType.name, 'multiple', true ) ) { - isDisabled = some( getBlocksByClientId( state, getBlockIdsUnfolded( state ) ), { name: blockType.name } ); + isDisabled = some( getBlocksByClientId( state, getClientIdsUnfolded( state ) ), { name: blockType.name } ); } const isContextual = isArray( blockType.parent ); diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 37be565161a50e..6db057e81e135c 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -44,8 +44,8 @@ const { getBlockName, getBlock, getBlocks, - getBlockIdsUnfolded, getBlockCount, + getClientIdsUnfolded, hasSelectedBlock, getSelectedBlock, getSelectedBlockClientId, @@ -1732,7 +1732,7 @@ describe( 'selectors', () => { } ); } ); - describe( 'getBlockIdsUnfolded', () => { + describe( 'getClientIdsUnfolded', () => { it( 'should return the ids for top-level blocks, any block referenced by a existing top-level shared block, and children of nested blocks.', () => { const state = { currentPost: {}, @@ -1774,7 +1774,7 @@ describe( 'selectors', () => { }, }, }; - expect( getBlockIdsUnfolded( state ) ).toEqual( [ + expect( getClientIdsUnfolded( state ) ).toEqual( [ 'uuid-6', 'uuid-8', 'uuid-2', From 4c00311997eb825e7b0922f9f0dcc57c1d9b7f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 26 Jul 2018 13:20:26 +0200 Subject: [PATCH 07/12] Refactor getClientIdsUnfolded to use flatMap --- packages/editor/src/store/selectors.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index 410540d2ea3762..b028212a11122f 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -3,6 +3,7 @@ */ import { castArray, + flatMap, find, first, get, @@ -599,13 +600,9 @@ const unfoldClientIds = ( state, block ) => { */ export const getClientIdsUnfolded = createSelector( ( state ) => { - const clientIds = []; const topLevelBlocks = getBlocks( state ); - topLevelBlocks.forEach( ( block ) => { - clientIds.push( block.clientId ); - clientIds.push( ...unfoldClientIds( state, block ) ); - } ); - return clientIds; + const getClientIdsFromBlock = ( block ) => [ block.clientId, ...unfoldClientIds( state, block ) ]; + return flatMap( topLevelBlocks, getClientIdsFromBlock ); }, ( state ) => [ state.editor.present.blockOrder, From c6a2b5705aa26afe2bf4c9cc19548f92e7267f7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 26 Jul 2018 13:58:08 +0200 Subject: [PATCH 08/12] Refactor unfoldClients to use flatMap --- packages/editor/src/store/selectors.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index b028212a11122f..4053d000fa085c 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -576,17 +576,14 @@ export const getBlocks = createSelector( * @return {Array} ids of referenced and inner blocks. */ const unfoldClientIds = ( state, block ) => { - const clientIds = []; - if ( block.name === 'core/block' ) { - clientIds.push( get( getSharedBlock( state, block.attributes.ref ), [ 'clientId' ], null ) ); - } - if ( block.innerBlocks.length > 0 ) { - block.innerBlocks.map( ( innerBlock ) => { - clientIds.push( innerBlock.clientId ); - clientIds.push( ...unfoldClientIds( state, innerBlock ) ); - } ); - } - return clientIds.filter( ( id ) => !! id ); + const getClientIdsFromInnerBlock = ( innerBlock ) => [ + innerBlock.clientId, + ...unfoldClientIds( state, innerBlock ), + ]; + return [ + block.name === 'core/block' ? get( getSharedBlock( state, block.attributes.ref ), [ 'clientId' ], null ) : null, + ...flatMap( block.innerBlocks, getClientIdsFromInnerBlock ), + ].filter( ( id ) => !! id ); }; /** From 3199792e887df6f193c78779034365e465954966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 26 Jul 2018 14:08:09 +0200 Subject: [PATCH 09/12] Extract getClientIdsFromSharedBlock --- packages/editor/src/store/selectors.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index 4053d000fa085c..bf10a74f803534 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -576,13 +576,17 @@ export const getBlocks = createSelector( * @return {Array} ids of referenced and inner blocks. */ const unfoldClientIds = ( state, block ) => { - const getClientIdsFromInnerBlock = ( innerBlock ) => [ + const getClientIdsFromSharedBlock = ( globalState, sharedBlock ) => + sharedBlock.name === 'core/block' ? + [ get( getSharedBlock( globalState, sharedBlock.attributes.ref ), [ 'clientId' ], null ) ] : + [ null ]; + const getClientIdsFromInnerBlock = ( globalState ) => ( innerBlock ) => [ innerBlock.clientId, - ...unfoldClientIds( state, innerBlock ), + ...unfoldClientIds( globalState, innerBlock ), ]; return [ - block.name === 'core/block' ? get( getSharedBlock( state, block.attributes.ref ), [ 'clientId' ], null ) : null, - ...flatMap( block.innerBlocks, getClientIdsFromInnerBlock ), + ...getClientIdsFromSharedBlock( state, block ), + ...flatMap( block.innerBlocks, getClientIdsFromInnerBlock( state ) ), ].filter( ( id ) => !! id ); }; From 2c52fb7b3a03472c92efc907e87ec3e766190ea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 26 Jul 2018 16:25:37 +0200 Subject: [PATCH 10/12] Unfold also shared blocks A shared block can contain nested blocks, so we need to unfold them. --- packages/editor/src/store/selectors.js | 14 ++++++++++---- packages/editor/src/store/test/selectors.js | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index bf10a74f803534..6e498ca1c31a6c 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -576,10 +576,16 @@ export const getBlocks = createSelector( * @return {Array} ids of referenced and inner blocks. */ const unfoldClientIds = ( state, block ) => { - const getClientIdsFromSharedBlock = ( globalState, sharedBlock ) => - sharedBlock.name === 'core/block' ? - [ get( getSharedBlock( globalState, sharedBlock.attributes.ref ), [ 'clientId' ], null ) ] : - [ null ]; + const getClientIdsFromSharedBlock = ( globalState, sharedBlock ) => { + if ( sharedBlock.name === 'core/block' ) { + const clientId = get( getSharedBlock( globalState, sharedBlock.attributes.ref ), [ 'clientId' ], null ); + return [ + clientId, + ...unfoldClientIds( globalState, ...getBlocksByClientId( globalState, clientId ) ), + ]; + } + return [ null ]; + }; const getClientIdsFromInnerBlock = ( globalState ) => ( innerBlock ) => [ innerBlock.clientId, ...unfoldClientIds( globalState, innerBlock ), diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 6db057e81e135c..8f19136f321bed 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -1749,9 +1749,14 @@ describe( 'selectors', () => { 'uuid-16': { clientId: 'uuid-16', name: 'core/quote', attributes: {} }, 'uuid-18': { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 } }, 'uuid-20': { clientId: 'uuid-20', name: 'core/gallery', attributes: {} }, + 'uuid-22': { clientId: 'uuid-22', name: 'core/block', attributes: { ref: 7 } }, + 'uuid-24': { clientId: 'uuid-24', name: 'core/columns', attributes: { } }, + 'uuid-26': { clientId: 'uuid-26', name: 'core/column', attributes: { } }, + 'uuid-28': { clientId: 'uuid-28', name: 'core/column', attributes: { } }, + 'uuid-30': { clientId: 'uuid-30', name: 'core/paragraph', attributes: { } }, }, blockOrder: { - '': [ 'uuid-6', 'uuid-8', 'uuid-10' ], + '': [ 'uuid-6', 'uuid-8', 'uuid-10', 'uuid-22' ], 'uuid-2': [ ], 'uuid-4': [ ], 'uuid-6': [ ], @@ -1762,6 +1767,10 @@ describe( 'selectors', () => { 'uuid-16': [ ], 'uuid-18': [ ], 'uuid-20': [ ], + 'uuid-22': [ ], + 'uuid-24': [ 'uuid-26', 'uuid-28' ], + 'uuid-26': [ ], + 'uuid-28': [ 'uuid-30' ], }, edits: {}, }, @@ -1771,6 +1780,7 @@ describe( 'selectors', () => { 1: { clientId: 'uuid-2', title: 'SharedImage' }, 3: { clientId: 'uuid-4', title: 'SharedParagraph' }, 5: { clientId: 'uuid-20', title: 'SharedGallery' }, + 7: { clientId: 'uuid-24', title: 'SharedColumns' }, }, }, }; @@ -1784,6 +1794,11 @@ describe( 'selectors', () => { 'uuid-14', 'uuid-18', 'uuid-20', + 'uuid-22', + 'uuid-24', + 'uuid-26', + 'uuid-28', + 'uuid-30', ] ); } ); } ); From aefead745a4e2e65db080c3654993c045d62b881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 27 Jul 2018 10:20:28 +0200 Subject: [PATCH 11/12] Bail early if no proper block --- packages/editor/src/store/selectors.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index 6e498ca1c31a6c..aad87a7b027053 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -576,6 +576,10 @@ export const getBlocks = createSelector( * @return {Array} ids of referenced and inner blocks. */ const unfoldClientIds = ( state, block ) => { + if ( ! block ) { + return [ null ]; + } + const getClientIdsFromSharedBlock = ( globalState, sharedBlock ) => { if ( sharedBlock.name === 'core/block' ) { const clientId = get( getSharedBlock( globalState, sharedBlock.attributes.ref ), [ 'clientId' ], null ); @@ -587,7 +591,7 @@ const unfoldClientIds = ( state, block ) => { return [ null ]; }; const getClientIdsFromInnerBlock = ( globalState ) => ( innerBlock ) => [ - innerBlock.clientId, + get( innerBlock, [ 'clientId' ], null ), ...unfoldClientIds( globalState, innerBlock ), ]; return [ From da45c30647cb66d03768065dbeb3a44430dcffd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Mon, 6 Aug 2018 11:03:20 +0200 Subject: [PATCH 12/12] Get rid of SharedBlocks logic --- packages/editor/src/store/selectors.js | 56 ++++----------------- packages/editor/src/store/test/selectors.js | 38 ++++++-------- 2 files changed, 25 insertions(+), 69 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index aad87a7b027053..4234d730196d17 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -566,58 +566,24 @@ export const getBlocks = createSelector( ); /** - * Given a block, returns an array containing all clientIds - * of referenced blocks (if it's a shared block) - * and innerBlocks. + * Returns an array containing the clientIds of the top-level blocks + * and their descendants of any depth (for nested blocks). * * @param {Object} state Global application state. - * @param {Object} block The block whose ids we want to unfold. * - * @return {Array} ids of referenced and inner blocks. + * @return {Array} ids of top-level and descendant blocks. */ -const unfoldClientIds = ( state, block ) => { - if ( ! block ) { - return [ null ]; - } - - const getClientIdsFromSharedBlock = ( globalState, sharedBlock ) => { - if ( sharedBlock.name === 'core/block' ) { - const clientId = get( getSharedBlock( globalState, sharedBlock.attributes.ref ), [ 'clientId' ], null ); - return [ - clientId, - ...unfoldClientIds( globalState, ...getBlocksByClientId( globalState, clientId ) ), - ]; - } - return [ null ]; - }; - const getClientIdsFromInnerBlock = ( globalState ) => ( innerBlock ) => [ - get( innerBlock, [ 'clientId' ], null ), - ...unfoldClientIds( globalState, innerBlock ), - ]; - return [ - ...getClientIdsFromSharedBlock( state, block ), - ...flatMap( block.innerBlocks, getClientIdsFromInnerBlock( state ) ), - ].filter( ( id ) => !! id ); -}; - -/** - * Returns an array containing the clientIds of the top-level blocks, - * all blocks referenced by any shared block, - * and all the inner blocks of existing top-level nested blocks. - * - * @param {Object} state Global application state. - * - * @return {Array} ids of top-level, referenced, and inner blocks. - */ -export const getClientIdsUnfolded = createSelector( +export const getClientIdsWithDescendants = createSelector( ( state ) => { - const topLevelBlocks = getBlocks( state ); - const getClientIdsFromBlock = ( block ) => [ block.clientId, ...unfoldClientIds( state, block ) ]; - return flatMap( topLevelBlocks, getClientIdsFromBlock ); + const getDescendants = ( clientIds ) => flatMap( clientIds, ( clientId ) => { + const descendants = getBlockOrder( state, clientId ); + return [ ...descendants, ...getDescendants( descendants ) ]; + } ); + const topLevelIds = getBlockOrder( state ); + return [ ...topLevelIds, ...getDescendants( topLevelIds ) ]; }, ( state ) => [ state.editor.present.blockOrder, - state.editor.present.blocksByClientId, ] ); @@ -1603,7 +1569,7 @@ export const getInserterItems = createSelector( let isDisabled = false; if ( ! hasBlockSupport( blockType.name, 'multiple', true ) ) { - isDisabled = some( getBlocksByClientId( state, getClientIdsUnfolded( state ) ), { name: blockType.name } ); + isDisabled = some( getBlocksByClientId( state, getClientIdsWithDescendants( state ) ), { name: blockType.name } ); } const isContextual = isArray( blockType.parent ); diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 8f19136f321bed..617ba9d9a53f10 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -45,7 +45,7 @@ const { getBlock, getBlocks, getBlockCount, - getClientIdsUnfolded, + getClientIdsWithDescendants, hasSelectedBlock, getSelectedBlock, getSelectedBlockClientId, @@ -1732,8 +1732,8 @@ describe( 'selectors', () => { } ); } ); - describe( 'getClientIdsUnfolded', () => { - it( 'should return the ids for top-level blocks, any block referenced by a existing top-level shared block, and children of nested blocks.', () => { + describe( 'getClientIdsWithDescendants', () => { + it( 'should return the ids for top-level blocks and their descendants of any depth (for nested blocks).', () => { const state = { currentPost: {}, editor: { @@ -1742,18 +1742,18 @@ describe( 'selectors', () => { 'uuid-2': { clientId: 'uuid-2', name: 'core/image', attributes: {} }, 'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph', attributes: {} }, 'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph', attributes: {} }, - 'uuid-8': { clientId: 'uuid-8', name: 'core/block', attributes: { ref: 1 } }, + 'uuid-8': { clientId: 'uuid-8', name: 'core/block', attributes: {} }, 'uuid-10': { clientId: 'uuid-10', name: 'core/columns', attributes: {} }, 'uuid-12': { clientId: 'uuid-12', name: 'core/column', attributes: {} }, 'uuid-14': { clientId: 'uuid-14', name: 'core/column', attributes: {} }, 'uuid-16': { clientId: 'uuid-16', name: 'core/quote', attributes: {} }, - 'uuid-18': { clientId: 'uuid-18', name: 'core/block', attributes: { ref: 5 } }, + 'uuid-18': { clientId: 'uuid-18', name: 'core/block', attributes: {} }, 'uuid-20': { clientId: 'uuid-20', name: 'core/gallery', attributes: {} }, - 'uuid-22': { clientId: 'uuid-22', name: 'core/block', attributes: { ref: 7 } }, - 'uuid-24': { clientId: 'uuid-24', name: 'core/columns', attributes: { } }, - 'uuid-26': { clientId: 'uuid-26', name: 'core/column', attributes: { } }, - 'uuid-28': { clientId: 'uuid-28', name: 'core/column', attributes: { } }, - 'uuid-30': { clientId: 'uuid-30', name: 'core/paragraph', attributes: { } }, + 'uuid-22': { clientId: 'uuid-22', name: 'core/block', attributes: {} }, + 'uuid-24': { clientId: 'uuid-24', name: 'core/columns', attributes: {} }, + 'uuid-26': { clientId: 'uuid-26', name: 'core/column', attributes: {} }, + 'uuid-28': { clientId: 'uuid-28', name: 'core/column', attributes: {} }, + 'uuid-30': { clientId: 'uuid-30', name: 'core/paragraph', attributes: {} }, }, blockOrder: { '': [ 'uuid-6', 'uuid-8', 'uuid-10', 'uuid-22' ], @@ -1765,7 +1765,7 @@ describe( 'selectors', () => { 'uuid-12': [ 'uuid-16' ], 'uuid-14': [ 'uuid-18' ], 'uuid-16': [ ], - 'uuid-18': [ ], + 'uuid-18': [ 'uuid-24' ], 'uuid-20': [ ], 'uuid-22': [ ], 'uuid-24': [ 'uuid-26', 'uuid-28' ], @@ -1775,26 +1775,16 @@ describe( 'selectors', () => { edits: {}, }, }, - sharedBlocks: { - data: { - 1: { clientId: 'uuid-2', title: 'SharedImage' }, - 3: { clientId: 'uuid-4', title: 'SharedParagraph' }, - 5: { clientId: 'uuid-20', title: 'SharedGallery' }, - 7: { clientId: 'uuid-24', title: 'SharedColumns' }, - }, - }, }; - expect( getClientIdsUnfolded( state ) ).toEqual( [ + expect( getClientIdsWithDescendants( state ) ).toEqual( [ 'uuid-6', 'uuid-8', - 'uuid-2', 'uuid-10', + 'uuid-22', 'uuid-12', - 'uuid-16', 'uuid-14', + 'uuid-16', 'uuid-18', - 'uuid-20', - 'uuid-22', 'uuid-24', 'uuid-26', 'uuid-28',