From 3616308b516ec2992bfda42e4e7914eede6bd07f Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 28 Nov 2024 11:38:10 +0800 Subject: [PATCH 01/16] Allow template part editing in write mode --- packages/block-editor/src/store/reducer.js | 23 +++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index edae9c392c37d..14af1bed290df 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -2287,11 +2287,24 @@ function getDerivedBlockEditingModesForTree( // If zoomed out, all blocks that aren't sections or the section root are // disabled. - // If the tree root is not in a section, set its editing mode to disabled. - if ( - isZoomedOut || - ! findParentInClientIdsList( state, clientId, sectionClientIds ) - ) { + if ( isZoomedOut ) { + derivedBlockEditingModes.set( clientId, 'disabled' ); + return; + } + + const isInSection = findParentInClientIdsList( + state, + clientId, + sectionClientIds + ); + if ( ! isInSection ) { + // If the block is not in a section, it should only be contentOnly if it's a template part. + // All other blocks should be disabled. + if ( block.name === 'core/template-part' ) { + derivedBlockEditingModes.set( clientId, 'contentOnly' ); + return; + } + derivedBlockEditingModes.set( clientId, 'disabled' ); return; } From e8c33f12bbfe1d2dda2a44e2a026def7712b9337 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 28 Nov 2024 11:49:03 +0800 Subject: [PATCH 02/16] Update unit tests for template parts --- .../block-editor/src/store/test/reducer.js | 75 ++++++++++++++----- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index dd1665d6736ad..5b2cf46e7b7a5 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -3975,38 +3975,58 @@ describe( 'state', () => { { type: 'UPDATE_SETTINGS', settings: { - [ sectionRootClientIdKey ]: '', + [ sectionRootClientIdKey ]: 'section-root', }, }, { type: 'RESET_BLOCKS', blocks: [ + { + name: 'core/template-part', + clientId: 'header', + attributes: {}, + innerBlocks: [], + }, { name: 'core/group', - clientId: 'group-1', + clientId: 'section-root', attributes: {}, innerBlocks: [ - { - name: 'core/paragraph', - clientId: 'paragraph-1', - attributes: {}, - innerBlocks: [], - }, { name: 'core/group', - clientId: 'group-2', + clientId: 'group-1', attributes: {}, innerBlocks: [ { name: 'core/paragraph', - clientId: 'paragraph-2', + clientId: 'paragraph-1', attributes: {}, innerBlocks: [], }, + { + name: 'core/group', + clientId: 'group-2', + attributes: {}, + innerBlocks: [ + { + name: 'core/paragraph', + clientId: + 'paragraph-2', + attributes: {}, + innerBlocks: [], + }, + ], + }, ], }, ], }, + { + name: 'core/template-part', + clientId: 'footer', + attributes: {}, + innerBlocks: [], + }, ], }, ], @@ -4022,7 +4042,10 @@ describe( 'state', () => { expect( initialState.derivedNavModeBlockEditingModes ).toEqual( new Map( Object.entries( { - '': 'contentOnly', // Section root. + '': 'disabled', + header: 'contentOnly', // Template part. + footer: 'contentOnly', // Template part. + 'section-root': 'contentOnly', // Section root. 'group-1': 'contentOnly', // Section block. 'paragraph-1': 'contentOnly', // Content block in section. 'group-2': 'disabled', // Non-content block in section. @@ -4047,7 +4070,10 @@ describe( 'state', () => { expect( derivedNavModeBlockEditingModes ).toEqual( new Map( Object.entries( { - '': 'contentOnly', + '': 'disabled', + header: 'contentOnly', + footer: 'contentOnly', + 'section-root': 'contentOnly', 'group-1': 'contentOnly', 'paragraph-1': 'contentOnly', } ) @@ -4060,7 +4086,7 @@ describe( 'state', () => { [ { type: 'INSERT_BLOCKS', - rootClientId: '', + rootClientId: 'section-root', blocks: [ { name: 'core/group', @@ -4091,7 +4117,10 @@ describe( 'state', () => { expect( derivedNavModeBlockEditingModes ).toEqual( new Map( Object.entries( { - '': 'contentOnly', // Section root. + '': 'disabled', // Section root. + header: 'contentOnly', // Template part. + footer: 'contentOnly', // Template part. + 'section-root': 'contentOnly', // Section root. 'group-1': 'contentOnly', // Section block. 'paragraph-1': 'contentOnly', // Content block in section. 'group-2': 'disabled', // Non-content block in section. @@ -4111,7 +4140,7 @@ describe( 'state', () => { type: 'MOVE_BLOCKS_TO_POSITION', clientIds: [ 'group-2' ], fromRootClientId: 'group-1', - toRootClientId: '', + toRootClientId: 'section-root', }, ], testReducer, @@ -4120,7 +4149,10 @@ describe( 'state', () => { expect( derivedNavModeBlockEditingModes ).toEqual( new Map( Object.entries( { - '': 'contentOnly', // Section root. + '': 'disabled', // Section root. + header: 'contentOnly', // Template part. + footer: 'contentOnly', // Template part. + 'section-root': 'contentOnly', // Section root. 'group-1': 'contentOnly', // Section block. 'paragraph-1': 'contentOnly', // Content block in section. 'group-2': 'contentOnly', // New section block. @@ -4148,10 +4180,13 @@ describe( 'state', () => { new Map( Object.entries( { '': 'disabled', - 'group-1': 'contentOnly', - 'paragraph-1': 'contentOnly', - 'group-2': 'contentOnly', - 'paragraph-2': 'contentOnly', + header: 'contentOnly', + footer: 'contentOnly', + 'section-root': 'disabled', + 'group-1': 'contentOnly', // New section root. + 'paragraph-1': 'contentOnly', // Section and content block + 'group-2': 'contentOnly', // Section. + 'paragraph-2': 'contentOnly', // Content block. } ) ) ); From 05542c695d423556abf492b9f0b49aec1a81c27c Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 27 Nov 2024 17:42:05 +0800 Subject: [PATCH 03/16] Refine block editing mode calculation - allow editing of inner template part content blocks --- packages/block-editor/src/store/reducer.js | 49 ++++++++++++++----- .../src/post-author-name/block.json | 6 ++- .../block-library/src/post-author/block.json | 6 ++- .../block-library/src/post-date/block.json | 3 +- .../src/post-featured-image/block.json | 9 ++-- .../block-library/src/post-title/block.json | 9 ++-- .../block-library/src/site-title/block.json | 6 ++- packages/blocks/src/api/utils.js | 12 +++++ 8 files changed, 75 insertions(+), 25 deletions(-) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 14af1bed290df..2765959a8333f 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -2212,6 +2212,10 @@ function traverseBlockTree( state, clientId, callback ) { * @return {string|undefined} The client ID of the parent block if found, undefined otherwise. */ function findParentInClientIdsList( state, clientId, clientIds ) { + if ( ! clientIds.length ) { + return; + } + let parent = state.blocks.parents.get( clientId ); while ( parent ) { if ( clientIds.includes( parent ) ) { @@ -2258,12 +2262,20 @@ function getDerivedBlockEditingModesForTree( // so the default block editing mode is set to disabled. const sectionRootClientId = state.settings?.[ sectionRootClientIdKey ]; const sectionClientIds = state.blocks.order.get( sectionRootClientId ); - const syncedPatternClientIds = Object.keys( - state.blocks.controlledInnerBlocks - ).filter( - ( clientId ) => - state.blocks.byClientId?.get( clientId )?.name === 'core/block' - ); + const templatePartClientIds = []; + const syncedPatternClientIds = []; + + Object.keys( state.blocks.controlledInnerBlocks ).forEach( ( clientId ) => { + const block = state.blocks.byClientId?.get( clientId ); + + if ( block?.name === 'core/template-part' ) { + templatePartClientIds.push( clientId ); + } + + if ( block?.name === 'core/block' ) { + syncedPatternClientIds.push( clientId ); + } + } ); traverseBlockTree( state, treeClientId, ( block ) => { const { clientId, name: blockName } = block; @@ -2292,21 +2304,34 @@ function getDerivedBlockEditingModesForTree( return; } - const isInSection = findParentInClientIdsList( + const isInSection = !! findParentInClientIdsList( state, clientId, sectionClientIds ); if ( ! isInSection ) { - // If the block is not in a section, it should only be contentOnly if it's a template part. - // All other blocks should be disabled. - if ( block.name === 'core/template-part' ) { + if ( clientId === '' ) { + derivedBlockEditingModes.set( clientId, 'disabled' ); + return; + } + + // Allow selection of template parts outside of sections. + if ( blockName === 'core/template-part' ) { derivedBlockEditingModes.set( clientId, 'contentOnly' ); return; } - derivedBlockEditingModes.set( clientId, 'disabled' ); - return; + const isInTemplatePart = !! findParentInClientIdsList( + state, + clientId, + templatePartClientIds + ); + // Allow contentOnly blocks in template parts outside of sections + // to be editable. Only disable blocks that aren't don't fit this criteria. + if ( ! isInTemplatePart && ! isContentBlock( blockName ) ) { + derivedBlockEditingModes.set( clientId, 'disabled' ); + return; + } } // Handle synced pattern content so the inner blocks of a synced pattern are diff --git a/packages/block-library/src/post-author-name/block.json b/packages/block-library/src/post-author-name/block.json index 68d2c49bd9105..23211f0bf5bf4 100644 --- a/packages/block-library/src/post-author-name/block.json +++ b/packages/block-library/src/post-author-name/block.json @@ -12,11 +12,13 @@ }, "isLink": { "type": "boolean", - "default": false + "default": false, + "role": "content" }, "linkTarget": { "type": "string", - "default": "_self" + "default": "_self", + "role": "content" } }, "usesContext": [ "postType", "postId" ], diff --git a/packages/block-library/src/post-author/block.json b/packages/block-library/src/post-author/block.json index d66498c8ee3df..c7f2f01550a61 100644 --- a/packages/block-library/src/post-author/block.json +++ b/packages/block-library/src/post-author/block.json @@ -26,11 +26,13 @@ }, "isLink": { "type": "boolean", - "default": false + "default": false, + "role": "content" }, "linkTarget": { "type": "string", - "default": "_self" + "default": "_self", + "role": "content" } }, "usesContext": [ "postType", "postId", "queryId" ], diff --git a/packages/block-library/src/post-date/block.json b/packages/block-library/src/post-date/block.json index 470bddae53bdf..dadc0d2f489fe 100644 --- a/packages/block-library/src/post-date/block.json +++ b/packages/block-library/src/post-date/block.json @@ -15,7 +15,8 @@ }, "isLink": { "type": "boolean", - "default": false + "default": false, + "role": "content" }, "displayType": { "type": "string", diff --git a/packages/block-library/src/post-featured-image/block.json b/packages/block-library/src/post-featured-image/block.json index 8b431ffc62579..3cd144caa0cf4 100644 --- a/packages/block-library/src/post-featured-image/block.json +++ b/packages/block-library/src/post-featured-image/block.json @@ -9,7 +9,8 @@ "attributes": { "isLink": { "type": "boolean", - "default": false + "default": false, + "role": "content" }, "aspectRatio": { "type": "string" @@ -30,11 +31,13 @@ "rel": { "type": "string", "attribute": "rel", - "default": "" + "default": "", + "role": "content" }, "linkTarget": { "type": "string", - "default": "_self" + "default": "_self", + "role": "content" }, "overlayColor": { "type": "string" diff --git a/packages/block-library/src/post-title/block.json b/packages/block-library/src/post-title/block.json index ecb5053d6cd39..5587d71b148d0 100644 --- a/packages/block-library/src/post-title/block.json +++ b/packages/block-library/src/post-title/block.json @@ -20,16 +20,19 @@ }, "isLink": { "type": "boolean", - "default": false + "default": false, + "role": "content" }, "rel": { "type": "string", "attribute": "rel", - "default": "" + "default": "", + "role": "content" }, "linkTarget": { "type": "string", - "default": "_self" + "default": "_self", + "role": "content" } }, "example": { diff --git a/packages/block-library/src/site-title/block.json b/packages/block-library/src/site-title/block.json index c75b1bc229beb..8edf6b945f9ce 100644 --- a/packages/block-library/src/site-title/block.json +++ b/packages/block-library/src/site-title/block.json @@ -20,11 +20,13 @@ }, "isLink": { "type": "boolean", - "default": true + "default": true, + "role": "content" }, "linkTarget": { "type": "string", - "default": "_self" + "default": "_self", + "role": "content" } }, "example": { diff --git a/packages/blocks/src/api/utils.js b/packages/blocks/src/api/utils.js index 1a21503649655..73c34ab3499d6 100644 --- a/packages/blocks/src/api/utils.js +++ b/packages/blocks/src/api/utils.js @@ -370,9 +370,21 @@ export const __experimentalGetBlockAttributesNamesByRole = ( ...args ) => { return getBlockAttributesNamesByRole( ...args ); }; +/** + * Checks if a block is a content block by examining its attributes. + * A block is considered a content block if it has at least one attribute + * with a role of 'content'. + * + * @param {string} name The name of the block to check. + * @return {boolean} Whether the block is a content block. + */ export function isContentBlock( name ) { const attributes = getBlockType( name )?.attributes; + if ( ! attributes ) { + return false; + } + return !! Object.keys( attributes )?.some( ( attributeKey ) => { const attribute = attributes[ attributeKey ]; return ( From 0ebb6ea27824b90dd39f02a9a48b695495f5dd0b Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 28 Nov 2024 15:01:21 +0800 Subject: [PATCH 04/16] Update tests for content blocks in template parts --- .../block-editor/src/store/test/reducer.js | 64 +++++++++++++++++-- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index 5b2cf46e7b7a5..b02650a003f41 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -4029,6 +4029,47 @@ describe( 'state', () => { }, ], }, + { + type: 'SET_HAS_CONTROLLED_INNER_BLOCKS', + clientId: 'header', + hasControlledInnerBlocks: true, + }, + { + type: 'REPLACE_INNER_BLOCKS', + rootClientId: 'header', + blocks: [ + { + name: 'core/group', + clientId: 'header-group', + attributes: {}, + innerBlocks: [ + { + name: 'core/paragraph', + clientId: 'header-paragraph', + attributes: {}, + innerBlocks: [], + }, + ], + }, + ], + }, + { + type: 'SET_HAS_CONTROLLED_INNER_BLOCKS', + clientId: 'footer', + hasControlledInnerBlocks: true, + }, + { + type: 'REPLACE_INNER_BLOCKS', + rootClientId: 'footer', + blocks: [ + { + name: 'core/paragraph', + clientId: 'footer-paragraph', + attributes: {}, + innerBlocks: [], + }, + ], + }, ], testReducer ); @@ -4044,7 +4085,10 @@ describe( 'state', () => { Object.entries( { '': 'disabled', header: 'contentOnly', // Template part. + 'header-group': 'disabled', // Content block in template part. + 'header-paragraph': 'contentOnly', // Content block in template part. footer: 'contentOnly', // Template part. + 'footer-paragraph': 'contentOnly', // Content block in template part. 'section-root': 'contentOnly', // Section root. 'group-1': 'contentOnly', // Section block. 'paragraph-1': 'contentOnly', // Content block in section. @@ -4071,8 +4115,11 @@ describe( 'state', () => { new Map( Object.entries( { '': 'disabled', - header: 'contentOnly', - footer: 'contentOnly', + header: 'contentOnly', // Template part. + 'header-group': 'disabled', // Content block in template part. + 'header-paragraph': 'contentOnly', // Content block in template part. + footer: 'contentOnly', // Template part. + 'footer-paragraph': 'contentOnly', // Content block in template part. 'section-root': 'contentOnly', 'group-1': 'contentOnly', 'paragraph-1': 'contentOnly', @@ -4119,7 +4166,10 @@ describe( 'state', () => { Object.entries( { '': 'disabled', // Section root. header: 'contentOnly', // Template part. + 'header-group': 'disabled', // Content block in template part. + 'header-paragraph': 'contentOnly', // Content block in template part. footer: 'contentOnly', // Template part. + 'footer-paragraph': 'contentOnly', // Content block in template part. 'section-root': 'contentOnly', // Section root. 'group-1': 'contentOnly', // Section block. 'paragraph-1': 'contentOnly', // Content block in section. @@ -4151,7 +4201,10 @@ describe( 'state', () => { Object.entries( { '': 'disabled', // Section root. header: 'contentOnly', // Template part. + 'header-group': 'disabled', // Content block in template part. + 'header-paragraph': 'contentOnly', // Content block in template part. footer: 'contentOnly', // Template part. + 'footer-paragraph': 'contentOnly', // Content block in template part. 'section-root': 'contentOnly', // Section root. 'group-1': 'contentOnly', // Section block. 'paragraph-1': 'contentOnly', // Content block in section. @@ -4180,8 +4233,11 @@ describe( 'state', () => { new Map( Object.entries( { '': 'disabled', - header: 'contentOnly', - footer: 'contentOnly', + header: 'contentOnly', // Template part. + 'header-group': 'disabled', // Content block in template part. + 'header-paragraph': 'contentOnly', // Content block in template part. + footer: 'contentOnly', // Template part. + 'footer-paragraph': 'contentOnly', // Content block in template part. 'section-root': 'disabled', 'group-1': 'contentOnly', // New section root. 'paragraph-1': 'contentOnly', // Section and content block From 700f05951a26f35f73ec146b04976fe17fcccfde Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 28 Nov 2024 15:10:02 +0800 Subject: [PATCH 05/16] Add content role to site logo block --- packages/block-library/src/site-logo/block.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/site-logo/block.json b/packages/block-library/src/site-logo/block.json index 3bdbdc1b809ab..1f5b3a5525e3e 100644 --- a/packages/block-library/src/site-logo/block.json +++ b/packages/block-library/src/site-logo/block.json @@ -12,11 +12,13 @@ }, "isLink": { "type": "boolean", - "default": true + "default": true, + "role": "content" }, "linkTarget": { "type": "string", - "default": "_self" + "default": "_self", + "role": "content" }, "shouldSyncIcon": { "type": "boolean" From 508bd39d72b2856bc9bb3f1bd6ce03c31af2c13b Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 4 Dec 2024 13:58:57 +0800 Subject: [PATCH 06/16] Consider template parts a section in navigation mode --- .../src/store/private-selectors.js | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/store/private-selectors.js b/packages/block-editor/src/store/private-selectors.js index c46778d889b3e..72b87a59e8f57 100644 --- a/packages/block-editor/src/store/private-selectors.js +++ b/packages/block-editor/src/store/private-selectors.js @@ -502,13 +502,23 @@ export const getParentSectionBlock = ( state, clientId ) => { * @return {boolean} Whether the block is a content locking parent. */ export function isSectionBlock( state, clientId ) { + const blockName = getBlockName( state, clientId ); + if ( + blockName === 'core/block' || + getTemplateLock( state, clientId ) === 'contentOnly' + ) { + return true; + } + + // Template parts become sections in navigation mode. + const _isNavigationMode = isNavigationMode( state ); + if ( _isNavigationMode && blockName === 'core/template-part' ) { + return true; + } + const sectionRootClientId = getSectionRootClientId( state ); const sectionClientIds = getBlockOrder( state, sectionRootClientId ); - return ( - getBlockName( state, clientId ) === 'core/block' || - getTemplateLock( state, clientId ) === 'contentOnly' || - ( isNavigationMode( state ) && sectionClientIds.includes( clientId ) ) - ); + return _isNavigationMode && sectionClientIds.includes( clientId ); } /** From 5e68eea9efe39e54d147f3980fe8897486144066 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 6 Dec 2024 15:14:03 +0800 Subject: [PATCH 07/16] Allow content blocks to be explicitly disabled even when they have a derived mode First iteration of handling disabled blocks as derived block editing modes --- packages/block-editor/src/store/reducer.js | 71 ++++++- packages/block-editor/src/store/selectors.js | 4 +- .../block-editor/src/store/test/reducer.js | 196 +++++------------- 3 files changed, 111 insertions(+), 160 deletions(-) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 2765959a8333f..d916d34be67a7 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -2186,19 +2186,19 @@ function getBlockTreeBlock( state, clientId ) { * The callback receives the current block as its argument. */ function traverseBlockTree( state, clientId, callback ) { - const parentTree = getBlockTreeBlock( state, clientId ); - if ( ! parentTree ) { + const tree = getBlockTreeBlock( state, clientId ); + if ( ! tree ) { return; } - callback( parentTree ); + callback( tree ); - if ( ! parentTree?.innerBlocks?.length ) { + if ( ! tree?.innerBlocks?.length ) { return; } - for ( const block of parentTree?.innerBlocks ) { - traverseBlockTree( state, block.clientId, callback ); + for ( const innerBlock of tree?.innerBlocks ) { + traverseBlockTree( state, innerBlock.clientId, callback ); } } @@ -2217,7 +2217,7 @@ function findParentInClientIdsList( state, clientId, clientIds ) { } let parent = state.blocks.parents.get( clientId ); - while ( parent ) { + while ( parent !== undefined ) { if ( clientIds.includes( parent ) ) { return parent; } @@ -2225,6 +2225,24 @@ function findParentInClientIdsList( state, clientId, clientIds ) { } } +/** + * Retrieves the block editing mode of the ancestor block of a given block. + * + * @param {Object} state The current state object. + * @param {string} clientId The client ID of the block to find the ancestor block for. + * + * @return {string|undefined} The block editing mode of the ancestor block, or undefined if not found. + */ +function getAncestorBlockEditingMode( state, clientId ) { + let parent = state.blocks.parents.get( clientId ); + while ( parent !== undefined ) { + if ( state.blockEditingModes.has( parent ) ) { + return state.blockEditingModes.get( parent ); + } + parent = state.blocks.parents.get( parent ); + } +} + /** * Checks if a block has any bindings in its metadata attributes. * @@ -2262,6 +2280,9 @@ function getDerivedBlockEditingModesForTree( // so the default block editing mode is set to disabled. const sectionRootClientId = state.settings?.[ sectionRootClientIdKey ]; const sectionClientIds = state.blocks.order.get( sectionRootClientId ); + const hasDisabledBlocks = state.blockEditingModes + .entries() + .some( ( [ , mode ] ) => mode === 'disabled' ); const templatePartClientIds = []; const syncedPatternClientIds = []; @@ -2279,6 +2300,27 @@ function getDerivedBlockEditingModesForTree( traverseBlockTree( state, treeClientId, ( block ) => { const { clientId, name: blockName } = block; + + // If the block already has an explicit block editing mode set, + // don't override it. + if ( state.blockEditingModes.has( clientId ) ) { + return; + } + + // An explicitly disabled block editing mode cascades to all its children. + // Check ancestors of the current block to see if the nearest one with a block + // editing mode set is disabled. + if ( hasDisabledBlocks ) { + const ancestorBlockEditingMode = getAncestorBlockEditingMode( + state, + clientId + ); + if ( ancestorBlockEditingMode === 'disabled' ) { + derivedBlockEditingModes.set( clientId, 'disabled' ); + return; + } + } + if ( isZoomedOut || isNavMode ) { // If the root block is the section root set its editing mode to contentOnly. if ( clientId === sectionRootClientId ) { @@ -2327,7 +2369,7 @@ function getDerivedBlockEditingModesForTree( templatePartClientIds ); // Allow contentOnly blocks in template parts outside of sections - // to be editable. Only disable blocks that aren't don't fit this criteria. + // to be editable. Only disable blocks that don't fit this criteria. if ( ! isInTemplatePart && ! isContentBlock( blockName ) ) { derivedBlockEditingModes.set( clientId, 'disabled' ); return; @@ -2369,7 +2411,7 @@ function getDerivedBlockEditingModesForTree( } } - if ( blockName && isContentBlock( blockName ) ) { + if ( blockName && isContentBlock( blockName, clientId ) ) { derivedBlockEditingModes.set( clientId, 'contentOnly' ); return; } @@ -2598,11 +2640,16 @@ export function withDerivedBlockEditingModes( reducer ) { } break; } + case 'SET_BLOCK_EDITING_MODE': + case 'UNSET_BLOCK_EDITING_MODE': case 'SET_HAS_CONTROLLED_INNER_BLOCKS': { - const updatedBlock = nextState.blocks.tree.get( + const updatedBlock = getBlockTreeBlock( + nextState, action.clientId ); - // The block might have been removed. + + // The block might have been removed in which case it'll be + // handled by the `REMOVE_BLOCKS` action. if ( ! updatedBlock ) { break; } @@ -2611,6 +2658,7 @@ export function withDerivedBlockEditingModes( reducer ) { getDerivedBlockEditingModesUpdates( { prevState: state, nextState, + removedClientIds: [ action.clientId ], addedBlocks: [ updatedBlock ], isNavMode: false, } ); @@ -2618,6 +2666,7 @@ export function withDerivedBlockEditingModes( reducer ) { getDerivedBlockEditingModesUpdates( { prevState: state, nextState, + removedClientIds: [ action.clientId ], addedBlocks: [ updatedBlock ], isNavMode: true, } ); diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 7f8ddc7a4be31..31ee6778da8d0 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -3087,9 +3087,7 @@ export const getBlockEditingMode = createRegistrySelector( const isContent = hasContentRoleAttribute( name ); return isContent ? 'contentOnly' : 'disabled'; } - // Otherwise, check if there's an ancestor that is contentOnly - const parentMode = getBlockEditingMode( state, rootClientId ); - return parentMode === 'contentOnly' ? 'default' : parentMode; + return 'default'; } ); diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index b02650a003f41..26398d87fba5d 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -12,8 +12,7 @@ import { createBlock, privateApis, } from '@wordpress/blocks'; -import { combineReducers, select } from '@wordpress/data'; -import { store as preferencesStore } from '@wordpress/preferences'; +import { combineReducers } from '@wordpress/data'; /** * Internal dependencies @@ -3576,6 +3575,7 @@ describe( 'state', () => { blocks, settings, zoomLevel, + blockEditingModes, } ) ); @@ -3598,15 +3598,6 @@ describe( 'state', () => { describe( 'edit mode', () => { let initialState; beforeAll( () => { - select.mockImplementation( ( storeName ) => { - if ( storeName === preferencesStore ) { - return { - get: jest.fn( () => 'edit' ), - }; - } - return select( storeName ); - } ); - initialState = dispatchActions( [ { @@ -3651,10 +3642,6 @@ describe( 'state', () => { ); } ); - afterAll( () => { - select.mockRestore(); - } ); - it( 'returns no block editing modes when zoomed out / navigation mode are not active and there are no synced patterns', () => { expect( initialState.derivedBlockEditingModes ).toEqual( new Map() @@ -3665,15 +3652,6 @@ describe( 'state', () => { describe( 'synced patterns', () => { let initialState; beforeAll( () => { - select.mockImplementation( ( storeName ) => { - if ( storeName === preferencesStore ) { - return { - get: jest.fn( () => 'edit' ), - }; - } - return select( storeName ); - } ); - // Simulates how the editor typically inserts controlled blocks, // - first the pattern is inserted with no inner blocks. // - next the pattern is marked as a controlled block. @@ -3818,10 +3796,6 @@ describe( 'state', () => { ); } ); - afterAll( () => { - select.mockRestore(); - } ); - it( 'returns the expected block editing modes for synced patterns', () => { // Only the parent pattern and its own children that have bindings // are in contentOnly mode. All other blocks are disabled. @@ -3840,60 +3814,8 @@ describe( 'state', () => { ); } ); - it( 'removes block editing modes when synced patterns are removed', () => { - const { derivedBlockEditingModes } = dispatchActions( - [ - { - type: 'REMOVE_BLOCKS', - clientIds: [ 'root-pattern' ], - }, - ], - testReducer, - initialState - ); - - expect( derivedBlockEditingModes ).toEqual( new Map() ); - } ); - - it( 'returns the expected block editing modes for synced patterns when switching to navigation mode', () => { - select.mockImplementation( ( storeName ) => { - if ( storeName === preferencesStore ) { - return { - get: jest.fn( () => 'navigation' ), - }; - } - return select( storeName ); - } ); - - const { - derivedBlockEditingModes, - derivedNavModeBlockEditingModes, - } = dispatchActions( - [ - { - type: 'SET_EDITOR_MODE', - mode: 'navigation', - }, - ], - testReducer, - initialState - ); - - expect( derivedBlockEditingModes ).toEqual( - new Map( - Object.entries( { - 'pattern-paragraph': 'disabled', - 'pattern-group': 'disabled', - 'pattern-paragraph-with-overrides': 'contentOnly', // Pattern child with bindings. - 'nested-pattern': 'disabled', - 'nested-paragraph': 'disabled', - 'nested-group': 'disabled', - 'nested-paragraph-with-overrides': 'disabled', - } ) - ) - ); - - expect( derivedNavModeBlockEditingModes ).toEqual( + it( 'returns the expected block editing modes for synced patterns in navigation mode', () => { + expect( initialState.derivedNavModeBlockEditingModes ).toEqual( new Map( Object.entries( { '': 'contentOnly', // Section root. @@ -3912,15 +3834,21 @@ describe( 'state', () => { } ) ) ); + } ); - select.mockImplementation( ( storeName ) => { - if ( storeName === preferencesStore ) { - return { - get: jest.fn( () => 'edit' ), - }; - } - return select( storeName ); - } ); + it( 'removes block editing modes when synced patterns are removed', () => { + const { derivedBlockEditingModes } = dispatchActions( + [ + { + type: 'REMOVE_BLOCKS', + clientIds: [ 'root-pattern' ], + }, + ], + testReducer, + initialState + ); + + expect( derivedBlockEditingModes ).toEqual( new Map() ); } ); it( 'returns the expected block editing modes for synced patterns when switching to zoomed out mode', () => { @@ -3961,15 +3889,6 @@ describe( 'state', () => { let initialState; beforeAll( () => { - select.mockImplementation( ( storeName ) => { - if ( storeName === preferencesStore ) { - return { - get: jest.fn( () => 'navigation' ), - }; - } - return select( storeName ); - } ); - initialState = dispatchActions( [ { @@ -4075,10 +3994,6 @@ describe( 'state', () => { ); } ); - afterAll( () => { - select.mockRestore(); - } ); - it( 'returns the expected block editing modes', () => { expect( initialState.derivedNavModeBlockEditingModes ).toEqual( new Map( @@ -4099,6 +4014,38 @@ describe( 'state', () => { ); } ); + it( 'allows content blocks to be disabled explicitly using the block editing mode', () => { + const { derivedNavModeBlockEditingModes } = dispatchActions( + [ + { + type: 'SET_BLOCK_EDITING_MODE', + clientId: 'paragraph-1', + mode: 'disabled', + }, + ], + testReducer, + initialState + ); + + expect( derivedNavModeBlockEditingModes ).toEqual( + new Map( + Object.entries( { + '': 'disabled', + header: 'contentOnly', + 'header-group': 'disabled', + 'header-paragraph': 'contentOnly', + footer: 'contentOnly', + 'footer-paragraph': 'contentOnly', + 'section-root': 'contentOnly', + 'group-1': 'contentOnly', + 'paragraph-1': 'disabled', // Block explicitly disabled. + 'group-2': 'disabled', + 'paragraph-2': 'contentOnly', + } ) + ) + ); + } ); + it( 'removes block editing modes when blocks are removed', () => { const { derivedNavModeBlockEditingModes } = dispatchActions( [ @@ -4315,49 +4262,6 @@ describe( 'state', () => { ); } ); - it( 'overrides navigation mode', () => { - select.mockImplementation( ( storeName ) => { - if ( storeName === preferencesStore ) { - return { - get: jest.fn( () => 'navigation' ), - }; - } - return select( storeName ); - } ); - - const { derivedBlockEditingModes } = dispatchActions( - [ - { - type: 'SET_EDITOR_MODE', - mode: 'navigation', - }, - ], - testReducer, - initialState - ); - - expect( derivedBlockEditingModes ).toEqual( - new Map( - Object.entries( { - '': 'contentOnly', // Section root. - 'group-1': 'contentOnly', // Section block. - 'paragraph-1': 'disabled', - 'group-2': 'disabled', - 'paragraph-2': 'disabled', - } ) - ) - ); - - select.mockImplementation( ( storeName ) => { - if ( storeName === preferencesStore ) { - return { - get: jest.fn( () => 'edit' ), - }; - } - return select( storeName ); - } ); - } ); - it( 'removes block editing modes when blocks are removed', () => { const { derivedBlockEditingModes } = dispatchActions( [ From f609354411b86eb5f6373dda88b51bb95548f2d4 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Tue, 10 Dec 2024 09:23:19 +0800 Subject: [PATCH 08/16] Avoid showing template parts when edting a page in Write Mode --- .../disable-non-page-content-blocks.js | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/packages/editor/src/components/provider/disable-non-page-content-blocks.js b/packages/editor/src/components/provider/disable-non-page-content-blocks.js index ae4fd1075fc26..c47a7e5d7de7f 100644 --- a/packages/editor/src/components/provider/disable-non-page-content-blocks.js +++ b/packages/editor/src/components/provider/disable-non-page-content-blocks.js @@ -16,9 +16,13 @@ import usePostContentBlocks from './use-post-content-blocks'; */ export default function DisableNonPageContentBlocks() { const contentOnlyIds = usePostContentBlocks(); - const templateParts = useSelect( ( select ) => { - const { getBlocksByName } = select( blockEditorStore ); - return getBlocksByName( 'core/template-part' ); + const { templateParts, isNavigationMode } = useSelect( ( select ) => { + const { getBlocksByName, isNavigationMode: _isNavigationMode } = + select( blockEditorStore ); + return { + templateParts: getBlocksByName( 'core/template-part' ), + isNavigationMode: _isNavigationMode(), + }; }, [] ); const disabledIds = useSelect( ( select ) => { @@ -41,8 +45,10 @@ export default function DisableNonPageContentBlocks() { for ( const clientId of contentOnlyIds ) { setBlockEditingMode( clientId, 'contentOnly' ); } - for ( const clientId of templateParts ) { - setBlockEditingMode( clientId, 'contentOnly' ); + if ( ! isNavigationMode ) { + for ( const clientId of templateParts ) { + setBlockEditingMode( clientId, 'contentOnly' ); + } } for ( const clientId of disabledIds ) { setBlockEditingMode( clientId, 'disabled' ); @@ -55,15 +61,23 @@ export default function DisableNonPageContentBlocks() { for ( const clientId of contentOnlyIds ) { unsetBlockEditingMode( clientId ); } - for ( const clientId of templateParts ) { - unsetBlockEditingMode( clientId ); + if ( ! isNavigationMode ) { + for ( const clientId of templateParts ) { + unsetBlockEditingMode( clientId ); + } } for ( const clientId of disabledIds ) { unsetBlockEditingMode( clientId ); } } ); }; - }, [ templateParts, contentOnlyIds, disabledIds, registry ] ); + }, [ + templateParts, + contentOnlyIds, + disabledIds, + isNavigationMode, + registry, + ] ); return null; } From d6243e47aa023c58308fd5b1c95459dabdb009d3 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Tue, 10 Dec 2024 09:42:49 +0800 Subject: [PATCH 09/16] entries().some is not widely available, so switch to Array.from().some --- packages/block-editor/src/store/reducer.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index d916d34be67a7..7520820f83dec 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -2280,9 +2280,9 @@ function getDerivedBlockEditingModesForTree( // so the default block editing mode is set to disabled. const sectionRootClientId = state.settings?.[ sectionRootClientIdKey ]; const sectionClientIds = state.blocks.order.get( sectionRootClientId ); - const hasDisabledBlocks = state.blockEditingModes - .entries() - .some( ( [ , mode ] ) => mode === 'disabled' ); + const hasDisabledBlocks = Array.from( state.blockEditingModes ).some( + ( [ , mode ] ) => mode === 'disabled' + ); const templatePartClientIds = []; const syncedPatternClientIds = []; From adde4d23f98ce9247f8b819b48dea9a21a4552c7 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Tue, 10 Dec 2024 09:43:09 +0800 Subject: [PATCH 10/16] Update test to show explicitly disabled block --- packages/block-editor/src/store/test/reducer.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index 26398d87fba5d..6706ff2fbb59e 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -4015,7 +4015,10 @@ describe( 'state', () => { } ); it( 'allows content blocks to be disabled explicitly using the block editing mode', () => { - const { derivedNavModeBlockEditingModes } = dispatchActions( + const { + derivedNavModeBlockEditingModes, + blockEditingModes: _blockEditingModes, + } = dispatchActions( [ { type: 'SET_BLOCK_EDITING_MODE', @@ -4027,6 +4030,15 @@ describe( 'state', () => { initialState ); + // Paragraph 1 is explicitly disabled and omitted from the + // derived block editing modes. + expect( _blockEditingModes ).toEqual( + new Map( + Object.entries( { + 'paragraph-1': 'disabled', + } ) + ) + ); expect( derivedNavModeBlockEditingModes ).toEqual( new Map( Object.entries( { @@ -4038,7 +4050,6 @@ describe( 'state', () => { 'footer-paragraph': 'contentOnly', 'section-root': 'contentOnly', 'group-1': 'contentOnly', - 'paragraph-1': 'disabled', // Block explicitly disabled. 'group-2': 'disabled', 'paragraph-2': 'contentOnly', } ) From 0d4cf1ddd13021e2d4642eb47d255228dea1d6b4 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 11 Dec 2024 11:09:13 +0800 Subject: [PATCH 11/16] Update unit tests --- .../src/store/test/private-selectors.js | 62 ++++++++++++++++--- .../block-editor/src/store/test/selectors.js | 23 +++++++ 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/packages/block-editor/src/store/test/private-selectors.js b/packages/block-editor/src/store/test/private-selectors.js index 268d463f227d4..07c133dbacafe 100644 --- a/packages/block-editor/src/store/test/private-selectors.js +++ b/packages/block-editor/src/store/test/private-selectors.js @@ -122,6 +122,7 @@ describe( 'private selectors', () => { '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f': {}, }, blockEditingModes: new Map( [] ), + derivedBlockEditingModes: new Map( [] ), }; const hasContentRoleAttribute = jest.fn( () => false ); @@ -142,6 +143,7 @@ describe( 'private selectors', () => { const state = { ...baseState, blockEditingModes: new Map( [] ), + derivedBlockEditingModes: new Map( [] ), }; expect( isBlockSubtreeDisabled( @@ -157,6 +159,12 @@ describe( 'private selectors', () => { blockEditingModes: new Map( [ [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], ] ), + derivedBlockEditingModes: new Map( [ + [ 'b26fc763-417d-4f01-b81c-2ec61e14a972', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], + [ 'b3247f75-fd94-4fef-97f9-5bfd162cc416', 'disabled' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'disabled' ], + ] ), }; expect( isBlockSubtreeDisabled( @@ -166,10 +174,18 @@ describe( 'private selectors', () => { ).toBe( true ); } ); - it( 'should return true when top level block is disabled via inheritence and there are no editing modes within it', () => { + it( 'should return true when top level block is disabled via inheritance and there are no editing modes within it', () => { const state = { ...baseState, blockEditingModes: new Map( [ [ '', 'disabled' ] ] ), + derivedBlockEditingModes: new Map( [ + [ '6cf70164-9097-4460-bcbf-200560546988', 'disabled' ], + [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], + [ 'b26fc763-417d-4f01-b81c-2ec61e14a972', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], + [ 'b3247f75-fd94-4fef-97f9-5bfd162cc416', 'disabled' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'disabled' ], + ] ), }; expect( isBlockSubtreeDisabled( @@ -186,6 +202,11 @@ describe( 'private selectors', () => { [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], [ 'b3247f75-fd94-4fef-97f9-5bfd162cc416', 'disabled' ], ] ), + derivedBlockEditingModes: new Map( [ + [ 'b26fc763-417d-4f01-b81c-2ec61e14a972', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'disabled' ], + ] ), }; expect( isBlockSubtreeDisabled( @@ -202,6 +223,11 @@ describe( 'private selectors', () => { [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], [ 'b3247f75-fd94-4fef-97f9-5bfd162cc416', 'default' ], ] ), + derivedBlockEditingModes: new Map( [ + [ 'b26fc763-417d-4f01-b81c-2ec61e14a972', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'disabled' ], + ] ), }; expect( isBlockSubtreeDisabled( @@ -218,6 +244,13 @@ describe( 'private selectors', () => { [ '', 'disabled' ], [ 'b3247f75-fd94-4fef-97f9-5bfd162cc416', 'default' ], ] ), + derivedBlockEditingModes: new Map( [ + [ '6cf70164-9097-4460-bcbf-200560546988', 'disabled' ], + [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], + [ 'b26fc763-417d-4f01-b81c-2ec61e14a972', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'disabled' ], + ] ), }; expect( isBlockSubtreeDisabled( @@ -303,6 +336,7 @@ describe( 'private selectors', () => { const state = { ...baseState, blockEditingModes: new Map( [] ), + derivedBlockEditingModes: new Map( [] ), }; expect( getEnabledClientIdsTree( state ) ).toEqual( [ { @@ -340,6 +374,7 @@ describe( 'private selectors', () => { const state = { ...baseState, blockEditingModes: new Map( [] ), + derivedBlockEditingModes: new Map( [] ), }; expect( getEnabledClientIdsTree( @@ -375,6 +410,10 @@ describe( 'private selectors', () => { [ 'b26fc763-417d-4f01-b81c-2ec61e14a972', 'contentOnly' ], [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'contentOnly' ], ] ), + derivedBlockEditingModes: new Map( [ + [ '6cf70164-9097-4460-bcbf-200560546988', 'disabled' ], + [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], + ] ), }; expect( getEnabledClientIdsTree( state ) ).toEqual( [ { @@ -412,6 +451,7 @@ describe( 'private selectors', () => { ] ), }, blockEditingModes: new Map(), + derivedBlockEditingModes: new Map(), }; expect( getEnabledBlockParents( @@ -433,7 +473,7 @@ describe( 'private selectors', () => { ], [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', - '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', + 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', ], [ '4c2b7140-fffd-44b4-b2a7-820c670a6514', @@ -442,6 +482,7 @@ describe( 'private selectors', () => { ] ), order: new Map( [ + [ '', [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337' ] ], [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', [ @@ -453,12 +494,15 @@ describe( 'private selectors', () => { 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', [ '4c2b7140-fffd-44b4-b2a7-820c670a6514' ], ], - [ '', [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337' ] ], ] ), }, blockEditingModes: new Map( [ [ '', 'disabled' ], - [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'default' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'default' ], + ] ), + derivedBlockEditingModes: new Map( [ + [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], ] ), blockListSettings: {}, }; @@ -467,10 +511,7 @@ describe( 'private selectors', () => { state, '4c2b7140-fffd-44b4-b2a7-820c670a6514' ) - ).toEqual( [ - '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', - 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', - ] ); + ).toEqual( [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c' ] ); } ); it( 'should order from bottom to top if ascending is true', () => { @@ -493,6 +534,7 @@ describe( 'private selectors', () => { ], ] ), order: new Map( [ + [ '', [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337' ] ], [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f' ], @@ -505,13 +547,15 @@ describe( 'private selectors', () => { 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', [ '4c2b7140-fffd-44b4-b2a7-820c670a6514' ], ], - [ '', [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337' ] ], ] ), }, blockEditingModes: new Map( [ [ '', 'disabled' ], [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'default' ], ] ), + derivedBlockEditingModes: new Map( [ + [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], + ] ), blockListSettings: {}, }; expect( diff --git a/packages/block-editor/src/store/test/selectors.js b/packages/block-editor/src/store/test/selectors.js index 51949bfd468ca..388d592787b66 100644 --- a/packages/block-editor/src/store/test/selectors.js +++ b/packages/block-editor/src/store/test/selectors.js @@ -4465,6 +4465,7 @@ describe( 'getBlockEditingMode', () => { '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f': {}, }, blockEditingModes: new Map( [] ), + derivedBlockEditingModes: new Map( [] ), }; const hasContentRoleAttribute = jest.fn( () => false ); @@ -4519,6 +4520,13 @@ describe( 'getBlockEditingMode', () => { blockEditingModes: new Map( [ [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], ] ), + derivedBlockEditingModes: new Map( [ + [ 'b26fc763-417d-4f01-b81c-2ec61e14a972', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], + [ 'b3247f75-fd94-4fef-97f9-5bfd162cc416', 'disabled' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fed515b958s', 'disabled' ], + ] ), }; expect( getBlockEditingMode( state, 'b3247f75-fd94-4fef-97f9-5bfd162cc416' ) @@ -4545,6 +4553,12 @@ describe( 'getBlockEditingMode', () => { [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'default' ], [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], ] ), + derivedBlockEditingModes: new Map( [ + [ '6cf70164-9097-4460-bcbf-200560546988', 'disabled' ], + [ 'b3247f75-fd94-4fef-97f9-5bfd162cc416', 'disabled' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fed515b958s', 'disabled' ], + ] ), }; expect( getBlockEditingMode( state, 'b3247f75-fd94-4fef-97f9-5bfd162cc416' ) @@ -4555,6 +4569,15 @@ describe( 'getBlockEditingMode', () => { const state = { ...baseState, blockEditingModes: new Map( [ [ '', 'disabled' ] ] ), + derivedBlockEditingModes: new Map( [ + [ '6cf70164-9097-4460-bcbf-200560546988', 'disabled' ], + [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', 'disabled' ], + [ 'b26fc763-417d-4f01-b81c-2ec61e14a972', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fe9515b958f', 'disabled' ], + [ 'b3247f75-fd94-4fef-97f9-5bfd162cc416', 'disabled' ], + [ 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', 'disabled' ], + [ '9b9c5c3f-2e46-4f02-9e14-9fed515b958s', 'disabled' ], + ] ), }; expect( getBlockEditingMode( state, 'b3247f75-fd94-4fef-97f9-5bfd162cc416' ) From 673f4b05686f04a9e402555e7e296eec7ac12656 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 11 Dec 2024 13:17:23 +0800 Subject: [PATCH 12/16] Remove unused argument --- packages/block-editor/src/store/reducer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 7520820f83dec..e21875016a589 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -2411,7 +2411,7 @@ function getDerivedBlockEditingModesForTree( } } - if ( blockName && isContentBlock( blockName, clientId ) ) { + if ( blockName && isContentBlock( blockName ) ) { derivedBlockEditingModes.set( clientId, 'contentOnly' ); return; } From 80d9048459cb4e8677dbc99f5485408158e469ef Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 11 Dec 2024 14:35:39 +0800 Subject: [PATCH 13/16] Add optimization for disabled blocks calculation --- packages/block-editor/src/store/reducer.js | 50 +++++++++++----------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index e21875016a589..da528d94de6da 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -2225,24 +2225,6 @@ function findParentInClientIdsList( state, clientId, clientIds ) { } } -/** - * Retrieves the block editing mode of the ancestor block of a given block. - * - * @param {Object} state The current state object. - * @param {string} clientId The client ID of the block to find the ancestor block for. - * - * @return {string|undefined} The block editing mode of the ancestor block, or undefined if not found. - */ -function getAncestorBlockEditingMode( state, clientId ) { - let parent = state.blocks.parents.get( clientId ); - while ( parent !== undefined ) { - if ( state.blockEditingModes.has( parent ) ) { - return state.blockEditingModes.get( parent ); - } - parent = state.blocks.parents.get( parent ); - } -} - /** * Checks if a block has any bindings in its metadata attributes. * @@ -2307,14 +2289,32 @@ function getDerivedBlockEditingModesForTree( return; } - // An explicitly disabled block editing mode cascades to all its children. - // Check ancestors of the current block to see if the nearest one with a block - // editing mode set is disabled. + // Disabled explicit block editing modes are inherited by children. + // It's an expensive calculation, so only do it if there are disabled blocks. if ( hasDisabledBlocks ) { - const ancestorBlockEditingMode = getAncestorBlockEditingMode( - state, - clientId - ); + // Look through parents to find one with an explicit block editing mode. + let ancestorBlockEditingMode; + let parent = state.blocks.parents.get( clientId ); + while ( parent !== undefined ) { + // There's a chance we only just calculated this for the parent, + // if so we can return that value for a faster lookup. + if ( derivedBlockEditingModes.has( parent ) ) { + ancestorBlockEditingMode = + derivedBlockEditingModes.get( parent ); + } else if ( state.blockEditingModes.has( parent ) ) { + // Checking the explicit block editing mode will be slower, + // as the block editing mode is more likely to be set on a + // distant ancestor. + ancestorBlockEditingMode = + state.blockEditingModes.get( parent ); + } + if ( ancestorBlockEditingMode ) { + break; + } + parent = state.blocks.parents.get( parent ); + } + + // If the ancestor block editing mode is disabled, it's inherited by the child. if ( ancestorBlockEditingMode === 'disabled' ) { derivedBlockEditingModes.set( clientId, 'disabled' ); return; From 98f98cb1a53354d50f27aaa7a7d22e4cbea9fa6a Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 11 Dec 2024 14:58:47 +0800 Subject: [PATCH 14/16] Avoid state updates when setting block editor modes to the same thing as they already are --- packages/block-editor/src/store/reducer.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index da528d94de6da..fc3803462d892 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -1964,8 +1964,14 @@ export function temporarilyEditingFocusModeRevert( state = '', action ) { export function blockEditingModes( state = new Map(), action ) { switch ( action.type ) { case 'SET_BLOCK_EDITING_MODE': + if ( state.get( action.clientId ) === action.mode ) { + return state; + } return new Map( state ).set( action.clientId, action.mode ); case 'UNSET_BLOCK_EDITING_MODE': { + if ( ! state.has( action.clientId ) ) { + return state; + } const newState = new Map( state ); newState.delete( action.clientId ); return newState; From e008553f7038de5a7618993aebc65bdf91ce19a5 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 11 Dec 2024 16:05:01 +0800 Subject: [PATCH 15/16] Update DisableNonPageContentBlocks to not set block editor modes on all blocks at once --- .../disable-non-page-content-blocks.js | 64 +++++++++++++++---- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/packages/editor/src/components/provider/disable-non-page-content-blocks.js b/packages/editor/src/components/provider/disable-non-page-content-blocks.js index c47a7e5d7de7f..95295574c56a8 100644 --- a/packages/editor/src/components/provider/disable-non-page-content-blocks.js +++ b/packages/editor/src/components/provider/disable-non-page-content-blocks.js @@ -36,15 +36,49 @@ export default function DisableNonPageContentBlocks() { const registry = useRegistry(); + // The code here is split into multiple `useEffects` calls. + // This is done to avoid setting/unsetting block editing modes multiple times unnecessarily. + // + // For example, the block editing mode of the root block (clientId: '') only + // needs to be set once, not when `contentOnlyIds` or `disabledIds` change. + // + // It's also unlikely that these different types of blocks are being inserted + // or removed at the same time, so using different effects reflects that. + useEffect( () => { + const { setBlockEditingMode, unsetBlockEditingMode } = + registry.dispatch( blockEditorStore ); + + setBlockEditingMode( '', 'disabled' ); + + return () => { + unsetBlockEditingMode( '' ); + }; + }, [ registry ] ); + useEffect( () => { const { setBlockEditingMode, unsetBlockEditingMode } = registry.dispatch( blockEditorStore ); registry.batch( () => { - setBlockEditingMode( '', 'disabled' ); for ( const clientId of contentOnlyIds ) { setBlockEditingMode( clientId, 'contentOnly' ); } + } ); + + return () => { + registry.batch( () => { + for ( const clientId of contentOnlyIds ) { + unsetBlockEditingMode( clientId ); + } + } ); + }; + }, [ contentOnlyIds, registry ] ); + + useEffect( () => { + const { setBlockEditingMode, unsetBlockEditingMode } = + registry.dispatch( blockEditorStore ); + + registry.batch( () => { if ( ! isNavigationMode ) { for ( const clientId of templateParts ) { setBlockEditingMode( clientId, 'contentOnly' ); @@ -57,27 +91,33 @@ export default function DisableNonPageContentBlocks() { return () => { registry.batch( () => { - unsetBlockEditingMode( '' ); - for ( const clientId of contentOnlyIds ) { - unsetBlockEditingMode( clientId ); - } if ( ! isNavigationMode ) { for ( const clientId of templateParts ) { unsetBlockEditingMode( clientId ); } } + } ); + }; + }, [ templateParts, isNavigationMode, registry, disabledIds ] ); + + useEffect( () => { + const { setBlockEditingMode, unsetBlockEditingMode } = + registry.dispatch( blockEditorStore ); + + registry.batch( () => { + for ( const clientId of disabledIds ) { + setBlockEditingMode( clientId, 'disabled' ); + } + } ); + + return () => { + registry.batch( () => { for ( const clientId of disabledIds ) { unsetBlockEditingMode( clientId ); } } ); }; - }, [ - templateParts, - contentOnlyIds, - disabledIds, - isNavigationMode, - registry, - ] ); + }, [ disabledIds, registry ] ); return null; } From 0051caf4472e11ad7862ef22708c24f5636d734b Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 11 Dec 2024 16:09:53 +0800 Subject: [PATCH 16/16] Try breaking DisableNonPageContentBlocks into separate useEffect calls --- .../components/provider/disable-non-page-content-blocks.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/editor/src/components/provider/disable-non-page-content-blocks.js b/packages/editor/src/components/provider/disable-non-page-content-blocks.js index 95295574c56a8..ffbf1ac062546 100644 --- a/packages/editor/src/components/provider/disable-non-page-content-blocks.js +++ b/packages/editor/src/components/provider/disable-non-page-content-blocks.js @@ -84,9 +84,6 @@ export default function DisableNonPageContentBlocks() { setBlockEditingMode( clientId, 'contentOnly' ); } } - for ( const clientId of disabledIds ) { - setBlockEditingMode( clientId, 'disabled' ); - } } ); return () => { @@ -98,7 +95,7 @@ export default function DisableNonPageContentBlocks() { } } ); }; - }, [ templateParts, isNavigationMode, registry, disabledIds ] ); + }, [ templateParts, isNavigationMode, registry ] ); useEffect( () => { const { setBlockEditingMode, unsetBlockEditingMode } =